Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unit test/ci: report when unit test is using either TestServer/TestCluster API #111366

Open
srosenberg opened this issue Sep 27, 2023 · 3 comments
Labels
A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team

Comments

@srosenberg
Copy link
Member

srosenberg commented Sep 27, 2023

To enable better tracking of flaky tests which rely on TestServer/TestCluster API [1], we'd like to surface a new parameter in GH issues. E.g., [2] is a unit test which failed during the nightly stress run; this test happens to use TestCluster API. See [3] for private slack discussion.

[1] https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/3155427384/TestServer+and+TestCluster
[2] #110187
[3] https://cockroachlabs.slack.com/archives/C023S0V4YEB/p1695754554482619

Jira issue: CRDB-31874

@srosenberg srosenberg added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-testing Testing tools and infrastructure T-testeng TestEng Team labels Sep 27, 2023
@blathers-crl
Copy link

blathers-crl bot commented Sep 27, 2023

cc @cockroachdb/test-eng

@srosenberg
Copy link
Member Author

srosenberg commented Sep 27, 2023

I've made a quick & dirty patch to extract the info. using call stack frames. The diff is below. I am also attaching the resulting tests, which use TestServer/TestCluster, at this time, on master.

diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
index f5554d65b0c..97097591cc3 100644
--- a/pkg/server/testserver.go
+++ b/pkg/server/testserver.go
@@ -19,6 +19,7 @@ import (
        "net/http"
        "os"
        "path/filepath"
+       "runtime"
        "strconv"
        "strings"
        "time"
@@ -63,6 +64,7 @@ import (
        "github.com/cockroachdb/cockroach/pkg/upgrade/upgradebase"
        "github.com/cockroachdb/cockroach/pkg/util"
        "github.com/cockroachdb/cockroach/pkg/util/admission"
+       "github.com/cockroachdb/cockroach/pkg/util/allstacks"
        "github.com/cockroachdb/cockroach/pkg/util/hlc"
        "github.com/cockroachdb/cockroach/pkg/util/log"
        "github.com/cockroachdb/cockroach/pkg/util/metric"
@@ -2311,6 +2313,64 @@ func (testServerFactoryImpl) PrepareRangeTestServer(srv interface{}) error {
        return nil
 }

+// getTestName returns the calling test function name, returning an empty string
+// if not found. The number of calls up the call stack is limited.
+func getTestName() string {
+       pcs := make([]uintptr, 20)
+       n := runtime.Callers(2, pcs)
+       frames := runtime.CallersFrames(pcs[:n])
+       match := func(s string) bool {
+               return strings.Contains(s, ".Test") || strings.Contains(s, ".Benchmark") ||
+                       strings.Contains(s, ".Example")
+       }
+
+       for {
+               frame, more := frames.Next()
+               fxn := frame.Function
+               if match(fxn) {
+                       return fxn
+               }
+               if !more {
+                       break
+               }
+       }
+       // No luck, the test must be on a different stack; e.g., datadriven tests use a goroutine for each subtest.
+       // Let's grab all the stacks and match anything that looks like a test function other than TestMain.
+       buf := allstacks.Get()
+       for _, g := range strings.Split(string(buf), "\n\n") {
+               sl := strings.SplitN(g, "\n", 2)
+               if len(sl) != 2 {
+                       continue
+               }
+               stack := strings.TrimSpace(sl[1])
+               // N.B. skip stack if it contains TestMain, as that's the entry point for the test binary.
+               if !strings.Contains(stack, ".TestMain(") && match(stack) {
+                       lines := strings.Split(stack, "\n")
+                       for _, line := range lines {
+                               if match(line) {
+                                       i := strings.Index(line, ".Test")
+                                       if i == -1 {
+                                               i = strings.Index(line, ".Benchmark")
+                                       }
+                                       if i == -1 {
+                                               i = strings.Index(line, ".Example")
+                                       }
+                                       if i != -1 {
+                                               line = line[i+1:]
+                                               i = strings.Index(line, "(")
+                                               if i != -1 {
+                                                       line = line[:i]
+
+                                                       return line
+                                               }
+                                       }
+                               }
+                       }
+               }
+       }
+       return ""
+}
+
 // New is part of TestServerFactory interface.
 func (testServerFactoryImpl) New(params base.TestServerArgs) (interface{}, error) {
        if params.Knobs.JobsTestingKnobs != nil {
@@ -2320,6 +2380,9 @@ func (testServerFactoryImpl) New(params base.TestServerArgs) (interface{}, error
                        }
                }
        }
+       // determine the top-level test function name
+       testName := getTestName()
+       fmt.Println("TestServerFactoryImpl::New", testName)

        cfg := makeTestConfigFromParams(params)
        ts := &testServer{Cfg: &cfg, params: params}

@srosenberg
Copy link
Member Author

srosenberg commented Sep 27, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team
Projects
No open projects
Status: Triage
Development

No branches or pull requests

1 participant