x/tools/go/analysis/internal/checker: tests failing on Plan 9 because of dependence on numeric exit codes

[millerresearch]

Go version

gotip

Output of go env in your module/workspace:

GOARCH=arm
GOBIN=
GOCACHE=/home/swarming/.swarming/w/ir/x/w/gocache
GOHOSTARCH=amd64
GOHOSTOS=linux
GOMAXPROCS=4
GOOS=plan9
GOPATH=/home/swarming/.swarming/w/ir/x/w/gopath
GOPLSCACHE=/home/swarming/.swarming/w/ir/x/w/goplscache
GOROOT=
GOROOT_BOOTSTRAP=/home/swarming/.swarming/w/ir/cache/tools/go_bootstrap
GOTOOLCHAIN=local
GO_BUILDER_NAME=x_tools-gotip-plan9-arm

What did you do?

In x/tools subrepo, Plan 9 builder ran go test -json -short ./...

What did you see happen?

Tests TestFixes and TestNoEnd are consistently failing:

--- FAIL: TestFixes (27.93s)
    fix_test.go:80: $ /home/swarming/.swarming/w/ir/x/t/go-build221040081/b410/checker.test -fix rename duplicate
        os.TempDir//analysistest609283431/src/rename/foo.go:4:2: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/rename/foo.go:5:6: renaming "bar```
" to "baz"
        os.TempDir//analysistest609283431/src/duplicate/dup.go:4:2: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/duplicate/dup.go:5:6: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/rename/intestfile_test.go:4:2: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/rename/intestfile_test.go:5:6: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/rename/foo_test.go:4:2: renaming "bar" to "baz"
        os.TempDir//analysistest609283431/src/rename/foo_test.go:5:6: renaming "bar" to "baz"
    fix_test.go:84: exit code was 1, want 3
--- FAIL: TestNoEnd (2.93s)
    fix_test.go:80: $ /home/swarming/.swarming/w/ir/x/t/go-build221040081/b410/checker.test -fix a
        os.TempDir//analysistest659219173/src/a/a.go:3:1: say hello
    fix_test.go:84: exit code was 1, want 3
FAIL
FAIL	golang.org/x/tools/go/analysis/internal/checker	213.128s

What did you expect to see?

Expected all tests to pass or be skipped.

The underlying problem is that process exit codes in UNIX-family operating systems are numeric, but in Plan 9 they are strings. The Plan 9 convention is that an empty string exit code means "success"; any other string means "failure" and the content of the string explains the reason, in more detail than a numeric errno value would allow.

The current Plan 9 implementation of os.ProcessState.ExitCode() is to return 0 if the Plan 9 exit string is empty, and 1 otherwise. This is sufficient in most cases, but not for applications which attempt to pass more information via exit codes than a simple fail/succeed indication -- as in the case of these failing tests where an exit code of 3 is expected.

My opinion is that it would not be unreasonable, when both parent and child process are Go programs, that if the child process exits with os.Exit(n) then the corresponding os.Wait() in the parent should return with an ExitCode() of n. This could be arranged by parsing the Plan 9 exit string, and if it contains a non-zero numeric value and nothing else, returning that value instead of 1.

In the meantime, however, the TestFixes and TestNoEnd tests should be modified to permit an exit code of 1 in the case of Plan 9,.

[gabyhelp]

Related Issues

• os/exec: TestExitCode failing on Plan 9 #27294 (closed)
• flag: TestExitCode failing on Plan 9 #38237 (closed)
• runtime: plan9: exit status is not passed correctly to exits syscall #53669 (closed)
• test: fixedbugs/issue30908.go failing on Plan 9 #31503
• os: TestRemoveAllWithMoreErrorThanReqSize is failing on Plan 9 and Windows #35117 (closed)
• x/tools/gopls/internal/regtest/misc: TestRenameFileFromEditor failures with `invalid argument` on plan9 #57111 (closed)
• testing/fstest: test that ReadDir on file returns error, or returns 0 entries if file is empty #44967
• cmd/test2json: Fails to mark a test as failed when using os.Exit #43263 (closed)
• x/crypto/ssh/agent: TestServerResponseTooLarge fails on Plan 9 #35888 (closed)
• testing: report non-zero exit status in -json output #61839

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[thanm]

Thanks for the report.

[gopherbot]

Change https://go.dev/cl/596735 mentions this issue: go/analysis/internal/checker: allow for Plan 9 reduced exit codes in tests