asdf icon indicating copy to clipboard operation
asdf copied to clipboard

Failing tests on windows.

Open abhishalya opened this issue 5 years ago • 13 comments

In reference to https://github.com/asdf-vm/asdf/issues/450 and https://github.com/asdf-vm/asdf/pull/451 I have been preparing appveyor CI to run tests. Using cygwin bash most of the tests pass. I need some help in debugging the failing tests and possibly filling up the gap between Windows and Linux platforms.

not ok 1 exports ASDF_DIR
# (in test file test/asdf_fish.bats, line 22)
#   `")' failed with status 127
# /tmp/bats.493.src: line 22: fish: command not found
not ok 2 adds asdf dirs to PATH
# (in test file test/asdf_fish.bats, line 37)
#   `")' failed with status 127
# /tmp/bats.493.src: line 37: fish: command not found
not ok 3 does not add paths to PATH more than once
# (in test file test/asdf_fish.bats, line 54)
#   `")' failed with status 127
# /tmp/bats.493.src: line 54: fish: command not found
not ok 4 defines the asdf function
# (in test file test/asdf_fish.bats, line 69)
#   `")' failed with status 127
# /tmp/bats.493.src: line 69: fish: command not found
not ok 69 asdf exec should pass all arguments to executable even if shim is not in PATH
# (in test file test/shim_exec.bats, line 42)
#   `[ "$output" == "" ]' failed

You can see the logs here.

abhishalya avatar Feb 13 '19 13:02 abhishalya

Do we actually care about windows as a CI platform? BATS is a bash test runner and it's not like windows has its own version of bash. We should probably just delete the windows CI stuff to not confuse people.

deiga avatar Dec 31 '19 08:12 deiga

It seems WSL2 is available on the GitHub Actions Windows environment, so I would like to co-opt this Issue to target adding that test environment and fixing the issues that will arise.

Related Issues (open) to WSL:

  • #730
  • #573
  • #558

Related Issues (closed) to WSL:

  • #450
  • #736
  • #743

Currently we're continuing on test failures on Windows: https://github.com/asdf-vm/asdf/blob/d37e99a21ed0c20e9ebab3166cca9ed28978ef7c/.github/workflows/workflow.yml#L57 We should change this.

These are the failing tests I found when running Bats on my local Win10, WSL2, Ubuntu 20.04 system:

 ✗ exports ASDF_DIR
   (in test file test/asdf_fish.bats, line 22)
     `")' failed with status 127
   /tmp/bats.8963.src: line 22: fish: command not found
 ✗ adds asdf dirs to PATH
   (in test file test/asdf_fish.bats, line 37)
     `")' failed with status 127
   /tmp/bats.8963.src: line 37: fish: command not found
 ✗ does not add paths to PATH more than once
   (in test file test/asdf_fish.bats, line 54)
     `")' failed with status 127
   /tmp/bats.8963.src: line 54: fish: command not found
 ✗ defines the asdf function
   (in test file test/asdf_fish.bats, line 69)
     `")' failed with status 127
   /tmp/bats.8963.src: line 69: fish: command not found
...
 ✗ asdf exec should pass all arguments to executable even if shim is not in PATH
   (in test file test/shim_exec.bats, line 42)
     `[ "$output" == "" ]' failed with status 127
...
 ✗ shim exec should fallback to system executable when specified version is system
   (in test file test/shim_exec.bats, line 205)
     `[ "$output" == "System" ]' failed with status 127
...
 ✗ shim exec should execute system if set first
   (in test file test/shim_exec.bats, line 234)
     `[ "$output" == "System" ]' failed with status 127
...
 ✗ shim exec doest not use custom exec-env for system version
   (in test file test/shim_exec.bats, line 278)
     `[ "$output" == "x System" ]' failed with status 127
...
 ✗ shim exec should remove shim_path from path on system version execution
   (in test file test/shim_exec.bats, line 334)
     `[ "$output" == "$ASDF_DIR/shims/dummy" ]' failed with status 127
   127 env: ‘Files/dotnet/:/mnt/c/Users/james/AppData/Local/Microsoft/WindowsApps:/mnt/c/Users/james/AppData/Local/Programs/Microsoft’: No such file or directory
...
 ✗ shim exec doesnt execute command if pre-hook failed
   (in test file test/shim_exec.bats, line 435)
     `[ "$output" == "hello no dummy world" ]' failed with status 127
...
 ✗ which should show path of system version
   (in test file test/which_command.bats, line 57)
     `[ "$status" -eq 0 ]' failed with status 127
...
 ✗ which should not return shim path
   (in test file test/which_command.bats, line 106)
     `[ "$status" -eq 1 ]' failed with status 127

