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

Port test/test_datasets_utils.py to pytest #4063

Closed
NicolasHug opened this issue Jun 15, 2021 · 1 comment · Fixed by #4114
Closed

Port test/test_datasets_utils.py to pytest #4063

NicolasHug opened this issue Jun 15, 2021 · 1 comment · Fixed by #4114

Comments

@NicolasHug
Copy link
Member

NicolasHug commented Jun 15, 2021

Currently, most tests in test/test_datasets_utils.py rely on unittest.TestCase. Now that we support pytest, we want to remove the use of the unittest module.

Instructions

Please comment below if you're interested to work on this and submit a PR for all the tests in this file.

Here are a few instructions specific to this file. I just gave it a brief look so I might be wrong on some of these, use your best jugement ;)

  • parametrize test_detect_file_type over (file, expected)
  • group all of test_detect_file_type_no_ext,
    test_detect_file_type_to_many_exts,
    test_detect_file_type_unknown_archive_type,
    test_detect_file_type_unknown_compression,
    test_detect_file_type_unknown_partial_ext, into a single test function
    parametrized over the file name, and convert the function names as
    comments to document the tests
  • test_decompress_gzip and test_decompress_lzma and
    test_decompress_remove_finished can probably be bundled into the same test
    function with some parametrization over the extension and over remove_finished
  • test_extract_archive_defer_to_decompress should be parametrized over ext and removed_finished (2 separate parametrizations)
  • for test_extract_archive_defer_to_decompress, use the mocker fixture as done in https://github.com/pytorch/vision/pull/4032/files
  • there are other stuff that could be parametrize, feel free to try things that make sense

How to port a test to pytest

Porting a test from unittest to pytest is usually fairly straightforward. For a typical example, see https://github.com/pytorch/vision/pull/3907/files:

  • take the test method out of the Tester(unittest.TestCase) class and just declare it as a function
  • Replace @unittest.skipIf with pytest.mark.skipif(cond, reason=...)
  • remove any use of self.assertXYZ.
    • Typically assertEqual(a, b) can be replaced by assert a == b when a and b are pure python objects (scalars, tuples, lists), and otherwise we can rely on assert_equal which is already used in the file.
    • self.assertRaises should be replaced with the pytest.raises(Exp, match=...): context manager, as done in https://github.com/pytorch/vision/pull/3907/files. Same for warnings with pytest.warns
    • self.assertTrue should be replaced with a plain assert
  • When a function uses for loops to tests multiple parameter values, one should usepytest.mark.parametrize instead, as done e.g. in https://github.com/pytorch/vision/pull/3907/files.

cc @pmeier

@AnirudhDagar
Copy link
Contributor

I'd love to work on this, will send a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants