Skip to content

refactor: Removed ImportTestCase in favor of ImportHelper#6680

Open
semohr wants to merge 11 commits into
masterfrom
pytest_import_helper
Open

refactor: Removed ImportTestCase in favor of ImportHelper#6680
semohr wants to merge 11 commits into
masterfrom
pytest_import_helper

Conversation

@semohr
Copy link
Copy Markdown
Contributor

@semohr semohr commented May 29, 2026

Description

This PR removes ImportTestCase and replaces all occurrences with ImportHelper + pytest.

This touches:

  • test_scrub
  • test_filefilter
  • test_replaygain
  • test_chroma
  • test_import

TODOs:

  • Changelog Not needed as this is an internal refactor only

This is related to the multi-step efforts to improve logging in beets #6553

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.45%. Comparing base (d179dbb) to head (d4d1fcb).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6680      +/-   ##
==========================================
- Coverage   75.57%   74.45%   -1.12%     
==========================================
  Files         162      162              
  Lines       20806    20806              
  Branches     3292     3292              
==========================================
- Hits        15725    15492     -233     
- Misses       4306     4563     +257     
+ Partials      775      751      -24     

see 2 files 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.

@semohr semohr changed the title refactor: Deprecated ImportTestCase in favor of PytestImportHelper refactor: Deprecated ImportTestCase in favor of ImportHelper Jun 3, 2026
@semohr semohr force-pushed the pytest_import_helper branch from b50ca40 to 27604e3 Compare June 3, 2026 20:02
@semohr semohr changed the title refactor: Deprecated ImportTestCase in favor of ImportHelper refactor: Removed ImportTestCase in favor of ImportHelper Jun 3, 2026
@semohr semohr force-pushed the pytest_import_helper branch 3 times, most recently from 043c48b to 9c8a977 Compare June 3, 2026 20:59
@semohr semohr mentioned this pull request Jun 3, 2026
@semohr semohr force-pushed the pytest_import_helper branch from 9c8a977 to 0420bc0 Compare June 4, 2026 08:44
@semohr semohr marked this pull request as ready for review June 4, 2026 08:44
@semohr semohr requested a review from a team as a code owner June 4, 2026 08:44
Copilot AI review requested due to automatic review settings June 4, 2026 08:44
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

grug see PR want remove ImportTestCase and move tests to ImportHelper + pytest style. this fit test harness cleanup work and help future logging overhaul by making importer tests more uniform.

Changes:

  • delete ImportTestCase alias and update helper inheritance to use ImportHelper directly
  • refactor many plugin/import tests from unittest patterns (setUp, skipTest, decorators) to pytest patterns (setup_beets, pytest.skip, pytest.mark.skipif)
  • rename a bunch of test classes to Test* names to match pytest discovery

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
beets/test/helper.py remove ImportTestCase alias; keep importer helpers centered on ImportHelper
test/ui/test_ui_importer.py update terminal importer tests to inherit from renamed importer test class
test/test_logging.py convert logging importer tests to ImportHelper and pytest lifecycle hooks
test/test_importer.py replace ImportTestCase usage, convert skips to pytest, rename tests
test/plugins/test_scrub.py convert scrub plugin import test to ImportHelper
test/plugins/test_filefilter.py convert filefilter tests to ImportHelper and pytest setup
test/plugins/test_replaygain.py convert replaygain tests off unittest patterns, use pytest skip/marks
test/plugins/test_permissions.py convert permissions tests to ImportHelper and pytest skip
test/plugins/test_keyfinder.py convert keyfinder test to ImportHelper
test/plugins/test_chroma.py convert chroma test to ImportHelper
Comments suppressed due to low confidence (1)

test/test_logging.py:243

  • grug see docstring still say old class name LoggingLevelTest. class renamed to TestLoggingLevel, so docstring now lie.
class TestConcurrentEvents(AsIsImporterMixin, ImportHelper):
    """Similar to LoggingLevelTest but lower-level and focused on multiple
    events interaction. Since this is a bit heavy we don't do it in
    LoggingLevelTest.
    """

Comment thread test/test_importer.py Outdated
Comment thread test/ui/test_ui_importer.py Outdated
Comment thread test/test_importer.py
Comment thread test/test_importer.py
Comment thread test/test_importer.py Outdated
Comment thread test/test_logging.py Outdated
Comment thread test/ui/test_ui_importer.py Outdated
@semohr semohr force-pushed the pytest_import_helper branch from 0420bc0 to 4ee7102 Compare June 6, 2026 09:27
@semohr semohr force-pushed the pytest_import_helper branch from 4ee7102 to d4d1fcb Compare June 6, 2026 09:54
@semohr semohr requested a review from snejus June 6, 2026 09:55
Comment thread test/test_logging.py
super().setUpClass()
@pytest.fixture(autouse=True, scope="class")
def _patch_dummy_module(self):
with patch.dict(sys.modules, {"beetsplug.dummy": DummyModule()}):
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.

Use monkeypatch

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.

Or is this not possible using monkeypatch?

Copy link
Copy Markdown
Contributor Author

@semohr semohr Jun 6, 2026

Choose a reason for hiding this comment

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

It seems not possible. monkeypatch does not work on class scope

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This works but needs function scope instead of class scope.

@pytest.fixture(autouse=True)
def _patch_dummy_module(self, monkeypatch):
    monkeypatch.setitem(sys.modules, "beetsplug.dummy", DummyModule())

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.

Yeah I think this is cleaner!

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