From a42d5c993db17f7722c1ee22c81c474e7fbfc92d Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 30 Sep 2021 10:33:32 -0400 Subject: [PATCH] reduce: show original test case output The `reduce` tool now shows the original test case output in verbose mode if it is not found to be interesting. This makes it easier for a user to determine why a reproduction is not producing output that matches the `--contains` regex. Release note: None --- pkg/cmd/reduce/main.go | 13 +++++++++---- pkg/testutils/reduce/reduce.go | 16 +++++++++++----- pkg/testutils/reduce/reduce_test.go | 6 +++--- pkg/testutils/reduce/reducesql/reducesql_test.go | 6 +++--- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/reduce/main.go b/pkg/cmd/reduce/main.go index 124e90920fb4..8307676169e4 100644 --- a/pkg/cmd/reduce/main.go +++ b/pkg/cmd/reduce/main.go @@ -52,7 +52,7 @@ var ( flags = flag.NewFlagSet(os.Args[0], flag.ExitOnError) binary = flags.String("binary", "./cockroach", "path to cockroach binary") file = flags.String("file", "", "the path to a file containing a SQL query to reduce") - verbose = flags.Bool("v", false, "print progress to standard output") + verbose = flags.Bool("v", false, "print progress to standard output and the original test case output if it is not interesting") contains = flags.String("contains", "", "error regex to search for") unknown = flags.Bool("unknown", false, "print unknown types during walk") workers = flags.Int("goroutines", goroutines, "number of worker goroutines (defaults to NumCPU/3") @@ -141,7 +141,7 @@ func reduceSQL( chunkReducer = reducesql.NewSQLChunkReducer(chunkReductions) } - interesting := func(ctx context.Context, sql string) bool { + isInteresting := func(ctx context.Context, sql string) (interesting bool, logOriginalHint func()) { // Disable telemetry and license generation. cmd := exec.CommandContext(ctx, binary, "demo", @@ -162,13 +162,18 @@ func reduceSQL( case errors.HasType(err, (*os.PathError)(nil)): log.Fatal(err) } - return containsRE.Match(out) + if verbose { + logOriginalHint = func() { + fmt.Fprintf(logger, "output did not match regex %s:\n\n%s", contains, string(out)) + } + } + return containsRE.Match(out), logOriginalHint } out, err := reduce.Reduce( logger, inputSQL, - interesting, + isInteresting, workers, reduce.ModeInteresting, chunkReducer, diff --git a/pkg/testutils/reduce/reduce.go b/pkg/testutils/reduce/reduce.go index e63844f0f7d5..d55a2f285d58 100644 --- a/pkg/testutils/reduce/reduce.go +++ b/pkg/testutils/reduce/reduce.go @@ -56,8 +56,11 @@ const ( type State interface{} // InterestingFn returns true if the string triggers the target test failure. It -// should be context-aware and stop work if the context is canceled. -type InterestingFn func(context.Context, string) bool +// should be context-aware and stop work if the context is canceled. It can +// return a function which will be called if the original test case is not +// interesting. The function should log a hint that will help the user +// understand why the original test case is not interesting. +type InterestingFn func(context.Context, string) (_ bool, logOriginalHint func()) // Mode is an enum specifying how to determine if forward progress was made. type Mode int @@ -89,7 +92,10 @@ func Reduce( } ctx, cancel := context.WithCancel(context.Background()) defer cancel() - if !isInteresting(ctx, originalTestCase) { + if interesting, logHint := isInteresting(ctx, originalTestCase); !interesting { + if logHint != nil { + logHint() + } return "", errors.New("original test case not interesting") } @@ -162,7 +168,7 @@ func Reduce( for i := 0; i < numGoroutines; i++ { g.GoCtx(func(ctx context.Context) error { for vs := range variants { - if isInteresting(ctx, vs.file) { + if interesting, _ := isInteresting(ctx, vs.file); interesting { // Wait for the previous test to finish. select { case <-ctx.Done(): @@ -327,7 +333,7 @@ func attemptChunkReduction( end := rand.Intn(chunkReducer.NumSegments()-start) + start + 1 localReduced := chunkReducer.DeleteSegments(start, end) - if isInteresting(ctx, localReduced) { + if interesting, _ := isInteresting(ctx, localReduced); interesting { reduced = localReduced log(logger, "\tchunk reduction: %d bytes\n", len(reduced)) failedAttempts = 0 diff --git a/pkg/testutils/reduce/reduce_test.go b/pkg/testutils/reduce/reduce_test.go index 3e44f43d742c..05c77f37ec10 100644 --- a/pkg/testutils/reduce/reduce_test.go +++ b/pkg/testutils/reduce/reduce_test.go @@ -52,11 +52,11 @@ var ( ) func isInterestingGo(contains string) reduce.InterestingFn { - return func(ctx context.Context, f string) bool { + return func(ctx context.Context, f string) (bool, func()) { _, err := parser.ParseExpr(f) if err == nil { - return false + return false, nil } - return strings.Contains(err.Error(), contains) + return strings.Contains(err.Error(), contains), nil } } diff --git a/pkg/testutils/reduce/reducesql/reducesql_test.go b/pkg/testutils/reduce/reducesql/reducesql_test.go index 95e201629384..85ffb994780b 100644 --- a/pkg/testutils/reduce/reducesql/reducesql_test.go +++ b/pkg/testutils/reduce/reducesql/reducesql_test.go @@ -38,7 +38,7 @@ func TestReduceSQL(t *testing.T) { } func isInterestingSQL(contains string) reduce.InterestingFn { - return func(ctx context.Context, f string) bool { + return func(ctx context.Context, f string) (bool, func()) { args := base.TestServerArgs{ Insecure: true, } @@ -67,8 +67,8 @@ func isInterestingSQL(contains string) reduce.InterestingFn { } _, err = db.Exec(ctx, f) if err == nil { - return false + return false, nil } - return strings.Contains(err.Error(), contains) + return strings.Contains(err.Error(), contains), nil } }