Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testscript: 'cmp stdout want' diff seems backwards #187

Open
bitfield opened this issue Nov 13, 2022 · 5 comments
Open

testscript: 'cmp stdout want' diff seems backwards #187

bitfield opened this issue Nov 13, 2022 · 5 comments

Comments

@bitfield
Copy link
Contributor

Consider the familiar:

want := "goodbye"
got := "hello"
if !cmp.Equal(want, got) {
    t.Error(cmp.Diff(want, got))
}

which produces:

-goodbye
+hello

Now compare this to what testscript does:

exec echo hello
cmp stdout want

-- want --
goodbye

produces:

-hello
+goodbye

When you're accustomed to using cmp.Diff(want, got), this looks backwards. It looks as though we expected hello, but got goodbye, which is the opposite of what happened.

To get a diff that appears the “right” way round, we'd need to write:

cmp want stdout

But that doesn't work:

open $WORK/stdout: no such file or directory

Is there a reason why stdout isn't allowed to be used as the second argument to cmp?

@bitfield bitfield changed the title testscript: 'cmp stdout golden.txt' diff seems backwards testscript: 'cmp stdout want' diff seems backwards Nov 13, 2022
@thepudds
Copy link
Contributor

Hi @bitfield, possibly related is the update feature uses the second argument to cmp as an updatable file:

// 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).
//

(Random example usage).

@bitfield
Copy link
Contributor Author

Good point @thepudds. Personally, I'm not convinced that auto-updating golden files is a wise idea:

The most serious objection to an -update flag is that it provides a very easy and automated way to render your tests useless. Suppose, for example, that you make a minor change to some file generation routine that creates megabytes of output data. And suppose that you rigorously scrutinise every byte of that output to make sure it’s correct, and finally you run go test -update to overwrite your golden file.

Well, from now on your tests will always pass, since you’ve defined the required behaviour as whatever the function currently does. You might be very diligent in checking the output before committing an updated golden file, but someone else might not be so careful. Sooner or later, someone will make a mistake. And from that point on, we’ll be in a very uncomfortable situation, because the test will say that everything’s okay when, in fact, the system is incorrect.

Worse, if someone should change the system to have the correct behaviour, the test will immediately fail! It takes considerable strength of mind to overrule a failing test, and in any case a good deal of time will probably be wasted figuring out what’s gone wrong.

The best golden file, then, is real data, taken from some external oracle or system. The next best is hand-rolled data, carefully constructed by someone who knows exactly what the output should look like. When this format needs to change, change the golden file manually. The very last resort, and something we should avoid if at all possible, is to use the system itself to generate the golden data, for the reasons we’ve discussed. If this ever has to be done, the results should be checked rigorously, and then manually updated as the requirements evolve.
The Power of Go: Tests

TL;DR it should be a big deal to change the golden file. We shouldn't make it easier to do things it's not a good idea to do.

But that's by the by. If cmp compares two things, one of which is stdout, it should be straightforward to arrange for -update to update the other one. If cmp is comparing two real files, it's not clear a priori which one should be updated, so perhaps -update should simply have no effect here.

@thepudds
Copy link
Contributor

Hi @bitfield

If cmp compares two things, one of which is stdout, it should be straightforward to arrange for -update to update the other one. If cmp is comparing two real files, it's not clear a priori which one should be updated, so perhaps -update should simply have no effect here.

FWIW, I do use cmp with things other than stdout, including in some cases write results to a file that is then used with cmp.

Perhaps rather than keying off of stdout/stderr, it could be that -update updates whichever cmp argument is present as a file in the testscript file. It already has logic I think to check if its second argument is present in the testscript file. (And perhaps -update could be a no-op or maybe even error if both arguments to cmp are files in the testscript file, which is also not the most useful comparison, or alternatively, in that case it could favor updating the second argument as it does today).

But I haven't fully thought that through, so maybe that is flawed. 🙃

All that said, there's also a cost to diverging further from the testscript version that lives in the Go project...

@bitfield
Copy link
Contributor Author

-update updates whichever cmp argument is present as a file in the testscript file

That works for me!

@rogpeppe
Copy link
Owner

Works for me too.

This is straying from the issue, but I found the arguments above against -update interesting, and I tend to agree in principle, but in practice golden files seem to make for a nice halfway house between no explicit checking of ouput and huge numbers of manual assertions about aspects of the output. Even when automatically updated, they give code reviewers insight into what's going on. In the CUE project, for example, where golden files are used extensively, the automatic update feature saves hours of manual labor each week while providing quite good assurance that things look ok.

Personally I quite like combining golden files with other independent (and manually updated) assurances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants