-
Notifications
You must be signed in to change notification settings - Fork 255
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
Steps toward hermetic integration tests #644
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for aya-rs-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
fd75ce0
to
d07b600
Compare
alessandrod
approved these changes
Jul 13, 2023
- rustfmt settings are hierarchical. - integration-ebpf is always compiled at a distance with flags provided. - .cargo/config.toml is not respected except at the root of the workspace[0]. [0] https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure
- Add libbpf as a submodule. This prevents having to plumb its location around (which can't be passed to Cargo build scripts) and also controls the version against which codegen has run. - Move bpf written in C to the integration-test crate and define constants for each probe. - Remove magic; each C source file must be directly enumerated in the build script and in lib.rs.
This command builds the integration test binaries and prints their paths to stdout.
Use the environment variable AYA_BUILD_INTEGRATION_BPF to indicate to the build script that it should *actually* build bpf, otherwise emitting empty files. This allows metadata builds to skip costly build steps without sacrificing ergonomics; all compile-time tools such as cargo clippy work out of the box. Cargo even gives each of these builds (depending on the value of the environment variable) its own cache key, so they do not invalidate each other when the user alternates between metadata and real builds. This allows the lint action to move out of the VM.
Now that building integration tests works out of the box, we can.
"integration tests" as defined by Cargo produce a binary per file in the tests directory. This is really not what we want and has a number of downsides, but the main one is binary size. Before: tamird@pc:~/src/aya$ cargo xtask build-integration-test | xargs ls -lah Finished dev [unoptimized + debuginfo] target(s) in 0.05s Running `target/debug/xtask build-integration-test` Compiling integration-test v0.1.0 (/home/tamird/src/aya/test/integration-test) Finished dev [unoptimized + debuginfo] target(s) in 0.68s -rwxrwxr-x 1 tamird tamird 34M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/bpf_probe_read-e03eb905a5e6209c -rwxrwxr-x 1 tamird tamird 35M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/btf_relocations-57a4fbb38bf06064 -rwxrwxr-x 1 tamird tamird 31M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/elf-98b7a32d6d04effb -rwxrwxr-x 1 tamird tamird 6.9M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/integration_test-0dd55ce96bfdad77 -rwxrwxr-x 1 tamird tamird 34M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/load-0718562e85b86d03 -rwxrwxr-x 1 tamird tamird 40M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/log-dbf355f9ea34068a -rwxrwxr-x 1 tamird tamird 36M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/rbpf-89a1bb848fa5cc3c -rwxrwxr-x 1 tamird tamird 34M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/relocations-cfe655c3bb413d8b -rwxrwxr-x 1 tamird tamird 34M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/smoke-ccd3974180a3fd29 After: tamird@pc:~/src/aya$ cargo xtask build-integration-test | xargs ls -lah Finished dev [unoptimized + debuginfo] target(s) in 0.05s Running `target/debug/xtask build-integration-test` Compiling integration-test v0.1.0 (/home/tamird/src/aya/test/integration-test) Finished dev [unoptimized + debuginfo] target(s) in 0.90s -rwxrwxr-x 1 tamird tamird 47M Jul 12 15:21 /home/tamird/src/aya/target/debug/deps/integration_test-0dd55ce96bfdad77 Since we plan to run these tests in a VM, copying 10x fewer bytes seems like a win.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The broad goal here is to avoid shelling out (to clang and llvm-objcopy) in the
integration tests so that they can be compiled once on the host and executed on
various targets. This doesn't quite get us there, but lays down a bunch of
infrastructure and cleans up the integration tests along the way.
Best to review commit-by-commit.