Repository navigation
3.12 starts to return an error when all tests in a test file are skipped #113661
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
on 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.
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:
it displays "NO TEST RUN" but the exit code is a success
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.
Reacted by Carl Meyer and Karl KnechtelAlso looking into the PR, its definition of
no_tests_runis:def no_tests_run(self): return not any((self.good, self.bad, self.skipped, self.interrupted, self.environment_changed))
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?Reacted by Carl MeyerThe 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?
#98903 and its related changes are about the CPython test suite and its
regrtestrunner, it is unrelated tounittest.#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.
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.
@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.
Reacted by Yuxuan 'fishy' Wang and wookie184Not 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?
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.
Reacted by Alex WaygoodI'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:
def no_tests_run(self): return not any((self.good, self.bad, self.skipped, self.interrupted, self.environment_changed))
with:
def no_tests_run(self): return not any((self.good, self.bad, self.skipped, self.interrupted, self.environment_changed)) and (not self.skipped)
Reacted by Carl Meyer and Gregory P. SmithMaybe we should not consider that "no tests were run" if at least one test was skipped?
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.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.@zware I don't see any discussion of the "nonzero tests found but all skipped" situation in any of the threads you linked.
Sorry, that's #106584, which to be fair has next to no discussion. cc author @EliseevEgor and committer @hugovk.
To be clear, this issue has nothing to do with the CPython test suite
Oh. Now I'm confused.
@fishy: Would you mind to write a short reproducer of your issue?
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 (...)
This example uses a third party module, so it's not easy to run without having to install Apache Thrift.
Sorry, that's #106584, which to be fair has next to no discussion. cc author @EliseevEgor and committer @hugovk.
cc committer @ambv (I closed the issue two months later).
🤦♂️ Sorry for the noise, Hugo.
Reacted by Hugo van Kemenade7 remaining items
- added a commit that references this issue
on Jan 9, 2024 thanks for the report and PR!
- added 6 commits that reference this issue
on Jan 11, 2024
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:
On python 3.11 this is fine with:
But on python 3.12, this starts to become an error:
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