fix/enable rust tests, make build faster #623
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
This commit fixes rust tests and configures ci to run them on commit. It also sets up CI to run those in a timely manner by caching dependencies and splitting jobs.
arighi
approved these changes
Sep 6, 2024
.github/workflows/caching-build.yml
Outdated
| prefix-key: "1" | ||
| - run: cargo build --manifest-path rust/Cargo.toml | ||
| - run: cargo test --manifest-path rust/Cargo.toml --no-run | ||
| - run: vng -m10G --force-9p -r linux/arch/x86/boot/bzImage --net user --exec "cargo test --manifest-path rust/Cargo.toml" |
There was a problem hiding this comment.
May I suggest to change the vng command as following:
vng -v -m10G -c8 --force-9p -r linux/arch/x86/boot/bzImage --net user -- cargo test --manifest-path rust/Cargo.toml
-vallows to catch potential OOM conditions (even if 10G should be enough to run the tests)-c8always using 8 CPUs allows to run tests in a consistent environment, independently on the CPUs on the host (I'm thinking at the Cpumask tests)--execis the old-style deprecated syntax, better to move to the new syntax
rust/scx_utils/src/cpumask.rs
Outdated
| //! let from_str_mask = cpumask::from_string(str); | ||
| //! use scx_utils::Cpumask; | ||
| //! let all_zeroes = Cpumask::new(); | ||
| //! let str = String::from("0xff00ff00"); |
There was a problem hiding this comment.
I think this one is going to fail unless we have all these CPUs inside the vng instance (hence my comment about using just 8 CPUs). So, maybe change this code chunk with:
use scx_utils::Cpumask;
let all_zeroes = Cpumask::new();
let str = String::from("0xf0");
There was a problem hiding this comment.
Ahh that makes sense, thank you!
set vng cpus to 8 for rust tests (for stability in testing), update relevant doc test.
htejun
approved these changes
Sep 6, 2024
arighi
approved these changes
Sep 6, 2024
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.
This PR fixes rust tests and configures CI to run them on commit.
It does this by configuring 3 CI jobs.
One CI job will, hourly, build and cache a new sched_ext kernel if there is a new commit on for-next. This periodic job only runs on main.
The other two CI jobs run meson tests and cargo tests. These use that cached kernel.
All branches can that cached kernel from main. not-main branches will create their own caches if one for main does not exist for for-next's current head, but these are less useful because they are not shared like main's is.
When the kernel is cached, job turnaround time (test time) looks to be ~12 minutes: https://github.com/likewhatevs/scx/actions/runs/10736459682
Not-cached job time looks like 20-25 minutes.
I'm not sure if I should replace the current configs with this config (I think it's a superset, or can be made one easily enough) or add another (as this PR does atm).
Lmk what you think, thanks!