From bb40e19acb356247518adc7c4f41b62ba3b94142 Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Wed, 16 Jun 2021 00:50:09 -0700 Subject: [PATCH] Allow all torchvision test rules to run with RE so that all CPU-only test rules are ignored in GPU CI machines Summary: This diff allows all torchvision test targets to run with RE, which allows the `IN_RE_WORKER` variable to be accurate and addresses https://www.internalfb.com/diff/D29105975?dst_version_fbid=579903510079850&transaction_fbid=282918386905496 Now, **all** CPU-only tests are ignored in the GPU contbuild (pytorch_vision-gpu). Previously, the CPU-only tests in files that had **no** CUDA tests were still run on GPU CI machines. Only the CPU-only tests in files that also had some CUDA tests were properly ignored. Note that just because we allow all test rules to run with RE doesn't mean they'll always be run with RE. Namely, they'll only run with RE when invoked from the pytorch_vision-gpu contbuild. They won't run with RE when invoked with the pytorch_vision (CPU-only) contbuild. Note 2: I had to deactivate RE for `test_image.py`. This is only temporary until nbjpeg is properly supported, see comments. Reviewed By: fmassa Differential Revision: D29132838 fbshipit-source-id: 206b0a635d723abf9930c6c71f4b35429a40b508 --- test/conftest.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index ab60cb5fa41..c860521df0b 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -11,12 +11,19 @@ def pytest_configure(config): def pytest_collection_modifyitems(items): - # This hook is called by pytest after it has collected the tests (google its name!) + # This hook is called by pytest after it has collected the tests (google its name to check out its doc!) # We can ignore some tests as we see fit here, or add marks, such as a skip mark. + # + # Typically here, we try to optimize CI time. In particular, the GPU CI instances don't need to run the + # tests that don't need CUDA, because those tests are extensively tested in the CPU CI instances already. + # This is true for both CircleCI and the fbcode internal CI. + # In the fbcode CI, we have an additional constraint: we try to avoid skipping tests. So instead of relying on + # pytest.mark.skip, in fbcode we literally just remove those tests from the `items` list, and it's as if + # these tests never existed. out_items = [] for item in items: - # The needs_cuda mark will exist if the test was explicitely decorated with + # The needs_cuda mark will exist if the test was explicitly decorated with # the @needs_cuda decorator. It will also exist if it was parametrized with a # parameter that has the mark: for example if a test is parametrized with # @pytest.mark.parametrize('device', cpu_and_gpu()) @@ -50,3 +57,19 @@ def pytest_collection_modifyitems(items): out_items.append(item) items[:] = out_items + + +def pytest_sessionfinish(session, exitstatus): + # This hook is called after all tests have run, and just before returning an exit status. + # We here change exit code 5 into 0. + # + # 5 is issued when no tests were actually run, e.g. if you use `pytest -k some_regex_that_is_never_matched`. + # + # Having no test being run for a given test rule is a common scenario in fbcode, and typically happens on + # the GPU test machines which don't run the CPU-only tests (see pytest_collection_modifyitems above). For + # example `test_transforms.py` doesn't contain any CUDA test at the time of + # writing, so on a GPU test machine, testpilot would invoke pytest on this file and no test would be run. + # This would result in pytest returning 5, causing testpilot to raise an error. + # To avoid this, we transform this 5 into a 0 to make testpilot happy. + if exitstatus == 5: + session.exitstatus = 0