-
Notifications
You must be signed in to change notification settings - Fork 0
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
Run partial test suite without sudo #7
Labels
testing
Improvement or additions to testing
Comments
For the tests that do require sudo, we should depend on an isolated enviroment that gets spun up (locally for development and within Github actions) where the user can run as root (or with specific capabilities enabled). |
mdaverde
added a commit
that referenced
this issue
Sep 23, 2022
bpf-rs & bpf-feature tests need capabilities (to read certain directories or load bpf programs) that are not available to a regular user. This is strictly possible by just wrapping `cargo test` with `sudo` (such as with `sudo -E "PATH=$PATH" $(which cargo) test --all-features`) but the issue here (besides ergonomics) is when cargo tests build & write new files they are now owned by root (making future non-sudo builds fail). It might be possible to configure sudo to strictly write new files as a separate user from root but I haven't seen that. Ideally, we just run the created test bins with sudo after compilation and this is possible with specifying a runner in .cargo/config.toml. We can specify this at the workspace level for the relevant target triple but the issue with this is that sudo disallows certain libraries to be dynamically linked which the proc_macro crate (used transitively by bpf-rs-macros) needs. So we're left with the option of forcing sudo to allow loading of these libraries or to only using sudo on the crate tests that need it. The problem is that cargo itself changes LD_LIBRARY_PATH: https://doc.rust-lang.org/cargo/reference/environment-variables.html#dynamic-library-paths which we don't have access to from the runner's perspective, especially given that variable shell expansion is not supported: rust-lang/cargo#10789 For now, we're choosing the latter (only using sudo on specific crates) but we ran into another issue: config.toml is ignored on a per-crate basis when running cargo test from a workspace. To workaround this, we change our local task `just test` to cd into each crate and run tests from there. This is not ideal but gets the job done for now. We keep our github actions to just run all the tests as the original sudo cmd for now since we don't care about the owners of the new files after the vm disappears. Related to #7 Signed-off-by: Milan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We have tests that can be easily run without root so we don't force root to run a partial test suite as we do in
just sudo-test
The text was updated successfully, but these errors were encountered: