From c3bdadf174fc1f17c189017c538d48fcc940bb71 Mon Sep 17 00:00:00 2001 From: Vasilios Pantazopoulos Date: Thu, 22 Oct 2020 00:30:32 -0400 Subject: [PATCH] Output Runner will only run each unique command+args once (#422) --- e2e/ibazel.go | 40 +++++++++++++++ e2e/output_runner/output_runner_test.go | 59 ++++++++++++++++++---- ibazel/output_runner/output_runner.go | 13 +++-- ibazel/output_runner/output_runner_test.go | 19 ++++--- 4 files changed, 112 insertions(+), 19 deletions(-) diff --git a/e2e/ibazel.go b/e2e/ibazel.go index 5e5c40aa..49f192ad 100644 --- a/e2e/ibazel.go +++ b/e2e/ibazel.go @@ -152,6 +152,46 @@ func (i *IBazelTester) GetIBazelError() string { return string(b) } +func (i *IBazelTester) ExpectFixCommands(want []string, delay ...time.Duration) { + i.t.Helper() + + i.checkExit() + + d := defaultDelay + if len(delay) == 1 { + d = delay[0] + } + + logRegexp := regexp.MustCompile("Executing command: `([^`]+)`") + + stopAt := time.Now().Add(d) + for time.Now().Before(stopAt) { + time.Sleep(5 * time.Millisecond) + + if len(logRegexp.FindAllStringSubmatch(i.GetIBazelError(), -1)) >= len(want) { + break + } + } + + matches := logRegexp.FindAllStringSubmatch(i.GetIBazelError(), -1) + if len(matches) != len(want) { + i.t.Errorf("Expected %v commands to be executed, but found %v.", len(want), len(matches)) + i.t.Errorf("Stderr: [%v]\niBazelStderr: [%v]", i.GetError(), i.GetIBazelError()) + } else { + var actual []string + for ind := range matches { + actual = append(actual, matches[ind][1]) + } + for ind, expected := range want { + if actual[ind] != expected { + i.t.Errorf("Expected the commands to have been executed in order:\nWanted [\n%s\n], got [\n%s\n]", + strings.Join(want, "\n"), strings.Join(actual, "\n")) + i.t.Errorf("Stderr: [%v]\niBazelStderr: [%v]", i.GetError(), i.GetIBazelError()) + } + } + } +} + func (i *IBazelTester) Expect(want string, stream func() string, history *string, delay time.Duration) { i.t.Helper() diff --git a/e2e/output_runner/output_runner_test.go b/e2e/output_runner/output_runner_test.go index 921e48c2..e159d917 100644 --- a/e2e/output_runner/output_runner_test.go +++ b/e2e/output_runner/output_runner_test.go @@ -12,11 +12,11 @@ import ( ) const mainFiles = ` --- defs.bzl -- +-- single/defs.bzl -- def fix_deps(): print("runacommand") --- BUILD -- -load("//:defs.bzl", "fix_deps") +-- single/BUILD -- +load("//single:defs.bzl", "fix_deps") fix_deps() @@ -29,10 +29,27 @@ sh_binary( name = "overwrite", srcs = ["overwrite.sh"], ) --- test.sh -- +-- single/test.sh -- printf "action" --- overwrite.sh -- +-- single/overwrite.sh -- printf "overwrite" +-- multiple/defs.bzl -- +def fix_deps(): + print("runcommand foo") + print("runcommand bar") + print("runcommand foo") + print("runcommand baz") +-- multiple/BUILD -- +load("//multiple:defs.bzl", "fix_deps") + +fix_deps() + +sh_binary( + name = "test", + srcs = ["test.sh"], +) +-- multiple/test.sh -- +printf "action" ` func TestMain(m *testing.M) { @@ -75,7 +92,7 @@ func TestOutputRunner(t *testing.T) { // First check that it doesn't run if there isn't a `.bazel_fix_commands.json` file. ibazel := e2e.NewIBazelTester(t) - ibazel.RunWithBazelFixCommands("//:overwrite") + ibazel.RunWithBazelFixCommands("//single:overwrite") // Ensure it prints out the banner. ibazel.ExpectIBazelError("Did you know") @@ -87,11 +104,11 @@ func TestOutputRunner(t *testing.T) { "args": ["%s"] }]`, sentinalFileName)) - e2e.MustWriteFile(t, "overwrite.sh", ` + e2e.MustWriteFile(t, "single/overwrite.sh", ` printf "overwrite1" `) - ibazel.RunWithBazelFixCommands("//:overwrite") + ibazel.RunWithBazelFixCommands("//single:overwrite") ibazel.ExpectOutput("overwrite1") checkSentinel(t, sentinelFile, "The first run should create a sentinel.") @@ -101,7 +118,7 @@ printf "overwrite1" // Invoke the test a 2nd time to ensure it works over multiple separate // invocations of ibazel. ibazel = e2e.NewIBazelTester(t) - ibazel.RunWithBazelFixCommands("//:overwrite") + ibazel.RunWithBazelFixCommands("//single:overwrite") ibazel.ExpectOutput("overwrite1") checkSentinel(t, sentinelFile, "The second run should create a sentinel.") @@ -122,13 +139,35 @@ def fix_deps(): checkNoSentinel(t, sentinelFile, "The third run should not create a sentinel.") } +func TestOutputRunnerUniqueCommandsOnly(t *testing.T) { + e2e.SetExecuteBit(t) + + e2e.MustWriteFile(t, ".bazel_fix_commands.json", ` + [{ + "regex": "^.*runcommand (.*)$", + "command": "echo", + "args": ["$1"] + }]`) + + + ibazel := e2e.NewIBazelTester(t) + ibazel.RunWithBazelFixCommands("//multiple:test") + defer ibazel.Kill() + + ibazel.ExpectFixCommands([]string{ + "echo foo", + "echo bar", + "echo baz", + }) +} + func TestNotifyWhenInvalidConfig(t *testing.T) { e2e.MustWriteFile(t, ".bazel_fix_commands.json", ` invalid json file `) ibazel := e2e.SetUp(t) - ibazel.RunWithBazelFixCommands("//:test") + ibazel.RunWithBazelFixCommands("//single:test") defer ibazel.Kill() // It should run the program and print out an error that says your JSON is diff --git a/ibazel/output_runner/output_runner.go b/ibazel/output_runner/output_runner.go index e1993cc7..43c65e12 100644 --- a/ibazel/output_runner/output_runner.go +++ b/ibazel/output_runner/output_runner.go @@ -138,6 +138,7 @@ func (o *OutputRunner) readConfigs(configPath string) []Optcmd { func matchRegex(optcmd []Optcmd, output *bytes.Buffer) ([]string, []string, [][]string) { var commandLines, commands []string var args [][]string + distinctCommands := map[string]bool{} scanner := bufio.NewScanner(output) for scanner.Scan() { line := escapeCodeCleanerRegex.ReplaceAllLiteralString(scanner.Text(), "") @@ -145,9 +146,15 @@ func matchRegex(optcmd []Optcmd, output *bytes.Buffer) ([]string, []string, [][] re := regexp.MustCompile(oc.Regex) matches := re.FindStringSubmatch(line) if matches != nil && len(matches) >= 0 { - commandLines = append(commandLines, matches[0]) - commands = append(commands, convertArg(matches, oc.Command)) - args = append(args, convertArgs(matches, oc.Args)) + command := convertArg(matches, oc.Command) + cmdArgs := convertArgs(matches, oc.Args) + fullCmd := strings.Join(append([]string{command}, cmdArgs...), " ") + if _, found := distinctCommands[fullCmd]; !found { + commandLines = append(commandLines, matches[0]) + commands = append(commands, command) + args = append(args, cmdArgs) + distinctCommands[fullCmd] = true + } } } } diff --git a/ibazel/output_runner/output_runner_test.go b/ibazel/output_runner/output_runner_test.go index a548b34a..feadb971 100644 --- a/ibazel/output_runner/output_runner_test.go +++ b/ibazel/output_runner/output_runner_test.go @@ -91,25 +91,32 @@ func TestMatchRegex(t *testing.T) { buf := bytes.Buffer{} buf.WriteString("buildozer 'add deps test_dep1' //target1:target1\n") buf.WriteString("buildozer 'add deps test_dep2' //target2:target2\n") - buf.WriteString("buildifier 'cmd_nvm' //target_nvm:target_nvm\n") - buf.WriteString("not_a_match 'nvm' //target_nvm:target_nvm\n") + buf.WriteString("Check that imports in Go sources match importpath attributes in deps.\n") + buf.WriteString("Other build output which does not match things.\n") + // Duplicate matches, to be deduplicated. + buf.WriteString("Check that imports in Go sources match importpath attributes in deps.\n") + buf.WriteString("buildozer 'add deps test_dep2' //target2:target2\n") optcmd := []Optcmd{ + {Regex: "^Check that imports in Go sources match importpath attributes in deps.$", Command: "bazel", Args: []string{"run", "//:gazelle"}}, {Regex: "^(buildozer) '(.*)'\\s+(.*)$", Command: "$1", Args: []string{"$2", "$3"}}, - {Regex: "^(buildifier) '(.*)'\\s+(.*)$", Command: "test_cmd", Args: []string{"test_arg1", "test_arg2"}}, } _, commands, args := matchRegex(optcmd, &buf) - for idx, c := range []struct { + expected := []struct { cls string cs string as []string }{ {"buildozer 'add deps test_dep1' //target1:target1", "buildozer", []string{"add deps test_dep1", "//target1:target1"}}, {"buildozer 'add deps test_dep2' //target2:target2", "buildozer", []string{"add deps test_dep2", "//target2:target2"}}, - {"buildifier 'cmd_nvm' //target_nvm:target_nvm", "test_cmd", []string{"test_arg1", "test_arg2"}}, - } { + {"Check that imports in Go sources match importpath attributes in deps.", "bazel", []string{"run", "//:gazelle"}}, + } + if len(expected) != len(commands) { + t.Errorf("Did not receive expected number of commands:\nGot: %d\nWant: %d", len(commands), len(expected)) + } + for idx, c := range expected { if !reflect.DeepEqual(c.cs, commands[idx]) { t.Errorf("Commands not equal: %v\nGot: %v\nWant: %v", c.cls, commands[idx], c.cs)