-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
nixos/cross-seed: create outputDir on start and re-enable test #384570
base: master
Are you sure you want to change the base?
Conversation
Also adds an update script
@kevincox here's the fix! As far as I could tell it's not really an issue with the sandboxing per-se, it's just a combination of a now-fixed bug in cross-seed and me forgetting to create the directory in the module. Both of those are now taken care of! |
services.cross-seed = { | ||
enable = true; | ||
settings = { | ||
outputDir = "/var/lib/cross-seed/output"; |
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 is the default right? Maybe we should not set it to emphasize that we are testing with only the minimal set of required options?
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.
Please ensure that you run the tests before sending for review.
@@ -294,6 +294,7 @@ in { | |||
curl-impersonate = handleTest ./curl-impersonate.nix {}; | |||
custom-ca = handleTest ./custom-ca.nix {}; | |||
croc = handleTest ./croc.nix {}; | |||
cross-seed = runTest ./cross-seed.nix {}; |
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.
cross-seed = runTest ./cross-seed.nix {}; | |
cross-seed = runTest ./cross-seed.nix; |
The test fails to evaluate.
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.
Ah damn, I was running the tests before rebasing on master and then accidentally re-added it wrong 🤦
start_all() | ||
machine.wait_for_unit("cross-seed.service") | ||
machine.wait_for_open_port(2468) | ||
# Check that the API is running | ||
machine.succeed("curl --fail http://localhost:2468/api/ping?apiKey=${apiKey}") |
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 indent is broken causing the test to fail.
Re-enables the test removed in #384279.
The issue with the broken test was multi-faceted:
I also changed the API endpoint to the
/ping
endpoint, and added an update script to the package.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.