-
-
Notifications
You must be signed in to change notification settings - Fork 270
chore: general linting and update integration tests to be cross-platform #133
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
Conversation
- Use `errors.Is` for comparing errors in `cmd/anubis/main.go`. - Use `errors.New` instead of `fmt.Errorf` for simple errors in `cmd/containerbuild/main.go` and `lib/anubis.go`. - Align fields in structs in `lib/policy/bot.go` and `lib/policy/config/config.go`. - Remove unnecessary variable shadowing in tests. - Use `t.TempDir()` in `internal/test/playwright_test.go`. - Use `strconv.Itoa` instead of `fmt.Sprint` for integer conversion in `lib/anubis_test.go`.
Updates integration tests to function correctly on both Windows and Unix-like systems. Replaces shell-specific commands with cross-platform alternatives for running and daemonizing processes. Also includes a more robust temporary file creation mechanism. Improves reliability and consistency of the test suite across different operating systems.
Signed-off-by: Jason Cameron <jasoncameron.all@gmail.com>
if runtime.GOOS == "windows" { | ||
cmd = exec.Command("cmd", "/C", "start", "/B", command) | ||
} else { | ||
cmd = exec.Command("sh", "-c", command+" &") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't do what you think it will I don't think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I tested it on windows/linux, and it worked fine. I think my only difference was the background (via & and /B), I can run the previous tests as well to double check they behave the same
Signed-off-by: Jason Cameron <jasoncameron.all@gmail.com>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Bump! The integration tests not working on windows is pretty annoying |
I'm not sure I want to merge this as-is. I understand the desire to do this testing from windows, but there's significant semantic differences between cmd.exe and bash syntax in ways that would make it annoying to write. I personally do all my development for Anubis in WSL (specifically in an Ubuntu container). I think it may be better to just tell people to use WSL to do this for now. I do want to have better windows support eventually, but Windows needs dedicated experience and knowledge in ways that I just don't have the time for. I was personally planning on making windows support first-class when there's an enterprise customer with windows specifically in mind. Well that, and the fact that Windows server costs enough that people are not willing to run it for open source projects. I'd love to be proven wrong though! |
Fair enough. When that enterprise client comes knocking, feel free to ping me, and I'm happy to work on that. I'll open a PR with just the linting changes sometime tomorrow. Thanks for the response! |
though maybe we should update the integration_test.go file to have !goos=windows buildflag as whenever I run |
Makes sense. Ship it! |
Updated the integration tests to be cross-platform (tested on Windows 11 & ubuntu)
Some linting fixes & fieldalignement where it didn't look too bad.
Didn't add changes to unreleased as all changes are already present
Checklist:
[Unreleased]
section of docs/docs/CHANGELOG.md