Skip to content
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

dandischema/tests/test_metadata.py::test_migrate_041 requires network access #87

Closed
TheChymera opened this issue Sep 27, 2021 · 5 comments · Fixed by #88
Closed

dandischema/tests/test_metadata.py::test_migrate_041 requires network access #87

TheChymera opened this issue Sep 27, 2021 · 5 comments · Fixed by #88
Assignees

Comments

@TheChymera
Copy link

The dandischema/tests/test_metadata.py::test_migrate_041 test fails inside the root sandbox for system-wide install. It works if I test it manually based on the source directory as my user. Not really a big issue since it can be easily be blacklisted for the system-wide install.

Is it in any case possible to make it work without network access?
This is the full log showing it fail: https://ppb.chymera.eu/cbaa53

@waxlamp
Copy link
Member

waxlamp commented Sep 27, 2021

It looks like maybe this was caused by a transient network failure on GitHub's end. If you run the tests again now, do you get the same error?

@TheChymera
Copy link
Author

@waxlamp well the sandbox for the system-wide install prevents any outgoing connections to avoid code injection, so very obviously it will never be able to reach github. All the files needed should be declaratively downloaded and have a checksum distributed to the package manager.

It all works ofc from outside the sandbox. Just wondering whether this test can be sandboxed since everything else worked so nicely, which was a pleasant surprise.

@waxlamp
Copy link
Member

waxlamp commented Sep 27, 2021

Ah, sorry, I missed the bit in your original comment about the sandbox.

Can you give me some more detail about how you're installing this package system wide? Are you using something like NixOS which forces all software installation through such a sandbox?

@yarikoptic
Copy link
Member

I think we are not yet annotating tests for which need network (well -- internet), so they could be excluded while testing in a sandbox. In dandi-cli we have @mark.skipif_no_network and we should introduce it here in dandischema as well. Dear @jwodder, please add that marker and may be even add a dedicated (single) CI run which would run only tests not requiring internet connection and also "cutting off" the network by providing some bogus http_proxy=http://127.0.0.1:9/ env var, so would fail if tests does need network and was not yet marked to be skipped if no network.

@jwodder
Copy link
Member

jwodder commented Sep 27, 2021

The repository has been modified since the original post so that the test in question no longer exists, and I am unable to get anything to fail with http_proxy set to an invalid value. @TheChymera Does running the latest code in your sandbox still produce failing tests?

yarikoptic added a commit that referenced this issue Sep 28, 2021
Mark tests requiring network access and add a test workflow that disables network access
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants