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

Migrate tests from unittest to pytest #5361

Open
snejus opened this issue Jul 12, 2024 · 1 comment
Open

Migrate tests from unittest to pytest #5361

snejus opened this issue Jul 12, 2024 · 1 comment
Labels
big refactor testing Relates to the testing/CI infrastructure

Comments

@snejus
Copy link
Member

snejus commented Jul 12, 2024

Following our discussion under #5229, I am adding this issue to track the progress.

Proposed solution

Make our tests use pytest instead of unittest.

Objective

This updates our testing codebase to be based on the currently accepted best practice - pytest.

Goals

Non-goals

Anti-goals

@snejus snejus added big refactor testing Relates to the testing/CI infrastructure labels Jul 12, 2024
@bal-e
Copy link
Member

bal-e commented Jul 18, 2024

I'd like to take on this, though most of it will have to wait until #5362, #5363, and #5229 are resolved. Once #5362 is complete, I'm happy to start refactoring some of the smaller test modules in small PR batches.

snejus added a commit that referenced this issue Jul 18, 2024
Fixes #5229, is part of #5361 and relates to #5285.

I have to admit thsi was a fairly tough task - I initially assumed that
the problem lies
with how the tests are setup, and that we're probably missing some
`teardown_beets` calls
here and there.

Unfortunately, it was not so simple. I came across several issues that
gave rise to
leftover temporary files:

1. `fetchart`, `artresizer` and `play` handling of temporary files.
These plugins created
isolated temporary files outside of the directories that tests clean up.
You will find
I added a couple of functions (namely `get_module_tempdir`) that force
these plugins to
create files in directories determined by their module names. This way
we can clean up
   after them using the new `CleanupModulesMixin`.

2. Tests that ran temporary directories setup twice, running
`_common.TestCase.setUp` and
`test.helper.TestHelper.setup_beets`. Both of these ran `self.temp_dir =
mkdtemp()`,
therefore the directories created by the initial setup persisted since
those have been
overridden and thus unreachable in the teardown. Here, I removed the
`setUp` calls, see

   - `test/plugins/test_embedart.py`
   - `test/test_importer.py`
   - and `test/test_plugins.py` where `setup_beets` was called twice

3. `test/test_config_command.py` attempted to manage the temporary
directory by itself,
where I found that `tearDown` failed to remove the directory for four
tests. Could not
figure out the cause, and found that delegating this task to
`TestHelper` fixed the
   issue.

4. Mediafile fixture removal depended on calling
`remove_mediafile_fixtures` method, which
`test/plugins/test_zero.py` failed to do. I made the fixtures to be
created within the
same `temp_dir` directory that gets removed in the teardown, so now they
are taken care
   of automatically.

In summary, see the test modules that left files behind:

