Skip to content

Test speedup: cache config.sources in ConfigMixin#6772

Open
snejus wants to merge 2 commits into
masterfrom
speed-up-tests
Open

Test speedup: cache config.sources in ConfigMixin#6772
snejus wants to merge 2 commits into
masterfrom
speed-up-tests

Conversation

@snejus

@snejus snejus commented Jun 24, 2026

Copy link
Copy Markdown
Member
  • Moves the expensive config setup into ConfigMixin, which now caches a default config.sources baseline once and rebuilds isolated test config from that baseline for each test instead of re-reading full config every time.
  • Changes the shared test config flow so the config fixture is now function-scoped, giving each test and parametrized case a fresh config by default.
  • Updates affected tests such as test/autotag/test_distance.py, test/plugins/test_lastgenre.py, and test/plugins/test_lyrics.py to use the new isolated flow, since per-test config setup is no longer too expensive.

High-level impact

  • Improves test isolation by preventing shared config state from leaking across tests.
  • Makes the suite easier to reason about because config lifecycle is handled in one place.
  • Speeds up tests up to 2x by avoiding repeated full config reads while keeping fresh per-test state.

See two test modules, for example, where ConfigMixin is used extensively:

Before

$ pytest test/test_library.py
...
test/test_library.py ...............ss........................................................................................... [148/174]
..........................                                                                                                        [174/174]
...
================================= 172 passed, 2 skipped in 10.42s ==========================================================================
$ pytest test/test_importer.py
...
test/test_importer.py ..s.ss.........................................s.......s.........................................           [137/137]
...
================================= 129 passed, 8 skipped in 12.90s ==========================================================================

After

$ pytest test/test_library.py
...
test/test_library.py ...............ss........................................................................................... [148/174]
..........................                                                                                                        [174/174]
...
================================== 172 passed, 2 skipped in 4.98s ==========================================================================
$ pytest test/test_importer.py
...
test/test_importer.py ..s.ss.........................................s.......s.........................................           [137/137]
...
================================== 129 passed, 8 skipped in 9.27s ==========================================================================

@snejus snejus requested a review from a team as a code owner June 24, 2026 12:29
Copilot AI review requested due to automatic review settings June 24, 2026 12:29
@github-actions

Copy link
Copy Markdown

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

PR try make tests faster and more isolated by building fresh beets.config per test without paying full config.read() cost each time. grug like less time waste, but current ConfigMixin.config reset look wrong: it can grow config.sources every test and poke private confuse cache with wrong type, so suite may get flaky / slow over time.

Changes:

  • Cache a “baseline” config.sources once in ConfigMixin, and rebuild test config from that baseline.
  • Switch config fixture to function scope (fresh config per test case / param set).
  • Update a few tests to match new per-test config lifecycle.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
beets/test/helper.py Add cached baseline for config sources; rebuild per-test config from baseline.
test/conftest.py Make config fixture function-scoped.
test/autotag/test_distance.py Adjust fixture scope to match function-scoped config.
test/plugins/test_lastgenre.py Remove now-unneeded “fresh config per param” shim fixture.
test/plugins/test_lyrics.py Update test to use plugin config context + per-test state.

Comment thread beets/test/helper.py
Comment thread beets/test/helper.py
Comment thread test/conftest.py
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.98%. Comparing base (c90f42d) to head (76113c1).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6772      +/-   ##
==========================================
+ Coverage   74.78%   74.98%   +0.19%     
==========================================
  Files         163      163              
  Lines       20966    20966              
  Branches     3302     3302              
==========================================
+ Hits        15680    15721      +41     
+ Misses       4529     4486      -43     
- Partials      757      759       +2     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@snejus snejus force-pushed the speed-up-tests branch 3 times, most recently from 8d9429c to a2ce444 Compare June 24, 2026 13:26
@snejus snejus requested a review from Copilot June 24, 2026 13:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@snejus snejus force-pushed the speed-up-tests branch 4 times, most recently from ad8438d to 163f9ac Compare June 25, 2026 00:57
@semohr

semohr commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

This feels a little like we're working around pytest's intended fixture model. Would it make sense to use a session-scoped config as the canonical instance and have function-scoped configs operate on copies of it? That would achieve essentially the same behavior as the current approach while staying within pytest's fixture-scoping semantics.

@snejus

snejus commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Would it make sense to use a session-scoped config as the canonical instance and have function-scoped configs operate on copies of it?

It does, however this is out of scope of this PR.

I actually tried doing this and found that we have the same issue that we had with TerminalImportMixin. On the other hand, since self.config is used within setup_beets, we don't have much other choice but to continue using config attribute:

    @cached_property
    def config(self) -> beets.IncludeLazyConfig:
        return self.request.getfixturevalue("config")

@snejus snejus changed the base branch from master to do-not-backup-db-in-tests June 27, 2026 02:45
@snejus snejus force-pushed the do-not-backup-db-in-tests branch from 5ded4f0 to 97b2430 Compare June 27, 2026 08:32
Base automatically changed from do-not-backup-db-in-tests to master June 27, 2026 11:16
@snejus snejus force-pushed the speed-up-tests branch 3 times, most recently from 63b72cc to 38c290a Compare June 30, 2026 12:01
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