Skip to content

Commit

Permalink
reduce: show original test case output
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mgartner committed Sep 30, 2021
1 parent c2af5b8 commit a42d5c9
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 15 deletions.
13 changes: 9 additions & 4 deletions pkg/cmd/reduce/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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",
Expand All @@ -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,
Expand Down
16 changes: 11 additions & 5 deletions pkg/testutils/reduce/reduce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/testutils/reduce/reduce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
6 changes: 3 additions & 3 deletions pkg/testutils/reduce/reducesql/reducesql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
}
}

0 comments on commit a42d5c9

Please sign in to comment.