]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: test that each script test increments at least one counter
authorMichael Matloob <matloob@golang.org>
Thu, 29 Feb 2024 22:34:59 +0000 (17:34 -0500)
committerMichael Matloob <matloob@golang.org>
Thu, 7 Mar 2024 18:33:17 +0000 (18:33 +0000)
Add code that will set a scriptGoInvoked bit for the testing.TB when
it invokes the go command. If the go command was invoked, make sure
that at least one counter was incremented.

Also add the counters cmd/go/gomodcache-entry-relative,
cmd/go/gopath-entry-relative, and cmd/go/invalid-toolchain-in-file so
we can increment counters when a test errors out before the flag
subcommand counters are processed. This enforces the invariant that at
least one counter is incremented by every test that invokes the go
command.

Add the counter cmd/go/exec-go-toolchain for when a toolchain switch
happens.

Add cmd/go/subcommand:help for invoking help without arguments and
cmd/go/help-unknown-topic for when an unknown command is provided
to help.

Change-Id: Id90f2bbe4c7e89b846da00ec1ed9595ece2b269c
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/568259
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/go/counters_test.go
src/cmd/go/internal/base/base.go
src/cmd/go/internal/help/help.go
src/cmd/go/internal/modfetch/cache.go
src/cmd/go/internal/toolchain/select.go
src/cmd/go/internal/toolchain/switch.go
src/cmd/go/main.go
src/cmd/go/script_test.go
src/cmd/go/scriptcmds_test.go
src/cmd/go/testdata/counters.txt