```
Temp files created by test/__init__.py
Temp files created by test/plugins/__init__.py
Temp files created by test/plugins/lyrics_download_samples.py
Temp files created by test/plugins/test_acousticbrainz.py
Temp files created by test/plugins/test_advancedrewrite.py
Temp files created by test/plugins/test_albumtypes.py
Temp files created by test/plugins/test_art.py
	/tmp/tmp11nicahe.jpg
	/tmp/tmp1bjmodum.png
	/tmp/tmped7nhls4.jpg
	/tmp/tmpflnzr9wz.jpg
	/tmp/tmpjngkauqs.png
	/tmp/tmpkzy9mn6t.jpg
	/tmp/tmpph_wmuea.jpg
	/tmp/tmps6gk58i_.jpg
	/tmp/tmpz2eji_o4.jpg
Temp files created by test/plugins/test_aura.py
Temp files created by test/plugins/test_bareasc.py
	/tmp/tmphl3kzhug
	/tmp/tmpnh2q6v02
	/tmp/tmpppw5qrhz
Temp files created by test/plugins/test_beatport.py
Temp files created by test/plugins/test_bucket.py
Temp files created by test/plugins/test_convert.py
Temp files created by test/plugins/test_discogs.py
Temp files created by test/plugins/test_edit.py
Temp files created by test/plugins/test_embedart.py
	/tmp/tmp1ayvqzhx
	/tmp/tmp58k6mdfx.jpg
	/tmp/tmp64c2lqiv
	/tmp/tmp6nar4kr5
	/tmp/tmp6u0d5dex
	/tmp/tmpacoq7w_f
	/tmp/tmpajnr_sxr
	/tmp/tmpasj16beh
	/tmp/tmpboyaixb5
	/tmp/tmpcrmcyt5r
	/tmp/tmpdomje5g3
	/tmp/tmplu3o6t6g
	/tmp/tmpns_xvkns
	/tmp/tmpo87o1h6o.jpg
	/tmp/tmpqem39h_j
	/tmp/tmprlzm18pb
	/tmp/tmpt22v4u6x
	/tmp/tmptp3rxdgv
Temp files created by test/plugins/test_embyupdate.py
Temp files created by test/plugins/test_export.py
Temp files created by test/plugins/test_fetchart.py
Temp files created by test/plugins/test_filefilter.py
Temp files created by test/plugins/test_ftintitle.py
Temp files created by test/plugins/test_hook.py
Temp files created by test/plugins/test_ihate.py
Temp files created by test/plugins/test_importadded.py
Temp files created by test/plugins/test_importfeeds.py
Temp files created by test/plugins/test_info.py
Temp files created by test/plugins/test_ipfs.py
Temp files created by test/plugins/test_keyfinder.py
Temp files created by test/plugins/test_lastgenre.py
Temp files created by test/plugins/test_limit.py
Temp files created by test/plugins/test_lyrics.py
Temp files created by test/plugins/test_mbsubmit.py
Temp files created by test/plugins/test_mbsync.py
Temp files created by test/plugins/test_mpdstats.py
Temp files created by test/plugins/test_parentwork.py
Temp files created by test/plugins/test_permissions.py
Temp files created by test/plugins/test_player.py
Temp files created by test/plugins/test_playlist.py
Temp files created by test/plugins/test_play.py
	/tmp/tmp6ohknmve.m3u
	/tmp/tmp8rw2z_j4.m3u
	/tmp/tmp9vi27ypx.m3u
	/tmp/tmpa_s66jh8.m3u
	/tmp/tmpb7h3cn3n.m3u
	/tmp/tmpexbmqvry.m3u
	/tmp/tmpinbqrt80.m3u
	/tmp/tmpql02hax5.m3u
	/tmp/tmpvbdzprsf.m3u
	/tmp/tmpzipim36x.m3u
Temp files created by test/plugins/test_plexupdate.py
Temp files created by test/plugins/test_plugin_mediafield.py
Temp files created by test/plugins/test_random.py
Temp files created by test/plugins/test_replaygain.py
Temp files created by test/plugins/test_smartplaylist.py
Temp files created by test/plugins/test_spotify.py
Temp files created by test/plugins/test_subsonicupdate.py
Temp files created by test/plugins/test_the.py
Temp files created by test/plugins/test_thumbnails.py
Temp files created by test/plugins/test_types_plugin.py
Temp files created by test/plugins/test_web.py
Temp files created by test/plugins/test_zero.py
	/tmp/tmp3ub9xmzy
Temp files created by test/rsrc/beetsplug/test.py
Temp files created by test/rsrc/convert_stub.py
Temp files created by test/testall.py
Temp files created by test/test_art_resize.py
	/tmp/tmp3p7p60ih.jpg
	/tmp/tmp8exclgit.jpg
	/tmp/tmpkrrjsitl.jpg
	/tmp/tmpw6n8ee8e.jpg
	/tmp/tmpygws_0aw.jpg
Temp files created by test/test_autotag.py
Temp files created by test/test_config_command.py
	/tmp/tmp333f0r2j
	/tmp/tmphr356z5r
	/tmp/tmporp4rag2
	/tmp/tmpy7sjqdsw
Temp files created by test/test_datequery.py
Temp files created by test/test_dbcore.py
Temp files created by test/test_files.py
Temp files created by test/test_hidden.py
Temp files created by test/test_importer.py
	/tmp/tmp0m363gfb
	/tmp/tmp2n3i13mc
	/tmp/tmpxk3v304s
Temp files created by test/test_library.py
Temp files created by test/test_logging.py
Temp files created by test/test_m3ufile.py
Temp files created by test/test_mb.py
Temp files created by test/test_metasync.py
Temp files created by test/test_pipeline.py
Temp files created by test/test_plugins.py
	/tmp/tmp6pxhx67u
	/tmp/tmpb8pqi9ui
	/tmp/tmpcx_658g7
	/tmp/tmp_giqb9jz
	/tmp/tmpgm9xk94_
	/tmp/tmpk60l6bt3
	/tmp/tmpqoj4la68
	/tmp/tmptcdu20rp
	/tmp/tmpvr7k5shn
	/tmp/tmpwnfnzs91
Temp files created by test/test_query.py
Temp files created by test/test_sort.py
Temp files created by test/test_template.py
Temp files created by test/test_ui_commands.py
	/tmp/tmpns2u94w6
Temp files created by test/test_ui_importer.py
Temp files created by test/test_ui_init.py
Temp files created by test/test_ui.py
Temp files created by test/test_util.py
Temp files created by test/test_vfs.py
```

