Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jgrieger opened this issue Nov 8, 2023 · 10 comments
Closed

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

jgrieger opened this issue Nov 8, 2023 · 10 comments

Comments

@jgrieger
Copy link
Contributor

jgrieger commented Nov 8, 2023

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 golang/go#41809).

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

This is related to #529.

@onsi
Copy link
Owner

onsi commented Nov 8, 2023

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
Copy link
Contributor Author

jgrieger commented Nov 8, 2023

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
Copy link
Owner

onsi commented Nov 8, 2023

thanks!

@jgrieger
Copy link
Contributor Author

jgrieger commented Nov 9, 2023

@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

  • simple, less error-prone
  • fastest

Contra

  • Might let pass non-executable files on Windows

Option 2

Pro

Contra

  • might take longer in terms of execution time (didn't do any perf checks yet)
  • P/invoke into OS with unsafe pointers etc. does never feel right 😂

What do you think?

@onsi
Copy link
Owner

onsi commented Nov 9, 2023

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
Copy link
Contributor Author

PR #1301 created.

@onsi
Copy link
Owner

onsi commented Nov 10, 2023

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

@jgrieger
Copy link
Contributor Author

Great, thanks!

@onsi
Copy link
Owner

onsi commented Nov 10, 2023

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants