Fix result when test task fails so that correct exit code is returned #303
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
smaifullerton-wk
changed the title
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
evanweible-wf
approved these changes
Jun 13, 2019
lib/src/dart_dev_cli.dart
Outdated
| @@ -182,9 +182,9 @@ Future _run(List<String> args) async { | |||
|
|
|||
| reporter.log(''); | |||
| if (result.successful) { | |||
| reporter.success(result.message, shout: true); | |||
| reporter.success(result.message ?? '', shout: true); | |||
There was a problem hiding this comment.
I'm a little concerned/unclear what empty string will accomplish if result.message is null.
There was a problem hiding this comment.
isNotEmpty threw here: https://github.com/Workiva/dart_dev/blob/master/lib/src/reporter.dart#L102
I will push a commit to fix it up farther down the stack though.
todbachman-wf
approved these changes
Jun 13, 2019
|
👏 , but CI failed. |
smaifullerton-wk
force-pushed
the
fix-exit-code-when-test-task-fails
branch
from
2a65d69 to
f4040ee
Compare
todbachman-wf
approved these changes
Jun 13, 2019
|
+10 Testing Performed
|
corwinsheahan-wf
approved these changes
Jun 14, 2019
|
@Workiva/release-management-p |
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.
In the test task, the
outputProcessedfuture was only completed if the tests ran or if the test regex didn't match any test names. If anything else happened, the task'sdonefuture never completed, the result was never returned todart_dev_cli.dart, and no matter what happened in the task, an exit code of0was returned.This PR makes sure that the future is completed in the test task no matter what. As a result, we can remove the extra build that was going on in the test task before. Now, if you run
ddev testand the build fails, an exit code of 1 will be returned.QA +1 Instructions
config.test.deleteConflictingOutputsoption set to true.ddev testwith a passing build, ensure that the--delete-conflicting-outputsflag is passed to the build command, and tests run successfully.ddev test. After the build fails, tryecho $?and ensure that the exit code returned was 0.