bpo-36674: Stops skipped tests from running in debug mode.#13180
bpo-36674: Stops skipped tests from running in debug mode.#13180lisroach wants to merge 2 commits into
Conversation
| self.setUp() | ||
| getattr(self, self._testMethodName)() | ||
| testMethod = getattr(self, self._testMethodName) | ||
| if (not getattr(self.__class__, "__unittest_skip__", False) and |
There was a problem hiding this comment.
I think it's not necessary use the round brackets for the condition.
There was a problem hiding this comment.
Do you mean the parens around the if statement? It is not necessary, but encouraged by the style guide to wrap multi-line if statements in parens: https://www.python.org/dev/peps/pep-0008/#multiline-if-statements
There was a problem hiding this comment.
Maybe it's out of the scope of this PR, but I'd like to suggest a small refactoring to make this condition a little bit clearer.
- Define a new
_is_skipped(self, testMethod)method:
def _is_skipped(self, testMethod):
return any(
getattr(t, "__unittest_skip__", False)
for t in (self.__class__, testMethod)
)- Update the new condition to
not self._is_skipped(testMethod)and the existing condition around line 665 toself._is_skipped(testMethod)
If it doesn't make sense to include this small update in this PR, but you think it makes sense in general, I'd be happy to work on it.
| def test_something(self): | ||
| self.addCleanup(_skip) | ||
|
|
||
| for klass in (Test1, Test2, Test3, Test4): |
There was a problem hiding this comment.
What did you mean with "klass"?
There was a problem hiding this comment.
This is a common variable name used throughout this file (see line 1774 for example), it's a variable representing a class.
|
How do I kick the CI to rerun their tests? I think these failures were transient. |
|
Close and Reopen -- as I did just now |
|
Thanks @asvetlov! |
|
It seems that Lines 654 to 662 in e4c431e With this in mind, and considering the docs would it make more sense to wrap in try/except like this: def debug(self):
"""Run the test without collecting errors in a TestResult"""
try:
self.setUp()
getattr(self, self._testMethodName)()
self.tearDown()
except SkipError:
pass
while self._cleanups:
function, args, kwargs = self._cleanups.pop(-1)
function(*args, **kwargs)Note: users and developers of pytest have been struggling with understanding the |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
setUp() should not be run for skipped tests. Perhaps a SkipTest should be raised for skipped tests.
If SkipTest is raised it the test method, it should be ignored. If it is raised in setUp() -- I am not sure whether tearDown() and doCleanup() should still be called.
|
https://bugs.python.org/issue36674 is closed. What is the status of this PR? |
|
Closing as this seems abandoned (see my previous comment). |
10 participants
TestCase.run() will skip tests that have skip/skipIf/skipWhatever applied to them, this PR applies the same skipped functionality when tests are run using TestCase.debug().
https://bugs.python.org/issue36674
https://bugs.python.org/issue36674