ci: test NixOS' VM test for PostgREST#4872
ci: test NixOS' VM test for PostgREST#4872wolfgangwalther wants to merge 1 commit intoPostgREST:mainfrom
Conversation
wolfgangwalther
force-pushed
the
nixpkgs-nixos-test
branch
2 times, most recently
from
f3cde9d to
c33b45f
Compare
| pkgs.callPackage nix/tools/docker { postgrest = postgrestStatic; }; | ||
|
|
||
| # NixOS VM tests | ||
| nixpkgs-nixos-test = runTest (pkgs.path + "/nixos/tests/postgrest.nix"); |
There was a problem hiding this comment.
Some questions:
- Does this work by invoking this test on nixpkgs?https://github.com/NixOS/nixpkgs/blob/151ad2221f19cbbc659c74dd1d1dd0cb29588d7a/nixos/tests/postgrest.nix#L1
- Shouldn't we vendor this test on our repo?
- Can we manually run this test? If so, can we leave a comment here on how to do it?
There was a problem hiding this comment.
Some questions:
Does this work by invoking this test on nixpkgs?https://github.com/NixOS/nixpkgs/blob/151ad2221f19cbbc659c74dd1d1dd0cb29588d7a/nixos/tests/postgrest.nix#L1
Correct. With the exception that it passes our postgrest, not the latest version available in NixOS.
Shouldn't we vendor this test on our repo?
Not sure about that. The test in itself doesn't give us any value - the things tested there we already test. The (little!) value we get from this is by the fact that we're including the Nixpkgs test without vendoring.
The most value I get from this test is a Nixpkgs-postgrest-maintainer, because I have some kind of notification ahead of time, when I will eventually need to adjust the Nixpkgs test to new PostgREST behavior. This is only the case when it's impossible to change the test from this repo.
Can we manually run this test? If so, can we leave a comment here on how to do it?
I would not expect this test to be needed to run locally at all. It should be entirely fine if we only do so on CI. Of course, you can run it: Just do nix-build -A nixpkgs-nixos-test - you can do so with all these attributes in the attrset returned from default.nix.
to be clear: It'd be entirely OK to not even merge this at all. I already got some information of running CI on this: aarch64-linux runners don't support KVM, so running VM tests would be slow for that architecture. But x86_64-linux seems to work fine in GitHub Actions.
There was a problem hiding this comment.
The most value I get from this test is a Nixpkgs-postgrest-maintainer, because I have some kind of notification ahead of time, when I will eventually need to adjust the Nixpkgs test to new PostgREST behavior.
🤔 Hm, I was thinking this is a bit out of scope. But..
This is essentially just a sanity check to make sure we're not breaking something fundamentally - and if we do, it's a head up for me to adjust the Nixpkgs tests accordingly. Those might otherwise break unnoticed since Nixpkgs does not have a good notification system for such breakages in place.
We do use nixpkgs to generate the static binary, could this new test help us identify if something fails with the binary in case something changes or is missing in nixpkgs? Or is that redundant with our own tests?
I had a case before were the x86 Nix static binary failed DNS resolution, this was patched by switching to a custom Ubuntu binary. AFAICT this is not cleared yet, though not sure how reproducible it is or if this nixos VM test could help prove it.
There was a problem hiding this comment.
I like the idea of using this as a test for the static binary itself. We don't test this at all, we could do it here.
I had a case before were the x86 Nix static binary failed DNS resolution, this was patched by switching to a custom Ubuntu binary. AFAICT this is not cleared yet, though not sure how reproducible it is or if this nixos VM test could help prove it.
Any chance you still know whether that was docker based or not?
wolfgangwalther
force-pushed
the
nixpkgs-nixos-test
branch
from
c33b45f to
58fe73a
Compare
This adds a CI job to run our latest postgrest version against the NixOS VM test currently available in Nixpkgs. Now, this will not always be in sync, so has the potential to be failing. However, since the Nixpkgs VM test is really simple, this should only happen when we introduce a breaking change on a very fundamental level. The failing test will then be resolved once the new version is available in Nixpkgs and we have updated our lock file. This is essentially just a sanity check to make sure we're not breaking something fundamentally - and if we do, it's a head up for me to adjust the Nixpkgs tests accordingly. Those might otherwise break unnoticed since Nixpkgs does not have a good notification system for such breakages in place. We do use the chance to run the static executable in this test, which was previously not tested at all. It also gives us a first test whether NixOS VM tests work well in GitHub Actions.
wolfgangwalther
force-pushed
the
nixpkgs-nixos-test
branch
from
58fe73a to
5821696
Compare
2 participants
This adds a CI job to run our latest postgrest version against the NixOS VM test currently available in Nixpkgs. Now, this will not always be in sync, so has the potential to be failing. However, since the Nixpkgs VM test is really simple, this should only happen when we introduce a breaking change on a very fundamental level. The failing test will then be resolved once the new version is available in Nixpkgs and we have updated our lock file.
This is essentially just a sanity check to make sure we're not breaking something fundamentally - and if we do, it's a head up for me to adjust the Nixpkgs tests accordingly. Those might otherwise break unnoticed since Nixpkgs does not have a good notification system for such breakages in place.
It also gives us a first test whether NixOS VM tests work well in GitHub Actions.