jthegedus avatar Aug 19 '20 10:08 jthegedus

GitHub Actions for WSL setup - https://github.com/marketplace/actions/setup-wsl

jthegedus avatar Sep 04 '20 03:09 jthegedus

Do we actually care about windows as a CI platform?

We do care about the test runner. We want to execute our tests in all environments/platforms asdf can run. WSL1 / WSL2 / Ubuntu / macOS all have system quirks and differences, notably filesystem interactions etc which we interact with and should capture.

#956 has added the WSL1 environment using setup/wsl GH Action. We still need to update the tests.

jthegedus avatar May 25 '21 13:05 jthegedus

wsl2 is just linux, not a windows runner. not really helpful for ci/cd of windows tools, services or packages. native windows support would rock for devs who are coding up windows platform tools

from what i've seen of pyenv, nvm and others, they have special support for windows in a separate repo, rather than including it in the main system

while this works, it's actually a pretty bad experience for multiplatform development, because they are all idiosyncratic and different from the base tools. (also there's no good reason for this with a proper abstraction later, and asdf-vm seems to have this already)

earonesty avatar Jun 03 '22 15:06 earonesty

@earonesty

wsl2 is just linux, not a windows runner

WSL has been different enough that we get issues specifically reported for it. Ideally we would capture new tests in that specific shell environment to ensure no regressions. That is the goal of running a WSL2 shell instance in GitHub Actions.

You are right, that for the most part, WSL2 works pretty well and is better than a hacky, bespoke Windows specific solution. This issue is by no means urgent.

jthegedus avatar Jun 16 '22 16:06 jthegedus

Just a March 2023 update on this, I ran things against Windows 11 WSL2 and all tests passed like they are supposed to with the exception of one from plugin_add_comand.bats:

