Fix tv show theme tests #1578
Fix tv show theme tests #1578
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes TV show theme tests by restoring a theme after the deleteTheme() test modifies the shared test data. The edit_theme test deletes the active theme from the show, which causes subsequent season and episode tests (which inherit themes from the show) to fail since they expect a theme to be available.
Changes:
- Added
plexparameter totest_video_Show_mixins_themesfunction - After theme deletion testing, re-uploads a metadata-provided theme to restore functionality for season/episode tests
- Uses walrus operator for concise conditional assignment
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if themes := show.themes(): | ||
| if theme := next((t for t in themes if t.ratingKey.startswith("metadata://")), None): | ||
| show.uploadTheme(url=plex.url(theme.key, includeToken=True)).unlockTheme() | ||
|
|
||
|
|
There was a problem hiding this comment.
The nested if statements will silently do nothing if no metadata themes are available after deletion. This could cause subsequent season and episode tests to fail without a clear indication of why. Consider adding error handling or logging to make failures more explicit. For example, you could assert that a metadata theme exists or raise an informative error if the restoration fails.
| if themes := show.themes(): | |
| if theme := next((t for t in themes if t.ratingKey.startswith("metadata://")), None): | |
| show.uploadTheme(url=plex.url(theme.key, includeToken=True)).unlockTheme() | |
| themes = show.themes() | |
| if not themes: | |
| pytest.fail("Failed to restore theme after edit_theme: no themes are available on the show.") | |
| theme = next((t for t in themes if t.ratingKey.startswith("metadata://")), None) | |
| if theme is None: | |
| pytest.fail( | |
| "Failed to restore theme after edit_theme: no metadata theme with 'metadata://' ratingKey found." | |
| ) | |
| show.uploadTheme(url=plex.url(theme.key, includeToken=True)).unlockTheme() |
| def test_video_Show_mixins_themes(show, plex): | ||
| test_mixins.edit_theme(show) | ||
|
|
||
| # Need to re-upload theme for future season/episode tests |
There was a problem hiding this comment.
The comment doesn't explain why the code specifically looks for themes with the "metadata://" prefix. Adding context about the difference between uploaded themes ("upload://") and metadata-provided themes ("metadata://") would improve code clarity. For example: "Re-upload a metadata-provided theme (not the uploaded test theme) for future season/episode tests".
| # Need to re-upload theme for future season/episode tests | |
| # Re-upload a metadata-provided theme (ratingKey starts with "metadata://"), | |
| # not the uploaded test theme (which would use an "upload://" prefix), | |
| # so that future season/episode tests have a stable metadata-based theme. |
| if themes := show.themes(): | ||
| if theme := next((t for t in themes if t.ratingKey.startswith("metadata://")), None): |
There was a problem hiding this comment.
The walrus operator is not used elsewhere in the codebase. While this is valid Python 3.8+ syntax and the code is correct, it introduces a new pattern. Consider using the more traditional pattern for consistency: assign to variables first, then check in separate if statements. This would match the style used throughout the rest of the codebase.
1 participant
Description
TV show theme is deleted by
deleteTheme()in another test. The theme is re-uploaded after the delete test so it is available in subsequent season/episode tests.Type of change
Please delete options that are not relevant.
Checklist: