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

Consistify and cleanup test-suite setup macros #3535

Merged
merged 4 commits into from
Jan 27, 2025

Conversation

pmatilai
Copy link
Member

More details in commits but addressing various items from #2773:

  • rename RPMTEST_SETUP to RPMTEST_INIT to free the name
  • introduce RPMTEST_SETUP as a wrapper to AT_SETUP and use it everywhere, to pair with RPMTEST_CLEANUP and gives a central place to control rpm-specifics of test setup
  • remove now redundant rpmdb reinitializations from the test by making rpmdb reset explicitly called when absolutely needed

Besides making for more consistent names and allowing future consolidation, gives a nice speed boost (from 2m 50s to 2m 20s on my laptop)

No functional changes, we just want to use the RPMTEST_SETUP name for
other things and this is necessary to clear the route.
This gives us a nice place to control the test execution centrally,
and also looks more consistent since we already have RPMTEST_CLEANUP
as a wrapper for AT_CLEANUP.

No functional changes though.
No functional change but this will let us differentiate from the
old meaning of RPMDB_INIT.
Many of our tests require RPMTEST_INIT to create a writable snapshot,
but especially after c17cc69,
relatively few require initializing or resetting the rpmdb in the image.

Separate the two and explicitly call RPMDB_RESET (renamed from RPMDB_INIT
for a clean break) where it is actually needed. The common pattern that
emerges is: build several packages at once and then do various things with
it. Each of these RPMDB_RESET uses is actually somewhat problematic because
whenever it's required there's a chance that previous tests could affect
the results. Can't help that just now, but at least we've identified the
issue.

Besides making the needs and usages clearer, this introduces a nice fat
speed boost for the test-suite - on my laptop, from 2m 50s to 2m 20s.
@pmatilai pmatilai requested a review from dmnks January 24, 2025 09:34
@pmatilai pmatilai requested a review from a team as a code owner January 24, 2025 09:34
@pmatilai pmatilai requested review from ffesti and removed request for a team January 24, 2025 09:34
@ffesti
Copy link
Contributor

ffesti commented Jan 24, 2025

Looks good to me.

@dmnks
Copy link
Contributor

dmnks commented Jan 27, 2025

Nice, this indeed makes the tests needing to reset the rpmdb stand out more so that we can fix them in the future. Thanks!

@dmnks dmnks merged commit 1a99ef7 into rpm-software-management:master Jan 27, 2025
1 check passed
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.

3 participants