Skip to content

Commit

Permalink
notify: Wait on subprocess so that ProcessState is populated
Browse files Browse the repository at this point in the history
In the Notify code path bazel-watcher uses `os.ProcessState` to determine if a process
is still running. However, it never calls `Wait` or `Run`, like
it does in the normal codepath. This patch adds an async goroutine that `Wait`s for the
subprocess to exisit so that `ProcessState` is populated correctly.

Without this change subprocesses that exit result in bazel-watcher never detecting that
the process is dead and trying to write to it continuously, which causes broken pipe
errors in macOS/Linux.
  • Loading branch information
dmiller-figma committed Nov 20, 2024
1 parent 778a4da commit ffa6c14
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 0 deletions.
11 changes: 11 additions & 0 deletions internal/e2e/notify/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test")

go_bazel_test(
name = "notify_test",
srcs = ["notify_test.go"],
deps = [
"//internal/e2e",
"@com_github_gorilla_websocket//:websocket",
"@io_bazel_rules_go//go/tools/bazel_testing:go_default_library",
],
)
114 changes: 114 additions & 0 deletions internal/e2e/notify/notify_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package notify

import (
"testing"

"github.com/bazelbuild/bazel-watcher/internal/e2e"
"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
)

const mainFiles = `
-- BUILD --
sh_binary(
name = "crash_script",
srcs = ["crash_script.sh"],
)
sh_binary(
name = "crash_script_notify",
srcs = ["crash_script_notify.sh"],
data = ["message.txt"],
tags = [
"ibazel_notify_changes",
],
)
-- crash_script.sh --
#!/bin/bash
echo "Starting crash script"
sleep 2
echo "Exiting after 2 seconds"
exit 1
-- message.txt --
HELLO
-- crash_script_notify.sh --
#!/bin/bash
# list all files in current directory
ls -l
# Filename to monitor
filename="message.txt"
# Read the initial content of the file
if [[ ! -f "$filename" ]]; then
echo "Error: File $filename does not exist."
exit 1
fi
content=$(cat "$filename")
# Function to check for CRASH in the file
check_for_crash() {
if grep -q "CRASH" "$filename"; then
echo "CRASH detected in $filename"
exit 1
fi
}
# Print the current content of the file
print_content() {
echo "Message: $filename:"
echo "$content"
}
# Monitor stdin for trigger message
while true; do
print_content
read -t 5 input
if [[ $? -eq 0 ]]; then
# Input received
if [[ "$input" == "IBAZEL_BUILD_COMPLETED SUCCESS" ]]; then
# Re-read the file
if [[ ! -f "$filename" ]]; then
echo "Error: File $filename does not exist."
continue
fi
content=$(cat "$filename")
check_for_crash
fi
fi
sleep 1
done
`

func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Main: mainFiles,
})
}

func TestNotifySubprocessCrashes(t *testing.T) {
ibazel := e2e.SetUp(t)
ibazel.Run([]string{}, "//:crash_script_notify")
defer ibazel.Kill()

// Expect the initial content of the file
ibazel.ExpectOutput("Message: message.txt:")
ibazel.ExpectOutput("HELLO")

// It reacts to changes
e2e.MustWriteFile(t, "message.txt", "WORLD")
ibazel.ExpectOutput("WORLD")

// This causes the subprocess to crash
e2e.MustWriteFile(t, "message.txt", "CRASH")
ibazel.ExpectOutput("CRASH detected in message.txt")

// The subprocess is restarted, and the next change is detected
e2e.MustWriteFile(t, "message.txt", "FIXED")
ibazel.ExpectOutput("FIXED")
}
6 changes: 6 additions & 0 deletions internal/ibazel/command/notify_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ func (c *notifyCommand) Start() (*bytes.Buffer, error) {
return outputBuffer, err
}
log.Log("Starting...")
go func() {
err := c.pg.Wait()
if err != nil {
log.Errorf("Error waiting for process: %v", err)
}
}()
c.termSync = sync.Once{}
return outputBuffer, nil
}
Expand Down

0 comments on commit ffa6c14

Please sign in to comment.