And that's what we have right now:

```
Temp files created by test/__init__.py
Temp files created by test/plugins/__init__.py
Temp files created by test/plugins/lyrics_download_samples.py
Temp files created by test/plugins/test_acousticbrainz.py
Temp files created by test/plugins/test_advancedrewrite.py
Temp files created by test/plugins/test_albumtypes.py
Temp files created by test/plugins/test_art.py
Temp files created by test/plugins/test_aura.py
Temp files created by test/plugins/test_bareasc.py
Temp files created by test/plugins/test_beatport.py
Temp files created by test/plugins/test_bucket.py
Temp files created by test/plugins/test_convert.py
Temp files created by test/plugins/test_discogs.py
Temp files created by test/plugins/test_edit.py
Temp files created by test/plugins/test_embedart.py
Temp files created by test/plugins/test_embyupdate.py
Temp files created by test/plugins/test_export.py
Temp files created by test/plugins/test_fetchart.py
Temp files created by test/plugins/test_filefilter.py
Temp files created by test/plugins/test_ftintitle.py
Temp files created by test/plugins/test_hook.py
Temp files created by test/plugins/test_ihate.py
Temp files created by test/plugins/test_importadded.py
Temp files created by test/plugins/test_importfeeds.py
Temp files created by test/plugins/test_info.py
Temp files created by test/plugins/test_ipfs.py
Temp files created by test/plugins/test_keyfinder.py
Temp files created by test/plugins/test_lastgenre.py
Temp files created by test/plugins/test_limit.py
Temp files created by test/plugins/test_lyrics.py
Temp files created by test/plugins/test_mbsubmit.py
Temp files created by test/plugins/test_mbsync.py
Temp files created by test/plugins/test_mpdstats.py
Temp files created by test/plugins/test_parentwork.py
Temp files created by test/plugins/test_permissions.py
Temp files created by test/plugins/test_player.py
Temp files created by test/plugins/test_playlist.py
Temp files created by test/plugins/test_play.py
Temp files created by test/plugins/test_plexupdate.py
Temp files created by test/plugins/test_plugin_mediafield.py
Temp files created by test/plugins/test_random.py
Temp files created by test/plugins/test_replaygain.py
Temp files created by test/plugins/test_smartplaylist.py
Temp files created by test/plugins/test_spotify.py
Temp files created by test/plugins/test_subsonicupdate.py
Temp files created by test/plugins/test_the.py
Temp files created by test/plugins/test_thumbnails.py
Temp files created by test/plugins/test_types_plugin.py
Temp files created by test/plugins/test_web.py
Temp files created by test/plugins/test_zero.py
Temp files created by test/rsrc/beetsplug/test.py
Temp files created by test/rsrc/convert_stub.py
Temp files created by test/testall.py
Temp files created by test/test_art_resize.py
Temp files created by test/test_autotag.py
Temp files created by test/test_config_command.py
Temp files created by test/test_datequery.py
Temp files created by test/test_dbcore.py
Temp files created by test/test_files.py
Temp files created by test/test_hidden.py
Temp files created by test/test_importer.py
Temp files created by test/test_library.py
Temp files created by test/test_logging.py
Temp files created by test/test_m3ufile.py
Temp files created by test/test_mb.py
Temp files created by test/test_metasync.py
Temp files created by test/test_pipeline.py
Temp files created by test/test_plugins.py
Temp files created by test/test_query.py
Temp files created by test/test_sort.py
Temp files created by test/test_template.py
Temp files created by test/test_ui_commands.py
Temp files created by test/test_ui_importer.py
Temp files created by test/test_ui_init.py
Temp files created by test/test_ui.py
Temp files created by test/test_util.py
Temp files created by test/test_vfs.py
```

