Conversation
github-actions
bot
added
the
store
Add unit tests to validate http-binary-cache. These were written to investigate NixOS#12910 unfortunately though after some research it turns out that '+' in the path is not encoded. I am contributing the unit tests nonetheless since they might prove fruitful in the future to test other functionality of the http binary cache store.
fzakaria
force-pushed
the
fzakaria/issue-12910
branch
from
acff068 to
dfc73fa
Compare
| { | ||
| nixFlake ? builtins.getFlake ("git+file://" + toString ../../..), | ||
| system ? builtins.currentSystem, | ||
| pkgs ? nixFlake.inputs.nixpkgs.legacyPackages.${system}, | ||
| stdenv ? "stdenv", | ||
| componentTestsPrefix ? "", | ||
| withInstrumentation ? false, | ||
| }@args: | ||
| let | ||
| # Create a pkgs set with your httplib overlay applied. | ||
| pkgs = import nixFlake.inputs.nixpkgs { | ||
| inherit system; | ||
| overlays = [ nixFlake.overlays.internal ]; | ||
| }; | ||
| in | ||
| import ./. ( | ||
| args | ||
| // { | ||
| inherit pkgs; | ||
| getStdenv = p: p.${stdenv}; | ||
| withSanitizers = withInstrumentation; | ||
| withCoverage = withInstrumentation; |
There was a problem hiding this comment.
@xokdvium you might want to review this change.
I am not sure why this wrapper.nix exists since the gha/tests file is also a check in the flake.nix.
I see you are the main author of it. The main problem was it was not propagating the overlays set in the flake.nix
| nix = final.nixComponents2.nix-cli; | ||
| }; | ||
|
|
||
| # We need to support pkgconfig until https://github.com/NixOS/nixpkgs/pull/452456 |
There was a problem hiding this comment.
| // FIXME: MacOS tests on Github actions were not able to bind to any port, | ||
| // it kept returning -1 for the bound port. | ||
| #if !defined(__linux__) | ||
| GTEST_SKIP() << "Skipping test on non-Linux platform due to unknown networking differences."; | ||
| #endif |
There was a problem hiding this comment.
@Ericson2314 I couldn't figure out why but the MacOS tests were not letting me opening up a port.
I tried "localhost" if it was IPV6 only but that also failed...
There was a problem hiding this comment.
@edolstra interesting; is the sandbox different on MacOS vs Linux for Nix for networking?
I would have thought localhost would always allowed.....
I am developing on a Linux machine so I couldn't dive deeper and the edit-cycle with GHA is quite slow.
This one was causing me to scratch my head... so if you have breadcrumbs I'd love to learn the root cause for edification.
🙏
There was a problem hiding this comment.
I think we need to add __darwinAllowLocalNetworking
Might be worth adding irrespective of the change to match Linux.
|
I am slightly apprehensive about adding deps such as these to the unit test suite, it feels a bit integration-test territory. OTOH, it might be possible to adapt |
|
In general, Nix tests should not listen on TCP ports because that doesn't work reliably unless the build runs in its own network namespace. And it opens a potential security issue if any process can connect to a server running inside the build. |
|
@edolstra is that true for localhost as well? also |
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
This addresses the concerns with network isolation that have been raised previously [1] by only running the tests by default in a network namespace. This way all networks tests are independent of each other and do not bind to ports in the host namespace. This is much neater than doing these sorts of tests in functional suite. [1]: NixOS#14266 (comment)
3 participants
Motivation
Add unit tests to validate http-binary-cache.
These were written to investigate #12910 unfortunately though after some research it turns out that '+' in the path is not encoded.
I am contributing the unit tests nonetheless since they might prove fruitful in the future to test other functionality of the http binary cache store.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.