3.12 starts to return an error when all tests in a test file are skipped #113661
Copy link
Copy link
Closed
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytestsTests in the Lib/test dirTests in the Lib/test dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
Bug description:
See https://github.com/apache/thrift/blob/5cf71b2beec3c67a4c8452ddabbbc6ae43fff16f/lib/py/test/test_sslsocket.py for a full example, we skipped all tests in that file via:
@unittest.skip("failing SSL test to be fixed in subsequent pull request")
class TSSLSocketTest(unittest.TestCase):
...On python 3.11 this is fine with:
/usr/bin/python3 test/test_sslsocket.py
sssssssssss
----------------------------------------------------------------------
Ran 11 tests in 0.000s
OK (skipped=11)
But on python 3.12, this starts to become an error:
/opt/hostedtoolcache/Python/3.12.1/x64/bin/python test/test_sslsocket.py
WARNING:thrift.transport.sslcompat:using legacy validation callback
sssssssssss
----------------------------------------------------------------------
Ran 0 tests in 0.000s
NO TESTS RAN (skipped=11)
make[1]: *** [Makefile:645: py3-test] Error 5
I have to add a not-skipped dummy test to work around this (https://github.com/apache/thrift/pull/2914/files)
Is this an intentional change? I found it weird that we consider a skipped test as a failure.
CPython versions tested on:
3.12
Operating systems tested on:
Linux
Linked PRs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytestsTests in the Lib/test dirTests in the Lib/test dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
terryjreedy commentedon Jan 2, 2024
The change is quite intentional. See #98903.
@vstinner The issue PR #98904 added 4 new exit codes -- 2, 3, 4, and 130 -- to libregtest.main, since moved to libregtest.results. As far as I can detect, they are not mentioned in What's New 3.12. I think they should be. But what needs to be said? Just the new error situations, or the numbers also?
To add confusion, the unittest.main doc says that all-skipped has exit code 5, whereas you added 4. The other additions are not mentioned and there is no new-in 3.12 notice. Are these errors and omissions?
#106584 quoted 5 from the doc. I changed this to 4 from the code, but then realized that unittest is not regrtest so wonder if that change is correct. Is does not matter in a sense because neither constant appears in the code example or the patch.
fishy commentedon Jan 2, 2024
I would argue that the original issue #98903 is different from the end result in 3.12.
I can understand the rationale to fail a test if "no test is found", with the intention to catch typos. But "all tests are intentionally skipped" is quite different from "there are not really any tests in the file because of typos", and it should not be too hard to distinguish between the 2 cases?
The original bug report also says:
Which reinforces my belief that this end result (that it fails when all tests were skipped) is unintentional, as that used to display "OK (skipped=N)" in 3.11.
fishy commentedon Jan 2, 2024
Also looking into the PR, its definition of
no_tests_runis:Which seems to suggest that when there were skipped tests, it's supposed to return
False? So failing a test for the reason ofNO TESTS RANwith skipped tests seem to be a bug?zware commentedon Jan 3, 2024
#98903 and its related changes are about the CPython test suite and its
regrtestrunner, it is unrelated tounittest.It was not; I've reverted the change to the comment in #106584.
All that said, @terryjreedy is correct that the behavior here is as desired: generally speaking, if all of your tests are skipped, you aren't actually testing what you think you are and your test runner should complain about it. See #62432/#102051 for the original discussion. We should have added a whatsnew entry about it at the time, though.
carljm commentedon Jan 3, 2024
@zware I don't see any discussion of the "nonzero tests found but all skipped" situation in any of the threads you linked.
I don't find the argument "if all tests are skipped you aren't testing what you think you are" to be convincing. I can imagine many reasons why all tests found in a particular test run might be skipped, which should not be considered an error. I can't really imagine when I'd ever want that situation to be reported as an error. The whole point of skipping a test is that it shouldn't cause the run to fail.
I find "no tests found is an error" a useful behavior, and quite different.
Unless someone has a link to previous discussion of the "all skipped" scenario that makes it clear this was intentional and has good rationale, I think the behavior reported here is both a bug and a regression that should be fixed and backported.
AlexWaygood commentedon Jan 4, 2024
Not proposing that we close this as a duplicate (as this issue has more discussion), but is this the same issue that was reported in #113267?
carljm commentedon Jan 4, 2024
It seems plausible that the cause could the same / related (I haven't dug into the implementation details here), but the symptoms are definitely distinct, so I wouldn't consider them the same issue.
vstinner commentedon Jan 8, 2024
I'm using "No tests ran" and exit code 4 to detect typos in the -m parameter. For example,
./python -m test test_os -m test_acces:test_acces(missing trailing S) instead oftest_access.Maybe we should not consider that "no tests were run" if at least one test was skipped?
For example, replace:
with:
vstinner commentedon Jan 8, 2024
We can also use a different heuristic using "are tests filtered?" condition. For example, are
-mand/or-icommand line options used? SeeRunTests.match_testsmember.Note:
RunTests.match_tests_dictis different, it's used by the--rerunfeature.zware commentedon Jan 8, 2024
To be clear, this issue has nothing to do with the CPython test suite (aka
libregrtest, akapython -m test), it's aboutunittest(python -m unittest,unittest.main()) only.Sorry, that's #106584, which to be fair has next to no discussion. cc author @EliseevEgor and committer @hugovk.
vstinner commentedon Jan 8, 2024
Oh. Now I'm confused.
@fishy: Would you mind to write a short reproducer of your issue?
This example uses a third party module, so it's not easy to run without having to install Apache Thrift.
hugovk commentedon Jan 8, 2024
cc committer @ambv (I closed the issue two months later).
zware commentedon Jan 8, 2024
🤦♂️ Sorry for the noise, Hugo.
7 remaining items
GH-113661: unittest runner: Don't exit 5 if tests were skipped (#113856)
pythonGH-113661: unittest runner: Don't exit 5 if tests were skipped (p…
[3.12] GH-113661: unittest runner: Don't exit 5 if tests were skipped (…
gpshead commentedon Jan 9, 2024
thanks for the report and PR!
Do not fail on Python 3.12 when there are tests but all skipped.
Do not fail on Python 3.12 when there are tests but all skipped.
Do not fail on Python 3.12 when there are tests but all skipped.
Do not fail on Python 3.12 when there are tests but all skipped.
Do not fail on Python 3.12 when there are tests but all skipped.
Do not fail on Python 3.12 when there are tests but all skipped.
pythonGH-113661: unittest runner: Don't exit 5 if tests were skipped (p…
pythonGH-113661: unittest runner: Don't exit 5 if tests were skipped (p…
pythonGH-113661: unittest runner: Don't exit 5 if tests were skipped (p…