nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: tilt-dev/tilt
Pull request #5888 (2022-07-08 18:31:12)

filewatch: fix a bug in error handling
Hello @milas,

Please review the following commits I made in branch nicks/issue5887:

4e52d618ea259b717a4276f55d49867cfe7e3cf6 (2022-07-08 14:30:56 -0400)
filewatch: fix a bug in error handling
fixes https://github.com/tilt-dev/tilt/issues/5887

Code review reminders, by giving a LGTM you attest that:

* Commits are adequately tested
* Code is easy to understand and conforms to style guides
* Incomplete code is marked with TODOs
* Code is suitably instrumented with logging and metrics


Patch:
diff --git a/internal/controllers/core/filewatch/controller.go b/internal/controllers/core/filewatch/controller.go
index 74c749be5..058703424 100644
--- a/internal/controllers/core/filewatch/controller.go
+++ b/internal/controllers/core/filewatch/controller.go
@@ -193,14 +193,17 @@ func (c *Controller) addOrReplace(ctx context.Context, name types.NamespacedName
 	}
 
 	ignoreMatcher := ignore.CreateFileChangeFilter(fw.Spec.Ignores)
+	startFileChangeLoop := false
 	notify, err := c.fsWatcherMaker(
 		append([]string{}, fw.Spec.WatchedPaths...),
 		ignoreMatcher,
 		logger.Get(ctx))
 	if err != nil {
-		status.Error = fmt.Sprintf("failed to initialize filesystem watch: %v", err)
+		status.Error = fmt.Sprintf("filewatch init: %v", err)
 	} else if err := notify.Start(); err != nil {
-		status.Error = fmt.Sprintf("failed to initialize filesystem watch: %v", err)
+		status.Error = fmt.Sprintf("filewatch init: %v", err)
+	} else {
+		startFileChangeLoop = true
 	}
 
 	if hasExisting {
@@ -208,7 +211,7 @@ func (c *Controller) addOrReplace(ctx context.Context, name types.NamespacedName
 		existing.cleanupWatch(ctx)
 	}
 
-	if notify != nil {
+	if startFileChangeLoop {
 		w.notify = notify
 		status.MonitorStartTime = apis.NowMicro()
 		ctx, cancel := context.WithCancel(ctx)


Test patch:
diff --git a/internal/controllers/core/filewatch/controller_test.go b/internal/controllers/core/filewatch/controller_test.go
index 6f692d6ef..77bcdf6ae 100644
--- a/internal/controllers/core/filewatch/controller_test.go
+++ b/internal/controllers/core/filewatch/controller_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/watch"
 	"github.com/tilt-dev/tilt/pkg/apis"
 	filewatches "github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
+	"github.com/tilt-dev/tilt/pkg/logger"
 )
 
 // Test constants
@@ -365,3 +366,17 @@ func TestController_Disable_Ignores_File_Changes(t *testing.T) {
 	f.MustGet(key, &fwAfterDisable)
 	require.Equal(t, 0, len(fwAfterDisable.Status.FileEvents))
 }
+
+func TestCreateSubError(t *testing.T) {
+	f := newFixture(t)
+	f.controller.fsWatcherMaker = fsevent.WatcherMaker(func(paths []string, ignore watch.PathMatcher, _ logger.Logger) (watch.Notify, error) {
+		var nilWatcher *fsevent.FakeWatcher = nil
+		return nilWatcher, fmt.Errorf("Unusual watcher error")
+	})
+	key, _ := f.CreateSimpleFileWatch()
+
+	// Expect that no file events were triggered
+	var fw filewatches.FileWatch
+	f.MustGet(key, &fw)
+	assert.Contains(t, fw.Status.Error, "filewatch init: Unusual watcher error")
+}