BATS Output
...
plugin_add_command.bats
 ✓ plugin_add command with plugin name matching all valid regex chars succeeds
 ✓ plugin_add command with LANG=sv_SE.UTF-8 and plugin name matching all valid regex chars succeeds
 ✓ plugin_add command with plugin name not matching valid regex fails 1
 ✓ plugin_add command with plugin name not matching valid regex fails 2
 ✓ plugin_add command with plugin name not matching valid regex fails 3
 ✓ plugin_add command with no URL specified adds a plugin using repo
   plugin_add command with no URL specified adds a plugin when short name repository is enabled                                                             ✗ plugin_add command with no URL specified adds a plugin when short name repository is enabled
   (in test file test/plugin_add_command.bats, line 72)
     `[ "$status" -eq 0 ]' failed
 ✓ plugin_add command with no URL specified fails to add a plugin when disabled
 ✓ plugin_add command with URL specified adds a plugin using repo
 ✓ plugin_add command with URL specified run twice returns error second time
 ✓ plugin_add command with no URL specified fails if the plugin doesn't exist
 ✓ plugin_add command executes post-plugin add script
 ✓ plugin_add command executes configured pre hook (generic)
 ✓ plugin_add command executes configured pre hook (specific)
 ✓ plugin_add command executes configured post hook (generic)
 ✓ plugin_add command executes configured post hook (specific)
...

Both MSYS2/CLANG64 and MINGW64 bundled with Git Bash on the other hand (I couldn't get things to run MSYS2/MINGW64) fails miserably with something like 70 failed tests.

Honestly, I don't think it is worth it to be supporting something like MINGW64 when we have something as excellent as WSL2 - we should be focusing our efforts there. Even if we get all the tests to pass, MINGW64 and CYGWIN are still going to be extremely slow - and it'll be more trouble than its worth - CI would take forever to complete too. Same thing with - WSL1 - WSL2 is better in ever area and WSL1 will only continue to be less relevent as time passes.

Previously, there were comments about how it's a different "operating environment" and there are some separate bug reports because of it. Ever since WSL started to support systemd, I don't think there is that much of a divergence from what a linux distibution would traditionally behave. Meaning, there is less risk to advertise that asdf "supports windows via wsl2".

My vote is to close this issue once the last remaining failing test on WSL2 is fixed, and more clearly advertise in the docs that asdf should work fine on WSL2, but state that we can't validate via CI just yet.

hyperupcall avatar Mar 25 '23 10:03 hyperupcall

Honestly, I don't think it is worth it to be supporting something like MINGW64 when we have something as excellent as WSL2 - we should be focusing our efforts there. Even if we get all the tests to pass, MINGW64 and CYGWIN are still going to be extremely slow - and it'll be more trouble than its worth - CI would take forever to complete too. Same thing with - WSL1 - WSL2 is better in ever area and WSL1 will only continue to be less relevent as time passes.

super, strongly, strongly, and more super strognly massively disagree with this.

There is nothing, n.o.t.h.i.n.g ... i repeat nothing that can replace asdf on windows.

no wsl is not the answer.

windows environment desparately needs a tooling manager for things that cant run in wsl2.

Why can't this run in the msys bash? or even the winbash?

What is asdf doing that's sooooooooo exotic that it can't be done in those shells?

airtonix avatar Apr 29 '23 00:04 airtonix

@hyperupcall on a side note:

are you creating https://github.com/version-manager/woof so we can have something like asdf on all platforms?

edit just realised it's not using shims, so it will suffer the same problems that nvm has.

airtonix avatar Apr 29 '23 00:04 airtonix

@airtonix asdf maintainers contribute to this library in their free time, which means we are limited in the platforms we support. You can see more information about windows support in #450, and support in general in #1436. Theoretically, asdf is supposed to be able to run in a mostly POSIX-like environment with a recent enough version of Bash, but as I'm sure you know, when supporting different platforms, quirks and edge cases always come up. It's simple to just say "just use X or Y", but it's less straightfoward to implement it, add testing for it, and ensure we are consistently able to fix issues for it. That's why we are talking about WSL2 as a solution - it is the closest environment to what we already support and our time is most efficient when focusing on that. So it's less of a question of the quirkiness of asdf, and more about how best we can support our users given constraints that are external to the code.

Maybe this issue should probably be closed since it's a duplicate of #450. This issue implies we support Windows when we don't.

hyperupcall avatar Apr 29 '23 00:04 hyperupcall

My personal project Woof only supports "running on Windows" through WSL2 - for personal projects of mine I don't bother to support operating systems other than Linux and maybe macOS. I think for Woof it is intended to support all POSIX-like platforms, but that is a longer-term goal.

I'm not sure what specifically what problems you are referring to about nvm, but shims are not the way to go because it reduces performance and it's difficult to reliably reshim things (so you end up having to do it manually). Symlinks are good for performance, but you still have to "reshim" - and it's difficult to have per-project/local specific versions with symlinks. Woof uses a different approach (similar to rtx) - if you have any questions about Woof, feel free to open an issue in my repository.

hyperupcall avatar Apr 29 '23 00:04 hyperupcall

Yeah don't agree.

If we're going to talk about "actions that convey implied meaning" then closing this and documenting "wont support: too hard, unix is magical and windows just cant do it" isn't true and feels like a shutdown of conversation.

If you want to open the floor to contribution then document what it is about asdfs current use of bash that creates issues for winbash/msysgit/etc.

This way, future contributors who are more motivated/free of bias than myself or you can laser focus in on the problem.

I'm not sure what specifically what problems you are referring to about nvm,

  • NVMs method of the path meant that only tooling launched from the shell would see the intended tool version
  • any tooling launched from within that process is also now locked into that version despite it living in another path with a different .nvmrc

had massive problems with this, quickly switcted to nodist instead which had none of those problems. This is also a major reason why I love asdf.

airtonix avatar Apr 29 '23 00:04 airtonix

If we're going to talk about "actions that convey implied meaning" then closing this and documenting "wont support: too hard, unix is magical and windows just cant do it" isn't true and feels like a shutdown of conversation.

I mentioned some of the practical limitations that the maintainers (and me as a frequent contributor) have when supporting new platforms, so when you exaggerate my position with the "unix is magical" comment, it feels like you're not listening. I do apologize if it seemed like I was shutting down the conversation, my intentions were to make things easier to follow as we have two different issue with pretty much the same discussion - #450 will still be open and the discussion can still be continued from there.

If you want to open the floor to contribution then document what it is about asdfs current use of bash that creates issues for winbash/msysgit/etc.

The way I see it, it is not worth it to document the things asdf does that makes it incompatible with winbash, etc. These things are unknowns until we start the actual porting process, and it would be easier just to resolve it right then and there, rather than make a list and wait for other people to fix it. In my opinion, when you know the cause of a bug, you're already most of the way there. By all means, if you are using a specific environment and asdf will Just Work with a few fixes, feel free to submit PRs, but we can't promise that it will continue to work. The maintainers may have a different point of view, but this is my perspective.

had massive problems with this, quickly switcted to nodist instead which had none of those problems. This is also a major reason why I love asdf.

Oh right, I forgot about that. Yeah I do recognize that issue and I understand there is a tradeoff - eventually I plan to implement some hybrid mode, but for now I'm not doing that

hyperupcall avatar Apr 29 '23 01:04 hyperupcall