-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Description
Currently, most tests in test/test_models.py rely on unittest.TestCase
. Now that we support pytest
, we want to remove the use of the unittest
module.
For a similar issue: see #3945 and #3956
Instructions
There are many tests in this file, but it should be possible to port all of them in one PR as they're all pretty straightforward. If you're interested in this issue, please comment below to indicate that you started working on a PR. Look below for some porting tips, and please don't hesitate to ask for help. Thanks!
In this file there are already some test functions like:
def test_classification_model(model_name, dev):
ModelTester()._test_classification_model(model_name, dev)
For those, we should just copy the body of _test_classification_model
into test_classification_model
so that we can get rid of the ModelTester
class altogether.
@pytest.mark.parametrize('dev', _devs)
should be changed into @pytest.mark.parametrize('dev', cpu_and_gpu())
where cpu_and_gpu()
is in common_utils
.
The tests that need cuda (e.g. test_fasterrcnn_switch_devices
) should use the @needs_cuda
decorator, also from common_utils
. The test that don't need cuda should use the @cpu_only
decorator.
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
withpytest.mark.skipif(cond, reason=...)
- remove any use of
self.assertXYZ
.- Typically
assertEqual(a, b)
can be replaced byassert a == b
when a and b are pure python objects (scalars, tuples, lists), and otherwise we can rely onassert_equal
which is already used in the file. self.assertRaises
should be replaced with thepytest.raises(Exp, match=...):
context manager, as done in https://github.com/pytorch/vision/pull/3907/files. Same for warnings withpytest.warns
self.assertTrue
should be replaced with a plainassert
- Typically
- When a function uses for loops to tests multiple parameter values, one should use
pytest.mark.parametrize
instead, as done e.g. in https://github.com/pytorch/vision/pull/3907/files.
cc @pmeier