search

cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.07k stars 288 forks source link

cmd/cue: some tests are probably not compatible with vanilla Windows #1821

Open mvdan opened 2 years ago

mvdan commented 2 years ago

Some test scripts like https://github.com/cue-lang/cue/blob/06484a39d8d44c656212ac2d3b5589f8aa9db033/cmd/cue/cmd/testdata/script/cmd_after.txt make use of exec.Run with sh, meaning that the test requires a POSIX shell in $PATH.

This is fine on any unix-y system, and apparently fine on the Windows we use in CI - it uses Bash as the shell, so it's probably running on a unix-like environment as well.

However, I'm willing to bet that go test ./... on a vanilla Windows machine would fail, because its cmd shell does not ship with a sh program from what I can remember.

We could add [!exec:sh] skip 'this test requires a posix shell', but I reckon that's not a good idea - we should keep our tests portable where possible. All that this test uses a shell for is sleeping and using timestamps, so we could easily do that in a small Go program, for example.

cc @myitcv

myitcv commented 2 years ago

Perhaps we first move away from defining the entire workflow as bash-based and instead do the "standard" thing per platform? That would flush out the tests that are broken.

mvdan commented 2 years ago

I would agree with that. Otherwise it's hard to say whether go test ./... really will work on all three major platforms. Or potentially worse than that - some features of cmd/cue might not work properly on Windows and we're not noticing because we're running the tests in this Unix-like compatibility layer on Windows.

mvdan commented 2 years ago

I gave that a quick go with https://review.gerrithub.io/c/cue-lang/cue/+/541803, and it fails as expected, but just via one test - presumably a LF vs CRLF discrepancy. I guess their CI environment still has sh installed on vanilla Windows.

Happy to pick this up again if/when Windows support, especially for the sake of developing or testing CUE itself, is more of a priority.

myitcv commented 2 years ago

Yeh, not a priority for now I would say.