index 04135979248b8065a64a721d91ec2ae611e80682..5e2f7cbf0e600685c6563142881abbb887d4916e 100644 (file)
@@ -29,11 +29,22 @@ func TestCounterNamesUpToDate(t *testing.T) {
        counters = append(counters, "cmd/go/flag:C", "cmd/go/subcommand:unknown")
        counters = append(counters, flagscounters("cmd/go/flag:", *flag.CommandLine)...)
 
+       // Add help (without any arguments) as a special case. cmdcounters adds go help <cmd>
+       // for all subcommands, but it's also valid to invoke go help without any arguments.
+       counters = append(counters, "cmd/go/subcommand:help")
        for _, cmd := range base.Go.Commands {
                counters = append(counters, cmdcounters(nil, cmd)...)
        }
-       cstr := []byte(strings.Join(counters, "\n") + "\n")
 
+       counters = append(counters, base.RegisteredCounterNames()...)
+       for _, c := range counters {
+               const counterPrefix = "cmd/go"
+               if !strings.HasPrefix(c, counterPrefix) {
+                       t.Fatalf("registered counter %q does not start with %q", c, counterPrefix)
+               }
+       }
+
+       cstr := []byte(strings.Join(counters, "\n") + "\n")
        const counterNamesFile = "testdata/counters.txt"
        old, err := os.ReadFile(counterNamesFile)
        if err != nil {
index 2171d139096a343c708450c613d80e923d8a7da4..73082df763bea79d956639845d532a7575bf66ef 100644 (file)
@@ -14,11 +14,14 @@ import (
        "os"
        "os/exec"
        "reflect"
+       "sort"
        "strings"
        "sync"
 
        "cmd/go/internal/cfg"
        "cmd/go/internal/str"
+
+       "golang.org/x/telemetry/counter"
 )
 
 // A Command is an implementation of a go command
@@ -221,3 +224,24 @@ func RunStdin(cmdline []string) {
 // Usage is the usage-reporting function, filled in by package main
 // but here for reference by other packages.
 var Usage func()
+
+var counterNames = map[string]bool{}
+
+// NewCounter registers a new counter. It must be called from an init function
+// or global variable initializer.
+func NewCounter(name string) *counter.Counter {
+       if counterNames[name] {
+               panic(fmt.Errorf("counter %q initialized twice", name))
+       }
+       counterNames[name] = true
+       return counter.New(name)
+}
+
+func RegisteredCounterNames() []string {
+       var names []string
+       for name := range counterNames {
+               names = append(names, name)
+       }
+       sort.Strings(names)
+       return names
+}
index 501f08eb2d63cbbdbc3f901dd62fe0ea542a069b..22a39ee40a8ecf8bd734693c5d3943ad41c626ff 100644 (file)
@@ -18,6 +18,8 @@ import (
        "cmd/go/internal/base"
 )
 
+var counterErrorHelpUnknownTopic = base.NewCounter("cmd/go/error:help-unknown-topic")
+
 // Help implements the 'help' command.
 func Help(w io.Writer, args []string) {
        // 'go help documentation' generates doc.go.
@@ -57,6 +59,7 @@ Args:
                if i > 0 {
                        helpSuccess += " " + strings.Join(args[:i], " ")
                }
+               counterErrorHelpUnknownTopic.Inc()
                fmt.Fprintf(os.Stderr, "go help %s: unknown help topic. Run '%s'.\n", strings.Join(args, " "), helpSuccess)
                base.SetExitStatus(2) // failed at 'go help cmd'
                base.Exit()
index 5a727c6dfa6d77faafa5a9bfe19b83a0e9c3a50f..c9364783af46c6397a1edab8c0c3c2a51da723e4 100644 (file)
@@ -777,6 +777,8 @@ func rewriteVersionList(ctx context.Context, dir string) (err error) {
 var (
        statCacheOnce sync.Once
        statCacheErr  error
+
+       counterErrorGOMODCACHEEntryRelative = base.NewCounter("cmd/go/error:gomodcache-entry-relative")
 )
 
 // checkCacheDir checks if the directory specified by GOMODCACHE exists. An
@@ -788,6 +790,7 @@ func checkCacheDir(ctx context.Context) error {
                return fmt.Errorf("module cache not found: neither GOMODCACHE nor GOPATH is set")
        }
        if !filepath.IsAbs(cfg.GOMODCACHE) {
+               counterErrorGOMODCACHEEntryRelative.Inc()
                return fmt.Errorf("GOMODCACHE entry is relative; must be absolute path: %q.\n", cfg.GOMODCACHE)
        }
 
index dcf3be92ccd84cf40cdd55ce9ada8a6bf8bd1e22..661a48317fb45836544332ec59d8523416c31f85 100644 (file)
@@ -81,6 +81,8 @@ func FilterEnv(env []string) []string {
        return out
 }
 
+var counterErrorInvalidToolchainInFile = base.NewCounter("cmd/go/error:invalid-toolchain-in-file")
+
 // Select invokes a different Go toolchain if directed by
 // the GOTOOLCHAIN environment variable or the user's configuration
 // or go.mod file.
@@ -174,6 +176,7 @@ func Select() {
                                // has a suffix like "go1.21.1-foo" and toolchain is "go1.21.1".)
                                toolVers := gover.FromToolchain(toolchain)
                                if toolVers == "" || (!strings.HasPrefix(toolchain, "go") && !strings.Contains(toolchain, "-go")) {
+                                       counterErrorInvalidToolchainInFile.Inc()
                                        base.Fatalf("invalid toolchain %q in %s", toolchain, base.ShortPath(file))
                                }
                                if gover.Compare(toolVers, minVers) > 0 {
@@ -230,9 +233,12 @@ func Select() {
                base.Fatalf("invalid GOTOOLCHAIN %q", gotoolchain)
        }
 
+       counterSelectExec.Inc()
        Exec(gotoolchain)
 }
 
+var counterSelectExec = base.NewCounter("cmd/go/select-exec")
+
 // TestVersionSwitch is set in the test go binary to the value in $TESTGO_VERSION_SWITCH.
 // Valid settings are:
 //
index 2c6a2b8f4363aa973f02261274ce75c99195fbca..06819c54679d0ea6681fb6a1f3936cc9088754bb 100644 (file)
@@ -98,10 +98,13 @@ func (s *Switcher) Switch(ctx context.Context) {
        }
 
        fmt.Fprintf(os.Stderr, "go: %v requires go >= %v; switching to %v\n", s.TooNew.What, s.TooNew.GoVersion, tv)
+       counterSwitchExec.Inc()
        Exec(tv)
        panic("unreachable")
 }
 
+var counterSwitchExec = base.NewCounter("cmd/go/switch-exec")
+
 // SwitchOrFatal attempts a toolchain switch based on the information in err
 // and otherwise falls back to base.Fatal(err).
 func SwitchOrFatal(ctx context.Context, err error) {
index c1433b47ad238253335aa1de1ec09ef1f0d84d59..dbb581d2797f2d06a59c4e391de5c6e5088e0387 100644 (file)
@@ -90,6 +90,8 @@ func init() {
 
 var _ = go11tag
 
+var counterErrorGOPATHEntryRelative = base.NewCounter("cmd/go/error:gopath-entry-relative")
+
 func main() {
        log.SetFlags(0)
        TelemetryStart() // Open the telemetry counter file so counters can be written to it.
@@ -107,6 +109,7 @@ func main() {
 
        cfg.CmdName = args[0] // for error messages
        if args[0] == "help" {
+               counter.Inc("cmd/go/subcommand:" + strings.Join(append([]string{"help"}, args[1:]...), "-"))
                help.Help(os.Stdout, args[1:])
                return
        }
@@ -145,6 +148,7 @@ func main() {
                                        // Instead of dying, uninfer it.
                                        cfg.BuildContext.GOPATH = ""
                                } else {
+                                       counterErrorGOPATHEntryRelative.Inc()
                                        fmt.Fprintf(os.Stderr, "go: GOPATH entry is relative; must be absolute path: %q.\nFor more details see: 'go help gopath'\n", p)
                                        os.Exit(2)
                                }
index 6efa9217de098b3403155c84e35e60f2261e3d30..6daa5d9e9abf1313918893fa27c47ce44129018f 100644 (file)
@@ -410,6 +410,11 @@ func checkCounters(t *testing.T, telemetryDir string) {
                }
        })
        counters := readCounters(t, telemetryDir)
+       if _, ok := scriptGoInvoked.Load(testing.TB(t)); ok {
+               if len(counters) == 0 {
+                       t.Fatal("go was invoked but no counters were incremented")
+               }
+       }
        for name := range counters {
                if !allowedCounters[name] {
                        t.Fatalf("incremented counter %q is not in testdata/counters.txt. "+
index db5e6cafdafe51d5a94f97e0841f1c3e9fa62a27..4ddf7ee654c223082f2a88428487bbc7708a5cfd 100644 (file)
@@ -13,6 +13,7 @@ import (
        "os"
        "os/exec"
        "strings"
+       "sync"
        "time"
 )
 
@@ -69,9 +70,23 @@ func scriptCC(cmdExec script.Cmd) script.Cmd {
                })
 }
 
+var scriptGoInvoked sync.Map // testing.TB → go command was invoked
+
 // scriptGo runs the go command.
 func scriptGo(cancel func(*exec.Cmd) error, waitDelay time.Duration) script.Cmd {
-       return script.Program(testGo, cancel, waitDelay)
+       cmd := script.Program(testGo, cancel, waitDelay)
+       // Inject code to update scriptGoInvoked before invoking the Go command.
+       return script.Command(*cmd.Usage(), func(state *script.State, s ...string) (script.WaitFunc, error) {
+               t, ok := tbFromContext(state.Context())
+               if !ok {
+                       return nil, errors.New("script Context unexpectedly missing testing.TB key")
+               }
+               _, dup := scriptGoInvoked.LoadOrStore(t, true)
+               if !dup {
+                       t.Cleanup(func() { scriptGoInvoked.Delete(t) })
+               }
+               return cmd.Run(state, s...)
+       })
 }
 
 // scriptStale checks that the named build targets are stale.
index 5e1a565cfdd832ece7bcfdc01f6cc31fd4710054..9fd1323293e4035c15e5571af427747319be5428 100644 (file)
@@ -40,6 +40,7 @@ cmd/go/flag:test.v
 cmd/go/flag:testsum
 cmd/go/flag:testwork
 cmd/go/flag:update
+cmd/go/subcommand:help
 cmd/go/subcommand:bug
 cmd/go/flag:bug-C
 cmd/go/flag:bug-v
@@ -659,3 +660,9 @@ cmd/go/subcommand:help-private
 cmd/go/subcommand:help-testflag
 cmd/go/subcommand:help-testfunc
 cmd/go/subcommand:help-vcs
+cmd/go/error:gomodcache-entry-relative
+cmd/go/error:gopath-entry-relative
+cmd/go/error:help-unknown-topic
+cmd/go/error:invalid-toolchain-in-file
+cmd/go/select-exec
+cmd/go/switch-exec