From 76dc4b3fc37c97df8520f01985a79bbac5d1585d Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Fri, 31 Jul 2020 16:24:24 +0100 Subject: [PATCH] testscript: relax constraints around actual source in cmp with update (#107) 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. --- testscript/cmd.go | 2 +- ...estscript_update_script_actual_is_file.txt | 19 +++++++++++++++++++ ...update_script_expected_not_in_archive.txt} | 2 ++ testscript/testscript.go | 8 ++++---- 4 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 testscript/testdata/testscript_update_script_actual_is_file.txt rename testscript/testdata/{testscript_update_script_not_in_archive.txt => testscript_update_script_expected_not_in_archive.txt} (75%) diff --git a/testscript/cmd.go b/testscript/cmd.go index aeebbd62..eada14dd 100644 --- a/testscript/cmd.go +++ b/testscript/cmd.go @@ -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 diff --git a/testscript/testdata/testscript_update_script_actual_is_file.txt b/testscript/testdata/testscript_update_script_actual_is_file.txt new file mode 100644 index 00000000..297c7aa4 --- /dev/null +++ b/testscript/testdata/testscript_update_script_actual_is_file.txt @@ -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 diff --git a/testscript/testdata/testscript_update_script_not_in_archive.txt b/testscript/testdata/testscript_update_script_expected_not_in_archive.txt similarity index 75% rename from testscript/testdata/testscript_update_script_not_in_archive.txt rename to testscript/testdata/testscript_update_script_expected_not_in_archive.txt index 333984a4..81766a65 100644 --- a/testscript/testdata/testscript_update_script_not_in_archive.txt +++ b/testscript/testdata/testscript_update_script_expected_not_in_archive.txt @@ -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 diff --git a/testscript/testscript.go b/testscript/testscript.go index 83e4e7c0..d915fda1 100644 --- a/testscript/testscript.go +++ b/testscript/testscript.go @@ -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