Fix handling of unexpected success results #190
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ugh, python2 is enforcing the object's type for the self argument. While python3 will take a different type we pass in. We'll have to figure out a different method of doing this I guess, maybe just building a helper to do manually what testtools.TestResult.wasSuccessful() is doing. |
mtreinish
force-pushed
the
use-testtools_wasSuccessful
branch
from
ec18f05
to
eb97f84
Compare
|
Ok, I just replaced the usage of TestResult.wasSuccessful() with a custom helper function that does the same thing. It's only a 1 line function, but I think it makes it cleaner than manually checking the counts in all the places we call wasSuccessful() |
|
Why not change this too? stestr/stestr/commands/last.py Line 157 in eb97f84 |
|
Heh, because I missed it. I'll push up a new revision in a minute to correct the oversight. |
mtreinish
force-pushed
the
use-testtools_wasSuccessful
branch
from
eb97f84
to
cbca1b9
Compare
This commit addresses a bug when unexpected success results are used by a test suite. stestr was relying on wasSuccessful() method in the StreamResult class from testtools, which doesn't properly account for unexpected success. So when stestr was run on a test suite it wouldn't properly handle tests with unexpected success result and thus treat the run as successful. This addresses the issue by adjusting our testtools api usage to manually query the results object for unxsuccess results instead of relying on the exisiting wasSuccessful() method from the StreamResult class. This is basically the same as the wasSuccessful() method from the testtools.TestResult class, which exhibits the correct behavior, but isn't something we're using. An issue has been opened in testtools testing-cabal/testtools#273 regarding the accounting error with StreamResult.wasSuccessful(), if/when a fix is introduced into a released testtools we can investigate switching these calls back to use that method and deleting our local helper function. But, for the time being this mitigates the issue. Fixes #189
mtreinish
force-pushed
the
use-testtools_wasSuccessful
branch
from
cbca1b9
to
a96c158
Compare
|
@masayukig ok, the latest revision now has the last command updated too |
masayukig
approved these changes
Aug 7, 2018
jogo
added a commit
to jogo/stestr
that referenced
this pull request
Sep 16, 2022
Previously `stestr run` and subunit_trace used slightly different output and validation logic. run wouldn't catch the case where a test exited without a status, while code using `last` and `history` would since they used subunit_trace. Refactor the subunit_trace output and validation logic and use it for `load`/`run`. One side effect of this, which required a change in tests, is the current logic will return an error if the only test that is run is an expected failure (Related to mtreinish#190). Converge on the subunit_trace formatted output, in addition to the summary also show if something goes wrong. The primary motivation for this change is to better detect when a subprocess is killed and not all tests are run as expected (segfault, OOM etc.).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit addresses a bug when unexpected success results are used
by a test suite. stestr was relying on wasSuccessful() method in the
StreamResult class from testtools, which doesn't properly account for
unexpected success. So when stestr was run on a test suite it wouldn't
properly handle tests with unexpected success result and thus treat
the run as successful. This addresses the issue by adjusting our
testtools api usage to call the wasSuccessful() method from the
TestResult class instead which has the correct behavior.
An issue has been opened in testtools testing-cabal/testtools#273
regarding the accounting error with StreamResult.wasSuccessful(),
if/when a fix is introduced into a released testtools we can investigate
switching these calls back to use that function. But for the time being
this mitigates the issue.
Fixes #189