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

Refactor lyrics tests, do not search for empty metadata #5452

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,24 @@ jobs:
poetry install --extras=replaygain --extras=reflink

- name: Install Python dependencies
run: poetry install --only=main,test --extras=autobpm
run: poetry install --only=main,test --extras=autobpm --extras=lyrics

- name: Get changed lyrics files
id: lyrics-update
uses: tj-actions/changed-files@v45
with:
files: |
beetsplug/lyrics.py
test/plugins/test_lyrics.py
Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely comfortable with this mode of testing-as-needed. These files necessarily reference many other parts of the codebase and it's always possible that they can break tests (even if they're pretty independent of the rest of the codebase right now, that can and probably will change in the future).

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently these integration tests only run once a week, and no one finds out about failures 😟


- if: ${{ env.IS_MAIN_PYTHON != 'true' }}
name: Test without coverage
run: poe test

- if: ${{ env.IS_MAIN_PYTHON == 'true' }}
name: Test with coverage
env:
LYRICS_UPDATED: ${{ steps.lyrics-update.outputs.any_changed }}
uses: liskin/gh-problem-matcher-wrap@v3
with:
linters: pytest
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ to get a basic view on how tests are written. Since we are currently migrating
the tests from `unittest`_ to `pytest`_, new tests should be written using
`pytest`_. Contributions migrating existing tests are welcome!

External API requests under test should be mocked with `requests_mock`_,
External API requests under test should be mocked with `requests-mock`_,
However, we still want to know whether external APIs are up and that they
return expected responses, therefore we test them weekly with our `integration
test`_ suite.
Expand Down
42 changes: 22 additions & 20 deletions beets/test/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

import beets
import beets.plugins
from beets import autotag, config, importer, logging, util
from beets import autotag, importer, logging, util
from beets.autotag.hooks import AlbumInfo, TrackInfo
from beets.importer import ImportSession
from beets.library import Album, Item, Library
Expand Down Expand Up @@ -153,12 +153,27 @@ def check_reflink_support(path: str) -> bool:
return reflink.supported_at(path)


class ConfigMixin:
@cached_property
def config(self) -> beets.IncludeLazyConfig:
"""Base beets configuration for tests."""
config = beets.config
config.sources = []
config.read(user=False, defaults=True)

config["plugins"] = []
config["verbose"] = 1
config["ui"]["color"] = False
config["threaded"] = False
return config


NEEDS_REFLINK = unittest.skipUnless(
check_reflink_support(gettempdir()), "no reflink support for libdir"
)


class TestHelper(_common.Assertions):
class TestHelper(_common.Assertions, ConfigMixin):
"""Helper mixin for high-level cli and plugin tests.

This mixin provides methods to isolate beets' global state provide
Expand All @@ -184,8 +199,6 @@ def setup_beets(self):
- ``libdir`` Path to a subfolder of ``temp_dir``, containing the
library's media files. Same as ``config['directory']``.

- ``config`` The global configuration used by beets.

- ``lib`` Library instance created with the settings from
``config``.

Expand All @@ -202,15 +215,6 @@ def setup_beets(self):
)
self.env_patcher.start()

self.config = beets.config
self.config.sources = []
self.config.read(user=False, defaults=True)

self.config["plugins"] = []
self.config["verbose"] = 1
self.config["ui"]["color"] = False
self.config["threaded"] = False

self.libdir = os.path.join(self.temp_dir, b"libdir")
os.mkdir(syspath(self.libdir))
self.config["directory"] = os.fsdecode(self.libdir)
Expand All @@ -229,8 +233,6 @@ def teardown_beets(self):
self.io.restore()
self.lib._close()
self.remove_temp_dir()
beets.config.clear()
beets.config._materialized = False

# Library fixtures methods

Expand Down Expand Up @@ -452,7 +454,7 @@ def setUp(self):
self.i = _common.item(self.lib)


class PluginMixin:
class PluginMixin(ConfigMixin):
plugin: ClassVar[str]
preload_plugin: ClassVar[bool] = True

Expand All @@ -473,7 +475,7 @@ def load_plugins(self, *plugins: str) -> None:
"""
# FIXME this should eventually be handled by a plugin manager
plugins = (self.plugin,) if hasattr(self, "plugin") else plugins
beets.config["plugins"] = plugins
self.config["plugins"] = plugins
beets.plugins.load_plugins(plugins)
beets.plugins.find_plugins()

Expand All @@ -494,7 +496,7 @@ def unload_plugins(self) -> None:
# FIXME this should eventually be handled by a plugin manager
for plugin_class in beets.plugins._instances:
plugin_class.listeners = None
beets.config["plugins"] = []
self.config["plugins"] = []
beets.plugins._classes = set()
beets.plugins._instances = {}
Item._types = getattr(Item, "_original_types", {})
Expand All @@ -504,7 +506,7 @@ def unload_plugins(self) -> None:

@contextmanager
def configure_plugin(self, config: Any):
beets.config[self.plugin].set(config)
self.config[self.plugin].set(config)
self.load_plugins(self.plugin)

yield
Expand Down Expand Up @@ -624,7 +626,7 @@ def _get_import_session(self, import_dir: bytes) -> ImportSession:
def setup_importer(
self, import_dir: bytes | None = None, **kwargs
) -> ImportSession:
config["import"].set_args({**self.default_import_config, **kwargs})
self.config["import"].set_args({**self.default_import_config, **kwargs})
self.importer = self._get_import_session(import_dir or self.import_dir)
return self.importer

Expand Down
Loading
Loading