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

feat(tests): faster smokes via process-compose #4324

Merged
merged 1 commit into from
May 6, 2024
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented May 6, 2024

Overview

A while back we ditched using containers for the smoke tests, mostly because the caching on the container-build story was atrocious, so test re-runs took a really long time. And frankly, container ergonomics on dev workstations, particularly macOS, are not awesome. Instead, let's assume the dev env can run processes for cargo, pd, and cometbft. If so, that's all we need to wire up our integration testing. Enter process-compose.

Implementation

The new smoke test setup ditches the bash script and delegates to process-compose for orchestrating processes:

  1. pre-building binaries and test artifacts
  2. testnet config generation
  3. running pd and cometbft
  4. running cargo test for each binary's integration tests
  5. reporting output

The wrapper script uses tty detection, so that running the smoke tests locally on your workstation displays a pleasant TUI:

smoke-tui-1

Logging is currently a bit too aggregated, would be nice to have discrete info about what failed.

When running in CI, however, log output will be displayed:

smoke-test-log-output-ci

Benchmarks

tl;dr running the smoke tests in CI now takes about ~10m, down from ~20m as in this recent run. With a warm cache locally, run times are as low as ~3m30s, although the CI caching setup isn't as complete as local context.

Testing the old script against the new invocation locally via hyperfine, results were:

hyperfine results
hyperfine --runs 3 --prepare 'cargo run -q --bin pd -- testnet unsafe-reset-all || true' -n new-smoke ./deployments/scripts/smoke-test.sh -n legacy-s
moke ./deployments/scripts/smoke-test-legacy.sh
Benchmark 1: new-smoke
  Time (mean ± σ):     191.469 s ±  1.009 s    [User: 421.365 s, System: 25.992 s]
  Range (min … max):   190.442 s … 192.459 s    3 runs

Benchmark 2: legacy-smoke
  Time (mean ± σ):     459.222 s ± 11.546 s    [User: 422.412 s, System: 22.178 s]
  Range (min … max):   445.889 s … 465.935 s    3 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It
 might help to use the '--warmup' or '--prepare' options.

Summary
  'new-smoke' ran
    2.40 ± 0.06 times faster than 'legacy-smoke'

Actual run times on this PR:

  1. 10m22s
  2. 8m18s
  3. 10m50s

Containers are out, process orchestration is in.

A while back we ditched using containers for the smoke tests,
mostly because the caching on the container-build story was
atrocious, so test re-runs took a really long time. And frankly,
container ergonomics on dev workstations, particularly macOS,
are not awesome. Instead, let's assume the dev env can run
processes for cargo, pd, and cometbft. If so, that's all
we need to wire up our integration testing. Enter process-compose [0].

The new smoke test setup ditches the bash script and delegates
to process-compose for orchestrating processes.

Benchmarking via hyperfine shows a decrease of over 2x in runtime.

There's one substantive change to the integration test logic,
in the pcli suite, that reduces the sleep time between tests,
refining it to be more precisely the duration necessary for
claiming an undelegation.

[0] https://github.com/F1bonacc1/process-compose
@conorsch
Copy link
Contributor Author

conorsch commented May 6, 2024

With a warm cache locally, run times are as low as ~3m30s, although the CI caching setup isn't as complete as local context.

Rerunning locally, I get super fast builds. I'm not going to drill into the caching story in this PR, because that's a bit of a rabbit hole, and a 2x speed-up is a big enough win that I want to merge this. It's worth exploring caching improvements because we'll likely have a lot of caching needs for #4323, too.

@conorsch
Copy link
Contributor Author

conorsch commented May 6, 2024

I'm just going to say explicitly that this PR is dedicated to @TalDerei, whose brave sacrifices during #4081 will be remembered. I tried to build the kind of setup that would avoid a lot of the friction you experienced when wrestling with the smoke tests locally.

@TalDerei
Copy link
Collaborator

TalDerei commented May 6, 2024

This is awesome! I was wondering what makes process-compose a more friendly container orchestration tool than say standard docker-compose or kubernetes for our purposes? I ask because I haven't heard of it before! Also what's the scope of the information stored in the local hot cache?

@conorsch
Copy link
Contributor Author

conorsch commented May 6, 2024

process-compose a more friendly container orchestration tool than say standard docker-compose or kubernetes?

Simply because running containers locally has been a bit of a bear in the past. A prior version of the smoke test setup used docker-compose, and it was not pleasant to work with. We've since updated the container build story enough that it's worth revisiting, particularly in the context of upgrade testing, but this is a "good enough" solution for an immediate win, and leaves the door open to more robust devenvs via nix/devbox in the future.

Also what's the scope of the information stored in the local hot cache?

The target/ directory for rust build artifacts. On multiple CI runs, I still see some of the test artifacts being re-compiled, which indicates the caching is not perfect. It's still pretty darn good, though.

Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like an awesome extension of our smoke tests, great work @conorsch!

crates/bin/pcli/tests/network_integration.rs Show resolved Hide resolved
@TalDerei
Copy link
Collaborator

TalDerei commented May 6, 2024

I really appreciate this work-stream for speeding up dev efficiency!

@TalDerei TalDerei added A-CI/CD Relates to continuous integration & deployment of Penumbra C-chore Codebase maintenance that doesn't fix bugs or add features, and isn't urgent or blocking. labels May 6, 2024
@conorsch conorsch merged commit 696ad22 into main May 6, 2024
13 checks passed
@conorsch conorsch deleted the faster-smokes branch May 6, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI/CD Relates to continuous integration & deployment of Penumbra C-chore Codebase maintenance that doesn't fix bugs or add features, and isn't urgent or blocking.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants