Skip to content

Split long-running tests into more focused tests - #242

Open
bbannier wants to merge 4 commits into
masterfrom
topic/bbannier/split-tests
Open

Split long-running tests into more focused tests#242
bbannier wants to merge 4 commits into
masterfrom
topic/bbannier/split-tests

Conversation

@bbannier

Copy link
Copy Markdown
Member

Many of our tests check multiple behaviors at the same time. This not only makes the tests hard to follow and debug when something breaks (e.g., a test might stop before all behaviors are checked), but it also causes some tests to run for a very long time. This PR breaks up our longest running tests into more focused tests instead. I see no test suite runtime improvement from that on my machine, but IMO it still improves the overall situation.

Warning

All code changes in this branch were generated with Claude Sonnet 4.6. I reviewed all changes and am accountable.

@bbannier bbannier self-assigned this Jul 22, 2026
@bbannier
bbannier marked this pull request as ready for review July 22, 2026 08:13
@bbannier
bbannier requested a review from ckreibich July 22, 2026 08:13
Comment thread testing/tests/metadata-suggests-install
Comment thread testing/tests/metadata-depends-install
Comment thread testing/tests/metadata-suggests-bundle
Comment thread testing/tests/metadata-depends-bundle
@@ -0,0 +1,33 @@
# @TEST-EXEC: bash %INPUT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some comments in each "section" of this test might help clarify what each part is doing.

@bbannier bbannier Jul 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, that would help, but also tells me that I should have broken this up further. I did that now so we have dedicated tests and no need to add sections, sorry for the churn. This forces us to copy %INPUT to more tests, but I think that makes sense for standalone tests (in contrast to introducing a say tests/scripts/setup which would couple stuff again and also make it harder to follow what is going on in each test).

tests.install still installs four different packages in one tests. Not sure we need section headers there, but could certainly add them. WDYT?

bbannier added 4 commits July 24, 2026 12:06
Extract each distinct scenario from the monolithic test:
- `install-branch-default`: installs corge at the latest version on the default branch
- `install-branch-explicit`: installs corge by explicit branch name
- `install-version-latest`: installs grault at the latest version
- `install-version-tag`: installs grault at a specific tag
- `install-version-detached`: installs grault from a local path with detached HEAD
- `install-version-commit-hash`: installs grault at a specific commit hash

The remaining `tests.install` covers the four unconditional installs
(foo, bar, baz, qux) that need no setup.
The original `tests.bundle` ran snapshot, `--manifest args`, and
`--manifest file` bundle scenarios sequentially in one test. Extract
each into its own test (`tests.bundle-snapshot`, `tests.bundle-args`,
`tests.bundle-manifest`) with the tag-setup script inlined per test.
The original `tests.bundle` is removed since all scenarios are covered
by the new tests.
The original test ran four scenarios sequentially behind a single
`bash %INPUT` setup. Extract each scenario (no-suggests baseline,
upgrade via `zkg refresh`+`upgrade`, fresh install, bundle/unbundle)
into its own test file with the repo-mutation setup inlined. The
original `tests.metadata-suggests` is removed.
Mirror the same split applied to `tests.metadata-suggests`: extract the
no-depends baseline, upgrade, install, and bundle/unbundle scenarios
into separate test files with the repo-mutation setup inlined in each.
The original `tests.metadata-depends` is removed.
@bbannier
bbannier force-pushed the topic/bbannier/split-tests branch from d7c7354 to 3fc2082 Compare July 24, 2026 10:20
@bbannier
bbannier requested a review from timwoj July 24, 2026 10:23
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

Successfully merging this pull request may close these issues.

2 participants