Skip to content

Commit

Permalink
dev: avoid re-generating logictest files if unnecessary
Browse files Browse the repository at this point in the history
Beforehand, running `dev testlogic` would always run all
tests regardless if they were modified, which wastes time.
This PR will check to see if logic test folders were modified
and only re-generates them if they are modified. It also
introduces a new flag, `--force-gen`, which will force
generate the logic tests.

Fixes: #94845
Release note: None
  • Loading branch information
Liam Gillies committed Sep 15, 2023
1 parent defdd31 commit 973eabb
Showing 1 changed file with 42 additions and 1 deletion.
43 changes: 42 additions & 1 deletion pkg/cmd/dev/testlogic.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package main

import (
"context"
"errors"
"fmt"
"log"
Expand All @@ -29,6 +30,7 @@ const (
configsFlag = "config"
showSQLFlag = "show-sql"
noGenFlag = "no-gen"
forceGenFlag = "force-gen"
flexTypesFlag = "flex-types"
workmemFlag = "default-workmem"
)
Expand Down Expand Up @@ -58,6 +60,7 @@ func makeTestLogicCmd(runE func(cmd *cobra.Command, args []string) error) *cobra
testLogicCmd.Flags().Bool(showSQLFlag, false, "show SQL statements/queries immediately before they are tested")
testLogicCmd.Flags().Bool(rewriteFlag, false, "rewrite test files using results from test run")
testLogicCmd.Flags().Bool(noGenFlag, false, "skip generating logic test files before running logic tests")
testLogicCmd.Flags().Bool(forceGenFlag, false, "skip generating logic test files before running logic tests")
testLogicCmd.Flags().Bool(streamOutputFlag, false, "stream test output during run")
testLogicCmd.Flags().Bool(stressFlag, false, "run tests under stress")
testLogicCmd.Flags().String(testArgsFlag, "", "additional arguments to pass to go test binary")
Expand Down Expand Up @@ -85,6 +88,7 @@ func (d *dev) testlogic(cmd *cobra.Command, commandLine []string) error {
timeout = mustGetFlagDuration(cmd, timeoutFlag)
verbose = mustGetFlagBool(cmd, vFlag)
noGen = mustGetFlagBool(cmd, noGenFlag)
forceGen = mustGetFlagBool(cmd, forceGenFlag)
showSQL = mustGetFlagBool(cmd, showSQLFlag)
count = mustGetFlagInt(cmd, countFlag)
stress = mustGetFlagBool(cmd, stressFlag)
Expand Down Expand Up @@ -127,7 +131,11 @@ func (d *dev) testlogic(cmd *cobra.Command, commandLine []string) error {
return err
}

if !noGen {
shouldGen, err := d.shouldGen(ctx)
if err != nil {
return err
}
if (shouldGen && !noGen) || forceGen {
err := d.generateLogicTest(cmd)
if err != nil {
return err
Expand Down Expand Up @@ -297,6 +305,39 @@ func (d *dev) testlogic(cmd *cobra.Command, commandLine []string) error {
return nil
}

// This function determines if any test_logic or execbuilder/testdata files were
// modified in the current branch, and if so, determines if we should re-generate logic tests.
func (d *dev) shouldGen(ctx context.Context) (bool, error) {
// List files changed against `master`.
remotes, err := d.exec.CommandContextSilent(ctx, "git", "remote", "-v")
if err != nil {
return true, err
}
var upstream string
for _, remote := range strings.Split(strings.TrimSpace(string(remotes)), "\n") {
if (strings.Contains(remote, "github.com/cockroachdb/cockroach") || strings.Contains(remote, "github.com:cockroachdb/cockroach")) && strings.HasSuffix(remote, "(fetch)") {
upstream = strings.Fields(remote)[0]
break
}
}
if upstream == "" {
return true, fmt.Errorf("could not find git upstream, run `git remote add upstream [email protected]:cockroachdb/cockroach.git`")
}

baseBytes, err := d.exec.CommandContextSilent(ctx, "git", "merge-base", fmt.Sprintf("%s/master", upstream), "HEAD")
if err != nil {
return true, err
}
base := strings.TrimSpace(string(baseBytes))

changedFiles, err := d.exec.CommandContextSilent(ctx, "git", "diff", "--no-ext-diff", "--name-only", base, "--", "**/logic_test/**", "**/execbuilder/testdata/**")
if err != nil {
return true, err
}
trimmedOutput := strings.TrimSpace(string(changedFiles))
return trimmedOutput != "", nil
}

// We know that the regular expressions for files should not contain whitespace
// because the file names do not contain whitespace. We support users passing
// whitespace separated regexps for multiple files.
Expand Down

0 comments on commit 973eabb

Please sign in to comment.