Skip to content

Commit

Permalink
testscript: relax constraints around actual source in cmp with update
Browse files Browse the repository at this point in the history
There are currently constraints on the actual source for a failed cmp
comparison when Params.UpdateScripts is true: the actual must be either
stdout or stderr.

However it's unclear why this constraint is necessary.

Indeed it's a painful constraint when the actual source in a comparison
is file written by a previous command.

Relax this constraint because, after discussion with @mvdan and
@rogpeppe, we believe it is safe to do so.

Add a test for this new logic where the actual is a file that is part of
the archive.
  • Loading branch information
myitcv committed Jul 31, 2020
1 parent be09da5 commit 28c666f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 5 deletions.
2 changes: 1 addition & 1 deletion testscript/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (ts *TestScript) doCmdCmp(args []string, env bool) {
if text1 == text2 {
return
}
if ts.params.UpdateScripts && !env && (args[0] == "stdout" || args[0] == "stderr") {
if ts.params.UpdateScripts && !env {
if scriptFile, ok := ts.scriptFiles[absName2]; ok {
ts.scriptUpdates[scriptFile] = text1
return
Expand Down
19 changes: 19 additions & 0 deletions testscript/testdata/testscript_update_script_actual_is_file.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
unquote scripts/testscript.txt
unquote testscript-new.txt
testscript-update scripts
cmp scripts/testscript.txt testscript-new.txt

-- scripts/testscript.txt --
>cmp got want
>
>-- got --
>right
>-- want --
>wrong
-- testscript-new.txt --
>cmp got want
>
>-- got --
>right
>-- want --
>right
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Verify that comparing stdout against a file not in the archive does nothing

unquote scripts/testscript.txt
cp scripts/testscript.txt unchanged
! testscript-update scripts
Expand Down
8 changes: 4 additions & 4 deletions testscript/testscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ type Params struct {
// error will be ignored.
IgnoreMissedCoverage bool

// UpdateScripts specifies that if a `cmp` command fails and
// its first argument is `stdout` or `stderr` and its second argument
// refers to a file inside the testscript file, the command will succeed
// and the testscript file will be updated to reflect the actual output.
// UpdateScripts specifies that if a `cmp` command fails and its second
// argument refers to a file inside the testscript file, the command will
// succeed and the testscript file will be updated to reflect the actual
// content (which could be stdout, stderr or a real file).
//
// The content will be quoted with txtar.Quote if needed;
// a manual change will be needed if it is not unquoted in the
Expand Down

0 comments on commit 28c666f

Please sign in to comment.