stderr from exec-ed 'go test' should be fully checked for correctness

[geomacy]

Summary

The stderr from an exec-ed go test command has an expected format, and is collected with .combinedOutput() and used to calculate a path. The collected stderr output should be fully parsed to ensure it matches the expectation and does not contain not some other error. This happens in builder.go / builder_go110.go, as below, but the same general principal applies anywhere this technique is used, so it would be worth reviewing other uses of .combinedOutput() in the code.

Detail

At

https://github.com/DATA-DOG/godog/blob/857a19d0bffef27ef42ed8c5c93c793d998f7591/builder_go110.go#L94

the builder execs 'go test', directing stdout > /dev/null and collecting the stderr using .combinedOutput(). This is then checked at

https://github.com/DATA-DOG/godog/blob/857a19d0bffef27ef42ed8c5c93c793d998f7591/builder_go110.go#L101

and if it begins as expected with 'WORK=', this is then stripped and the resulting value is subsequently used to construct the testdir file path.

However, just checking the 'WORK=' prefix is not enough to ensure that the output of the command is correct.

On my Mac I ran into the problem crypto/x509: macOS -framework Security produces link warning, so that the combined output of the go test -c -work -o /dev/null command looked like

WORK=/var/folders/10/7wxypczs3ng9dny8kpnwc9nr0000gn/T/go-build324094032
# crypto/x509
ld: warning: text-based stub file /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks/Security.framework/Security.tbd and library file /System/Library/Frameworks/Security.framework/Security are out of sync. Falling back to library file for linking.

Note this is the expected format of output but with additional lines.

The upshot of this is that when I ran godog, it failed with output like

$ godog
open /var/folders/wg/9g2tjgcj2350_wn8g4x8kj0h0000gp/T/go-build385337462
# crypto/x509
ld: warning: text-based stub file /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks/Security.framework/Security.tbd and library file /System/Library/Frameworks/Security.framework/Security are out of sync. Falling back to library file for linking../b001/_testmain.go: file name too long

It's not at all obvious that the whole thing after the open is the actual file name it thinks it is opening, line breaks and all.

Ideally the code in builder_go110.go (and the same thing done in builder.go) should check the output fully. Is the output a single line matching WORK=/some/path? Does that file path exist? Only then should it use the result.

[l3pp4rd]

Hi @geomacy yes, makes sense, will need to adapt this to properly read stderr as error information details. Maybe you'd like to contribute this one?

[geomacy]

hi @l3pp4rd, sorry, I missed your comment on this last week; I would have been happy to contribute to this one, but I see you've already sorted it out. The changes in 5ed3399 look good to me. I am happy to report that I have pulled this latest code this morning and it has fixed my problem, i.e. I can now run my unit tests again even with the latest xcode command line tools version 10.0.0.0.1. Many thanks!

[l3pp4rd]

great, cheers