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

Long runtimes for integration tests #845

Open
simonsan opened this issue May 9, 2023 · 2 comments
Open

Long runtimes for integration tests #845

simonsan opened this issue May 9, 2023 · 2 comments

Comments

@simonsan
Copy link
Contributor

simonsan commented May 9, 2023

https://github.com/iqlusioninc/abscissa/blob/main/cli/template/tests/acceptance.rs.hbs#L31

I built an integration tests with some stages, unfortunately the default configuration of the CmdRunner invokes cargo each time which rebuilds/checks the binary each time as well. There was a huge performance penalty because of that. When I initialized the CmdRunner with:

pub static RUNNER: Lazy<CmdRunner> = Lazy::new(|| {
    let mut runner = CmdRunner::new(env!("CARGO_BIN_EXE_RUSTIC"));
    runner.exclusive().capture_stdout();
    runner
});

the gain was huge, tests that ran 90 seconds on my device ran in 1 second.

2023-05-09 09_20_56-Environment Variables - The Cargo Book — Mozilla Firefox

src.: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

Is there any downsides about this, that I don't understand/know? If not, would it be reasonable to adopt that as a default option?

@simonsan
Copy link
Contributor Author

TL;DR: this is a worthwhile change, but I'd like to see it on the framework level (i.e. in abscissa) rather than fix up every user individually.

I've compared this against the main branch, measuring the second run of the tests (once the crates.io index cloning is done, which takes up the vast majority of the time), and it takes 50 seconds regardless of these changes.

With an up-to-date crates.io index, running cargo clean; cargo test --release --all-features --no-run and then measuring cargo test --release --all-features shows the new approach to be faster, because cargo run is invoked without --release and genuinely needs compile the debug build instead of using the release build that's already been compiled.

Originally posted by @Shnatsel in rustsec/rustsec#883 (comment)

Shall I make a PR and add a with_bin method on CmdRunner utilizing this?

@Shnatsel
Copy link
Contributor

Does abscissa know the name of the binary? If so, it should just use it internally in CmdRunner::default()

If not, we'll have to just update documentation and examples instead to explain this superior approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants