Currently, tests in Lib/test that are erroring/failing are marked as failures or skips in various ways. I propose that, for each test_*.py file in Lib/test/**, we collect most (if not all) instances of skipped tests into a single section at the bottom of the test file.
Detailed Explanation and Rationale
Often, when someone adds a CPython test to Lib/test, function decorations from unittest will have to be added in order for the test suite to pass muster. If a test function should fail, it should get marked as an expected failure so the CI can flag it once it passes. However, I have found several problems with this approach, based on my observations:
Unexpected passes are often hard to find within a large file of tests. (Line numbers for unexpected passes are not reported when the suite is invoked with -v, and function names can overlap.)
When a test fails under certain test cases but not others, I have often placed a four-liner within the offending test case/class (the last four lines in the example below). This four-liner can be difficult for newcomers to intuit, especially when the test unexpectedly passes, and the construct has to be dismantled.
When a test fails on a certain OS (or only on GitHub Actions), I have often placed a three/four-liner within the offending test case/class, but outside the test function (see examples below). Like the example above, this construct can be difficult for newcomers to intuit, especially when the test unexpectedly passes, and the construct has to be dismantled.
# TODO: RUSTPYTHONimportos, sys# depends on what has already been imported at the top of the test fileifsys.platform=="win32"andos.getenv("CI"):
test_windows_only_failure=unittest.expectedFailure(test_windows_only_failure)
When a test is not explicitly defined as a function, I would have to explicitly mark it as a test failure without using the annotation. Newer contributors may skip the test case outright.
Interspersing our annotations within the CPython test suite can make it difficult to upgrade the test suite to newer versions of CPython.
I propose adding a block near the bottom of every CPython test file that contains most (if not all) of the RustPython-specific test annotations. The block in question would be delineated by comments and would appear before the invocation of main() or unittest.main(), as shown:
...
### TODO: RUSTPYTHON, BEGIN TEST ANNOTATION BLOCKimportos, sys, unittestCPythonTestCase.test_failing_function=unittest.expectedFailure(CPythonTestCase.test_failing_function)
CPythonTestCase.test_fails_only_sometimes=unittest.expectedFailure(lambda*args, **kwargs: CPythonTestCase.mro()[1].fails_only_sometimes(*args, **kwargs))
CPythonTestCase.test_panic=unittest.skip(CPythonTestCase.test_panic, "TODO: RUSTPYTHON, thread 'main' panicked at 'panic!()'")
ifsys.platform=="win32"andos.getenv("CI"):
CPythonTestCase.test_fails_on_windows=unittest.expectedFailure(CPythonTestCase.test_fails_on_windows)
### TODO: RUSTPYTHON, END TEST ANNOTATION BLOCKif__name__=="__main__":
unittest.main()
(In the tes_fails_only_sometimes example, lambda is used to create a completely new function, so the call to unittest.expectedFailure() does not affect the other cases in which this function passes. The mro()[1] indirection is to prevent a recursion error (although the indexing could be improved).)
This would also confer the benefit of making it easier for someone to (semi-)automate marking a test file from CPython. A tool could run a test suite, find the failing tests (if the suite doesn't cause RustPython to panic), and add them to the block. Once a test unexpectedly passes, the relevant annotation can easily be removed from the block.
Drawbacks and Alternatives
I see a couple of drawbacks with this solution:
Not everything can fit "neatly" within this block. Sometimes, manually editing a test file is unavoidable because that could be the only way to make sure the test file can get parsed without errors. (See, for example, these lines in test_time.py.)
Converting our current suite from inline annotations to the annotation block is a mammoth endeavor, and the efforts may not pay off immediately.
The obvious alternative is to do nothing, as the status quo is working well enough.
Unresolved Questions
Should the block have a specified format?
Are there any existing tools that can modify python files in this or a similar way?
Suggestions are welcome!
The text was updated successfully, but these errors were encountered:
fanninpm
changed the title
[RFC] Consolidating RustPython's annotations in tjhe CPython test suite
[RFC] Consolidating RustPython's annotations in the CPython test suite from interleaved annotations to single blocks
Oct 19, 2021
Summary
Currently, tests in
Lib/testthat are erroring/failing are marked as failures or skips in various ways. I propose that, for eachtest_*.pyfile inLib/test/**, we collect most (if not all) instances of skipped tests into a single section at the bottom of the test file.Detailed Explanation and Rationale
Often, when someone adds a CPython test to
Lib/test, function decorations fromunittestwill have to be added in order for the test suite to pass muster. If a test function should fail, it should get marked as an expected failure so the CI can flag it once it passes. However, I have found several problems with this approach, based on my observations:-v, and function names can overlap.)I propose adding a block near the bottom of every CPython test file that contains most (if not all) of the RustPython-specific test annotations. The block in question would be delineated by comments and would appear before the invocation of
main()orunittest.main(), as shown:(In the
tes_fails_only_sometimesexample,lambdais used to create a completely new function, so the call tounittest.expectedFailure()does not affect the other cases in which this function passes. Themro()[1]indirection is to prevent a recursion error (although the indexing could be improved).)This would also confer the benefit of making it easier for someone to (semi-)automate marking a test file from CPython. A tool could run a test suite, find the failing tests (if the suite doesn't cause RustPython to panic), and add them to the block. Once a test unexpectedly passes, the relevant annotation can easily be removed from the block.
Drawbacks and Alternatives
I see a couple of drawbacks with this solution:
test_time.py.)The obvious alternative is to do nothing, as the status quo is working well enough.
Unresolved Questions
The text was updated successfully, but these errors were encountered: