cmd/go/internal/test: keep looking for go command flags after ambiguous test flag
Fixes #40763
Change-Id: I275970d1f8561414571a5b93e368d68fa052c60f
Reviewed-on: https://go-review.googlesource.com/c/go/+/248618
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
index 1ff34f7..4f0a892 100644
--- a/src/cmd/go/internal/test/testflag.go
+++ b/src/cmd/go/internal/test/testflag.go
@@ -214,9 +214,13 @@
explicitArgs := make([]string, 0, len(args))
inPkgList := false
+ afterFlagWithoutValue := false
for len(args) > 0 {
f, remainingArgs, err := cmdflag.ParseOne(&CmdTest.Flag, args)
+ wasAfterFlagWithoutValue := afterFlagWithoutValue
+ afterFlagWithoutValue = false // provisionally
+
if errors.Is(err, flag.ErrHelp) {
exitWithUsage()
}
@@ -233,10 +237,24 @@
if nf := (cmdflag.NonFlagError{}); errors.As(err, &nf) {
if !inPkgList && packageNames != nil {
// We already saw the package list previously, and this argument is not
- // a flag, so it — and everything after it — must be a literal argument
- // to the test binary.
- explicitArgs = append(explicitArgs, args...)
- break
+ // a flag, so it — and everything after it — must be either a value for
+ // a preceding flag or a literal argument to the test binary.
+ if wasAfterFlagWithoutValue {
+ // This argument could syntactically be a flag value, so
+ // optimistically assume that it is and keep looking for go command
+ // flags after it.
+ //
+ // (If we're wrong, we'll at least be consistent with historical
+ // behavior; see https://golang.org/issue/40763.)
+ explicitArgs = append(explicitArgs, nf.RawArg)
+ args = remainingArgs
+ continue
+ } else {
+ // This argument syntactically cannot be a flag value, so it must be a
+ // positional argument, and so must everything after it.
+ explicitArgs = append(explicitArgs, args...)
+ break
+ }
}
inPkgList = true
@@ -272,6 +290,9 @@
explicitArgs = append(explicitArgs, nd.RawArg)
args = remainingArgs
+ if !nd.HasValue {
+ afterFlagWithoutValue = true
+ }
continue
}
diff --git a/src/cmd/go/testdata/script/test_flags.txt b/src/cmd/go/testdata/script/test_flags.txt
index d38e37f..63385e6 100644
--- a/src/cmd/go/testdata/script/test_flags.txt
+++ b/src/cmd/go/testdata/script/test_flags.txt
@@ -10,7 +10,7 @@
! stderr .
# For backward-compatibility with previous releases of the 'go' command,
-# arguments that appear after unrecognized flags should not be treated
+# arguments that appear after unrecognized flags should not be treated
# as packages, even if they are unambiguously not arguments to flags.
# Even though ./x looks like a package path, the real package should be
# the implicit '.'.
@@ -18,6 +18,22 @@
stderr '^no Go files in .+$'
! stderr '/x'
+# However, *flags* that appear after unrecognized flags should still be
+# interpreted as flags, under the (possibly-erroneous) assumption that
+# unrecognized flags are non-boolean.
+
+go test -v -x ./x -timeout 24h -boolflag=true foo -timeout 25h
+stdout 'args: foo -timeout 25h'
+stdout 'timeout: 24h0m0s$' # -timeout is unambiguously not a flag, so the real flag wins.
+
+go test -v -x ./x -timeout 24h -boolflag foo -timeout 25h
+stdout 'args: foo -test\.timeout=25h0m0s' # For legacy reasons, '-timeout ' is erroneously rewritten to -test.timeout; see https://golang.org/issue/40763.
+stdout 'timeout: 24h0m0s$' # Actual flag wins.
+
+go test -v -x ./x -timeout 24h -stringflag foo -timeout 25h
+stdout 'args: $'
+stdout 'timeout: 25h0m0s$' # Later flag wins.
+
# An explicit '-outputdir=' argument should set test.outputdir
# to the 'go' command's working directory, not zero it out
# for the test binary.
@@ -30,23 +46,23 @@
# with the 'test.' prefix in the GOFLAGS entry...
env GOFLAGS='-test.timeout=24h0m0s -count=1'
go test -v -x ./x
-stdout '.*: 24h0m0s$'
+stdout 'timeout: 24h0m0s$'
stderr '-test.count=1'
# ...or without.
env GOFLAGS='-timeout=24h0m0s -count=1'
go test -v -x ./x
-stdout '.*: 24h0m0s$'
+stdout 'timeout: 24h0m0s$'
stderr '-test.count=1'
# Arguments from the command line should override GOFLAGS...
go test -v -x -timeout=25h0m0s ./x
-stdout '.*: 25h0m0s$'
+stdout 'timeout: 25h0m0s$'
stderr '-test.count=1'
# ...even if they use a different flag name.
go test -v -x -test.timeout=26h0m0s ./x
-stdout '.*: 26h0m0s$'
+stdout 'timeout: 26h0m0s$'
stderr '-test\.timeout=26h0m0s'
! stderr 'timeout=24h0m0s'
stderr '-test.count=1'
@@ -99,11 +115,18 @@
import (
"flag"
+ "strings"
"testing"
)
var _ = flag.String("usage_message", "", "dummy flag to check usage message")
+var boolflag = flag.Bool("boolflag", false, "ignored boolean flag")
+var stringflag = flag.String("stringflag", "", "ignored string flag")
func TestLogTimeout(t *testing.T) {
- t.Log(flag.Lookup("test.timeout").Value)
+ t.Logf("timeout: %v", flag.Lookup("test.timeout").Value)
+}
+
+func TestLogArgs(t *testing.T) {
+ t.Logf("args: %s", strings.Join(flag.Args(), " "))
}
