Skip to content

Commit

Permalink
Speed up UI tests (#342)
Browse files Browse the repository at this point in the history
We currently have ~140 different test projects, with [the fastest
platform taking almost 10 minutes to run the whole UI test
suite](https://github.com/LukeMathWalker/pavex/actions/runs/10775293351/job/29887140633).
That's not sustainable.
This PR significantly restructures `pavex_test_runner`, the custom test
harness used to write UI tests for Pavex.

The main obstacle is `cargo`'s locking system—each test needs to compile
some crates (the generated app, sometimes an integration test) as well
as generate some JSON documentation via `rustdoc`. All these activities
compete for the lock.

This PR rearchitects `pavex_test_runner` to minimise lock contention and
maximise core usage.
In particular:
- All JSON docs are generated upfront, in batch, ensuring that each test
hits the SQLite cache rather than invoking `cargo` directly, thus avoid
contention;
- All UI tests now live in the same (separate) workspace, under
`ui_tests`. They share a single target folder and their features are
unified via a dedicated workspace hack crate. This ensures that we get
full cache re-use across tests.
- All compilation jobs (e.g. generated apps or integration tests) are
batched to maximise core utilisation

The speed-up has been substantial. In CI, we now run UI tests on Linux
in
[~7m](https://github.com/LukeMathWalker/pavex/actions/runs/11052805801/job/30706076683)
compared to the
[~11m](https://github.com/LukeMathWalker/pavex/actions/runs/10775293351/job/29887139838)
we were seeing on `main`. The difference is even more extreme on
Windows: from
[16m](https://github.com/LukeMathWalker/pavex/actions/runs/10775293351/job/29887141009)
to
[9m](https://github.com/LukeMathWalker/pavex/actions/runs/11052805801/job/30706078013).
We also scored further wins there, by carving out some particularly slow
tests, saving 5 minutes on executing unit tests.

Some leftover work to be done in followup PRs:

- Fix all warnings in UI test code
- Ensure that all `test_config.toml` files are only using valid keys
- Enforce programmatically the crate naming structure
(`{prefix}_{hash}`) to avoid divergence over time
- Add utilities to generate the scaffolding for a new test
  • Loading branch information
LukeMathWalker committed Sep 26, 2024
1 parent 4565811 commit c6f10c5
Show file tree
Hide file tree
Showing 932 changed files with 12,808 additions and 1,165 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/libs/ui_test_envs
/libs/ui_tests/target
/libs/ui_tests/**/generated_app/
/libs/ui_tests/Cargo.lock
/libs/target
/libs/vendor
/doc_examples/tutorial_envs/
Expand Down
116 changes: 51 additions & 65 deletions libs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions libs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ biscotti = "0.3"
bytes = "1.5.0"
camino = "1"
cargo-like-utils = "0.1.2"
cargo_metadata = "0.18.0"
cargo-manifest = "0.14.0"
clap = "4"
clap-stdin = "0.4.0"
Expand Down
13 changes: 3 additions & 10 deletions libs/pavex_cli/tests/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,10 @@ use std::str::FromStr;

fn main() -> Result<(), Box<dyn std::error::Error>> {
let manifest_dir = PathBuf::from_str(env!("CARGO_MANIFEST_DIR")).unwrap();
let test_data_folder = manifest_dir.join("tests").join("ui_tests");
let test_runtime_folder = manifest_dir.parent().unwrap().join("ui_test_envs");
let pavex_cli_path = get_pavex_cli_path()?;
let test_runtime_folder = manifest_dir.parent().unwrap().join("ui_tests");
let pavexc_cli_path = get_pavexc_cli_path()?;
run_tests(
pavex_cli_path,
pavexc_cli_path,
test_data_folder,
test_runtime_folder,
)?
.exit();
let pavex_cli_path = get_pavex_cli_path()?;
run_tests(pavex_cli_path, pavexc_cli_path, test_runtime_folder)?.exit();
}

fn get_pavex_cli_path() -> Result<PathBuf, Box<dyn std::error::Error>> {
Expand Down
Loading

0 comments on commit c6f10c5

Please sign in to comment.