cmd/go: report all loading errors in tests as "setup failed"

Currently, under *most* circumstances, if there's a package loading
error during "go test", that will get reported as a "FAIL p [setup
failed]" or "FAIL p [build failed] message and won't prevent running
unaffected test packages.

However, if there's a loading error from a non-test file in a package
listed directly on the "go test" command line, that gets reported as
an immediate fatal error, without any "FAIL" line, and without
attempting to run other tests listed on the command line. Likewise,
certain early build errors (like a package containing no Go files) are
currently immediately fatal rather than reporting a test failure.

Fix this by eliminating the check that causes that immediate failure.
This causes one minor follow-up problem: since
load.TestPackagesAndErrors was never passed a top-level package with
an error before, it doesn't currently propagate such an error to the
packages it synthesizes (even though it will propagate errors in
imported packages). Fix this by copying the error from the top-level
package into the synthesized test package while we're copying
everything else.

For #62067.

Change-Id: Icd563a3d9912256b53afd998050995e5260ebe5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/558637
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>

diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go
index 5f0be712556459fa512e063f76e1dd9f2d534d5a..4cac7ba432b58cbc942e614bf2088fe93403defb 100644 (file)
--- a/src/cmd/go/internal/load/test.go
+++ b/src/cmd/go/internal/load/test.go
@@ -176,8 +176,10 @@ func TestPackagesAndErrors(ctx context.Context, done func(), opts PackageOpts, p
        if len(p.TestGoFiles) > 0 || p.Name == "main" || cover != nil && cover.Local {
                ptest = new(Package)
                *ptest = *p
-               ptest.Error = ptestErr
-               ptest.Incomplete = incomplete
+               if ptest.Error == nil {
+                       ptest.Error = ptestErr
+               }
+               ptest.Incomplete = ptest.Incomplete || incomplete
                ptest.ForTest = p.ImportPath
                ptest.GoFiles = nil
                ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...)

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 3f4f3accaa3529893b828736c74a9d2e9aad5002..2a83890a33c64d5b21d6ff4543acec0bf4d38880 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -703,7 +703,8 @@ func runTest(ctx context.Context, cmd *base.Command, args []string) {
 
        pkgOpts := load.PackageOpts{ModResolveTests: true}
        pkgs = load.PackagesAndErrors(ctx, pkgOpts, pkgArgs)
-       load.CheckPackageErrors(pkgs)
+       // We *don't* call load.CheckPackageErrors here because we want to report
+       // loading errors as per-package test setup errors later.
        if len(pkgs) == 0 {
                base.Fatalf("no packages to test")
        }

diff --git a/src/cmd/go/testdata/script/test_flags.txt b/src/cmd/go/testdata/script/test_flags.txt
index 3f7964b0a725b234a95b9d7c2afbe7dc2bdaae08..7adf4e273c569a697760fe9cf460af5cf7c9b4ee 100644 (file)
--- a/src/cmd/go/testdata/script/test_flags.txt
+++ b/src/cmd/go/testdata/script/test_flags.txt
@@ -15,7 +15,8 @@ stdout '\Aok\s+example.com/x\s+[0-9.s]+\n\z'
 # Even though ./x looks like a package path, the real package should be
 # the implicit '.'.
 ! go test --answer=42 ./x
-stderr '^no Go files in '$PWD'$'
+stdout '^FAIL\t. \[build failed\]'
+stderr '^\.: no Go files in '$PWD'$'
 
 # However, *flags* that appear after unrecognized flags should still be
 # interpreted as flags, under the (possibly-erroneous) assumption that

diff --git a/src/cmd/go/testdata/script/test_setup_error.txt b/src/cmd/go/testdata/script/test_setup_error.txt
new file mode 100644 (file)
index 0000000..2999067
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_setup_error.txt
@@ -0,0 +1,74 @@
+[short] skip
+
+# Test that a loading error in a test file is reported as a "setup failed" error
+# and doesn't prevent running other tests.
+! go test -o=$devnull ./t1/p ./t
+stderr '# m/t1/p\n.*package x is not in std'
+stdout 'FAIL   m/t1/p \[setup failed\]'
+stdout 'ok     m/t'
+
+# Test a loading error in a test package, but not in the test file
+! go test -o=$devnull ./t2/p ./t
+stderr '# m/t2/p\n.*package x is not in std'
+stdout 'FAIL   m/t2/p \[setup failed\]'
+stdout 'ok     m/t'
+
+# Test a loading error in a package imported by a test file
+! go test -o=$devnull ./t3/p ./t
+stderr '# m/t3/p\n.*package x is not in std'
+stdout 'FAIL   m/t3/p \[setup failed\]'
+stdout 'ok     m/t'
+
+# Test a loading error in a package imported by a test package
+! go test -o=$devnull ./t4/p ./t
+stderr '# m/t4/p\n.*package x is not in std'
+stdout 'FAIL   m/t4/p \[setup failed\]'
+stdout 'ok     m/t'
+
+# Test that two loading errors are both reported.
+! go test -o=$devnull ./t1/p ./t2/p ./t
+stderr '# m/t1/p\n.*package x is not in std'
+stdout 'FAIL   m/t1/p \[setup failed\]'
+stderr '# m/t2/p\n.*package x is not in std'
+stdout 'FAIL   m/t2/p \[setup failed\]'
+stdout 'ok     m/t'
+
+# Finally, this one is a build error, but produced by cmd/go directly
+! go test -o=$devnull . ./t
+stderr '^\.: no Go files in '$PWD'$'
+stdout 'FAIL   . \[build failed\]'
+stdout 'ok     m/t'
+
+-- go.mod --
+module m
+go 1.21
+-- t/t_test.go --
+package t
+
+import "testing"
+
+func TestGood(t *testing.T) {}
+-- t1/p/p_test.go --
+package p
+
+import "x"
+-- t2/p/p_test.go --
+package p
+-- t2/p/p.go --
+package p
+
+import "x"
+-- t3/p/p_test.go --
+package p
+
+import "m/bad"
+-- t4/p/p_test.go --
+package p
+-- t4/p/p.go --
+package p
+
+import "m/bad"
+-- bad/bad.go --
+package bad
+
+import "x"