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 more tests files to pytest #4033

Closed
8 tasks done
NicolasHug opened this issue Jun 10, 2021 · 6 comments
Closed
8 tasks done

Port more tests files to pytest #4033

NicolasHug opened this issue Jun 10, 2021 · 6 comments

Comments

@NicolasHug
Copy link
Member

NicolasHug commented Jun 10, 2021

Now that we support pytest, we want to remove the use of the unittest module for different test files. We've already ported a lot of tests in previous issues (#3987, #3945, #3956, #3951), and we want to extend this to more files now.

Instructions

I listed the list of files that need to be ported below. If you're interested in working on this issue, please comment below with "I'm working on file X" so that others don't pick the same file as you do. To keep things simple, please only submit one PR per file, and don't pick more than 2 files at once. You can pick more files once your PRs are merged. Before picking a group, make sure it wasn't picked by another contributor first.

Please don't hesitate to ask for guidance and help! Thanks!!

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 methods out of the Tester(unittest.TestCase) class and just declare it as a function. Note: for some files, it might make sense to keep the class structure, and just rename the class as e.g TestBlahBlah:
  • 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 or torch.testing.assert_close (look for other usage of these files in the codebase if needed)
    • 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.
  • It may make sense to keep related tests within a single class. Not all groups need a dedicated class though, it's on a case-by-case basis.

Files that need to be ported:

cc @pmeier

@AnirudhDagar
Copy link
Contributor

I'm working on test_datasets_video_utils.py and test_hub.py.

@DevPranjal
Copy link
Contributor

Hey @NicolasHug, I will try working on test_transforms_video.py.

@vivekkumar7089
Copy link
Contributor

Hi @NicolasHug !! I will work on test_datasets_samplers.py and test_models_detection_anchor_utils.py.

@zhiqwang
Copy link
Contributor

Hey @NicolasHug, I will try working on test_onnx.py and test_models_detection_negative_samples.py.

@AnirudhDagar
Copy link
Contributor

I'll also pickup the only one left test_models_detection_utils.py. Should be quite easy too.

@NicolasHug
Copy link
Member Author

All done! Thank you so much everyone for the blazing fast PRs!!

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

No branches or pull requests

5 participants