cmd/testscript: add a -continue flag to not stop at the first error#189
Merged
mvdan merged 1 commit intorogpeppe:masterfrom Nov 23, 2022
Merged
cmd/testscript: add a -continue flag to not stop at the first error#189mvdan merged 1 commit intorogpeppe:masterfrom
mvdan merged 1 commit intorogpeppe:masterfrom
Conversation
32e554d to
da4eb80
Compare
Collaborator
|
@mvdan I helped out in a small way by adding a test :) |
rogpeppe
approved these changes
Nov 23, 2022
Owner
rogpeppe
left a comment
There was a problem hiding this comment.
LGTM modulo one suggestion for a slight test improvement, thanks (and good catch on runT!)
Collaborator
Author
|
I did forget a test, didn't I :) |
da4eb80 to
45ae1ac
Compare
This helps with writing and running bug reproducers using testscript. Since they are often meant to represent the desired behavior, and not the current behavior, they are designed to fail. However, some reproducers consist of multiple commands, and cmd/testscript would stop at the first command to error. When running a reproducer which is designed to fail, we want to run the entire script and see all failures. Add a `-continue` flag which does just that. With it, `T.FailNow` still marks the test as failed, but does not stop its execution via a panic. We also make two small changes to behave correctly when we reach the end of a script but `T.Failed` is true; we should "FAIL" rather than "PASS". The previous code only reached that point if no errors had happened. Note that we also altered `runT` to use pointer method receivers. It used atomics to modify and read its `failed` field, but without pointer receivers, `T.Failed` always returned false. It appears that bug had been present for a long time. Co-Authored-By: Paul Jolly <[email protected]>
45ae1ac to
5cef7b9
Compare
Collaborator
Author
|
Thanks! Will merge on green. |
myitcv
approved these changes
Nov 23, 2022
Collaborator
myitcv
left a comment
There was a problem hiding this comment.
LGTM. But I'm biased, because I helped write the test :)
rogpeppe
added a commit
that referenced
this pull request
Jan 5, 2023
The flag was added in #189 but not documented in the usage message.
rogpeppe
added a commit
that referenced
this pull request
Jan 6, 2023
The flag was added in #189 but not documented in the usage message.
This file contains hidden or 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
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.
(see commit message)