Make environment-dependent Go tests local-friendly#199
Conversation
gabyx
left a comment
There was a problem hiding this comment.
Thanks for the PR:
The test changes are good and I am fine with them.
However I am a bit unsure if I want a pre-commit hook which runs Go tests cause I am not sure how long they take and also how useful
it is.
Ground truth is always CI, so nothing should be merged anyway to main. At least for me this hook is not that useful and I will anyway disable it if I work in this repo cause, my workflow is change things, break it, fix tests/lints - merge etc... So I don't wont to be bothered by tests with committing.
The other thing (which does not affect merging) is that I work with Nix and the flake.nix inside a DevShell, that means there is never an environment where some tools are missing, which is nice. I work everywhere with Nix shells which are robust, stable, reproducible and make the whole dev cycle a breeze.
You should try it.
I of course understand not everybody wants this awesomeness 😉,
therefore my suggestion:
The pre-commit hook should be option al by an env var. PRECOMMIT_TEST=true or so or removed entirely.
We could have an .env.tmpl file which suggests to set that variable in .env which can be sourced (in the Nix shell) or by other means...
What do you think.
You're very welcome. That makes sense.I will delete the unit test pre-commit hook, then. I think anyone who wants this can just add pre-commit hook by themselves rather than The rationale was that unit test should be fast without any dependencies to environments rather than code itself. I will come back with better proposal after a few more iterations. |
e00bf16 to
41236d7
Compare
|
Removed the pre-commit unit-tests hook per your feedback. Force-pushed; PR now contains only the test hermeticity fixes (skip-locally / fail-in-CI guards in |
I tried with nix. It didn't help me much, unfortunately. Only "format" worked in addition. What it helped was that I could use "just" and "treefmt" without installing it, which was a small benefit. "go" natively supports downloading the right toolchain versions and tool versions, so nix was unnecessary. |
|
Some tests are still failing not sure if it can be ignored. For the next week I dont have time to look at it directly |
Closes #198
Summary
Testing
go test -mod=vendor ./hooksGOCACHE=/tmp/gocache CGO_ENABLED=0 go test -mod=vendor ./...