Note that the command which provides the output is now available through
`poe`.
snejus added a commit that referenced this issue Jul 31, 2024
This PR deduplicates shared tests setup that was used across test files.
It attempts to centralise the setup into a single source of truth
implementation in `beets.test.helper`.

This works towards the eventual migration to `pytest` (#5361) making it
easier to replace the tests setup with `pytest` fixtures.

It builds upon the temporary files cleanup in #5345.

## Details

### Test setup

* Mostly duplicate test setup implementations in
`beets.test._common.TestCase` and `beets.test.helper.TestHelper` were
merged into a single implementation as
`beets.test.helper.BeetsTestCase`.
* Replaced `unittest.TestCase` and `beets.test.helper.TestHelper`
combination with `beets.test.helper.ImportTestCase`
* Refactored `test/test_plugins.py` removing duplicate import files
setup.
* Centralised setting up the database on disk.
* Centralised plugin test setup into `beets.test.helpers.PluginMixin`.

### Removals

* Around 1/3 of the removed lines correspond to the removal of `def
suite()` functions that were previously used for tests collection.
* Removed redundant `setUp` and `tearDown` methods from many test files
given that the functions that they performed had been centralised in
`beets.test.helper`.
* Removed redundant library initialisations


### Importing

* Centralised import directory creation (only gets created on the first
access).
* Deduplicated `_setup_import_session` implementations in
`TerminalImportSessionSetup` and `ImportHelper`.
* Merged `ImportHelper._create_import_dir` and
`TestHelper.create_importer` implementations into
`ImportHelper.setup_importer`.
* Merged various takes on import files prep into
`ImportHelper.prepare_albums_for_import` and
`ImportHelper.prepare_album_for_import`.
* Seeing that many tests used it, defined `AsIsImporterMixin` which
provides a method `run_asis_importer` to setup _asis_ (non-autotag)
importer and run it
snejus added a commit that referenced this issue Aug 19, 2024
... continuing the test suite migration to `pytest` (#5361), this PR
replaces `unittest` method assertions with `assert` statement.

See the table below for a list of the replaced methods and their counts.
This should help clarify the size of the pull request 😆.

To keep things simple for the review, I have replaced each method in its
own commit. This
way, things should be easy to follow if you step through the commits one
by one. 🙂

method                   | count
---                      | ---
**`assertEqual`**        | 1747
**`assertIn`**           | 200
**`assertTrue`**         | 149
**`assertIsNone`**       | 87
**`assertFalse`**        | 85
**`assertRaises`**       | 69
**`assertNotIn`**        | 64
**`assertNotEqual`**     | 39
**`assertIsNotNone`**    | 35
**`assertIsInstance`**   | 35
**`assertLessEqual`**    | 23
**`assertGreater`**      | 15
**`assertLess`**         | 14
**`assertGreaterEqual`** | 14
**`assertAlmostEqual`**  | 9
**`assertRaisesRegex`**  | 4
**`assertCountEqual`**   | 2
**`assertListEqual`**    | 1


❗ Note that this *only* replaces methods provided by
`unittest` by default.
**Custom** assertion method replacements (like `assertExists` will be
addressed in a separate PR).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big refactor testing Relates to the testing/CI infrastructure
Projects
None yet
Development

No branches or pull requests

2 participants