Allow RunException to fail tests correctly #10909
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
A couple RunException tests were allowed to pass when they should have been failing. However, we still want these tests to pass, so use expect_out, instead of expect_error for the deprecated tests located in misc/check_error. Closes idaholab#10907
milljm
commented
Feb 26, 2018
| @@ -67,7 +67,7 @@ def processResults(self, moose_dir, options, output): | |||
| output += "#"*80 + "\n\nUnable to match the following pattern against the program's output:\n\n" + specs['expect_err'] + "\n" | |||
|
|
|||
| if reason == '': | |||
| RunApp.testFileOutput(self, moose_dir, options, output) | |||
| reason = RunApp.testFileOutput(self, moose_dir, options, output) | |||
There was a problem hiding this comment.
woops :)
If RunApp.testFileOutput set this test as failed, this test would later (the next few lines) set it as pass.
|
Job Documentation on 0ef0fdf wanted to post the following: View the site here This comment will be updated on new commits. |
RunException tests need to exit with a non-zero. Closes idaholab#10907
brianmoose
reviewed
Feb 26, 2018
| @@ -18,3 +18,4 @@ | |||
| sample.update() | |||
| x = sample[0].getDistance() | |||
| y = sample[0].getSample('invalid_name') # testing this error | |||
| exit(1) | |||
There was a problem hiding this comment.
@aeslaughter This test was a RunException test that requires a non-zero exit status.
It is either this change or change the test spec into a RunApp test with expect_out and setting errors to a dummy value (since there is an ERROR in the output). Thoughts?
There was a problem hiding this comment.
Just looking for the error in the output should be fine.
Allow line_sample_error to cause an exception, but use RunApp to capture expect_out. To do this, we need to suppress RunApp's default behavior of labeling this exception causing test as a failure, by overriding the errors parameter with a string which will never occur in the output. Closes idaholab#10907
|
The failing Mac Tests are not related to this PR |
brianmoose
approved these changes
Feb 27, 2018
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.
A couple RunException tests were allowed to pass when they should
have been failing. However, we still want these tests to pass, so
use expect_out, instead of expect_error for the deprecated tests
located in misc/check_error.
Closes #10907