onsi / ginkgo

A Modern Testing Framework for Go
http://onsi.github.io/ginkgo/
MIT License
8.12k stars 644 forks source link

ginkgo refuses to run pre-compiled test binary on Windows #1296

Closed jgrieger closed 8 months ago

jgrieger commented 8 months ago

If I run ginkgo build \my-package-dir on Windows (21H2), the output is my-package.test binary.

If I run ginko my-package.test, the validation in https://github.com/onsi/ginkgo/blob/7161a9d90a24fd70df1e687b71d4ae186ec85dd7/ginkgo/internal/test_suite.go#L195C33-L195C33 fails, because Go does not provide the executable bit in Windows (see https://github.com/golang/go/issues/41809).

The workaround is to rename the test binary to my-package.exe.

This is related to https://github.com/onsi/ginkgo/issues/529.

onsi commented 8 months ago

hey @jgrieger are you up for a PR for this? i think my preferred approach here would be to only enforce the executable bit check if the OS is not windows, but i don’t have a windows machine so it’s possible something else gets in the way further downstream - if you could try to put something together and validate that it works for you that would be great.

jgrieger commented 8 months ago

hey @jgrieger are you up for a PR for this? i think my preferred approach here would be to only enforce the executable bit check if the OS is not windows, but i don’t have a windows machine so it’s possible something else gets in the way further downstream - if you could try to put something together and validate that it works for you that would be great.

Sure, working on it.

onsi commented 8 months ago

thanks!

jgrieger commented 8 months ago

@onsi I think we have two options here:

  1. simply omit the bit check for Windows (runtime.GOOS == "windows")
  2. use the Go package golang.org/x/sys/windows for calling GetBinaryTypeW function of Windows' kernel32.dll (I created a POC yesterday, for a test binary built by Ginkgo I received SCS_64BIT_BINARY => "A 64-bit Windows-based application.")

Option 1

Pro

Contra

Option 2

Pro

Contra

What do you think?

onsi commented 8 months ago

hey @jgrieger thanks for taking the time to explore options. I’d go the simpler route - option 1 - those non-executable files aren’t actually going to run any tests and succeed and so the user will get an error even if the file in question gets past the guard. Thanks!

jgrieger commented 8 months ago

PR #1301 created.

onsi commented 8 months ago

merged it in, thanks! i’ll cut a release soon.

jgrieger commented 8 months ago

Great, thanks!

onsi commented 8 months ago

2.13.1 is out now. You can close this out if you're happy with where we've ended up. Thanks for the PR!

jgrieger commented 8 months ago

Thank you for your support, much appreciated :-).