Skip to content

Commit

Permalink
filewatch: refactor tests to rely on state vs actions (#4296)
Browse files Browse the repository at this point in the history
This will make the tests easier to migrate to an apiserver world,
but the real motivation is for splitting the actions between the
create and reconcile, which results in more complex store interactions
across multiple subscribers that isn't practical to test using the
testing store.

This uses the "real" store with a minimal reducer that just handles
FileWatch actions.

New assertions have also been added that verify the FileWatch spec
is created as expected and also serve to force synchronization on
the naturally async store, so that the tests are still as deterministic
as possible.
  • Loading branch information
milas committed Mar 10, 2021
1 parent 2b7b0c4 commit f382940
Show file tree
Hide file tree
Showing 3 changed files with 252 additions and 136 deletions.
2 changes: 2 additions & 0 deletions internal/engine/fswatch/fake_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,13 @@ func (w *FakeMultiWatcher) loop() {
if !ok {
return
}
w.mu.Lock()
for _, watcher := range w.watchers {
if watcher.matches(e.Path()) {
watcher.inboundCh <- e
}
}
w.mu.Unlock()
case e, ok := <-w.Errors:
if !ok {
return
Expand Down
27 changes: 14 additions & 13 deletions internal/engine/fswatch/watchmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,20 @@ func globalIgnores(es store.EngineState) []model.Dockerignore {
}
ignores = append(ignores, es.WatchSettings.Ignores...)

outputs := []string{}
for _, manifest := range es.Manifests() {
for _, iTarget := range manifest.ImageTargets {
customBuild := iTarget.CustomBuildInfo()
if customBuild.OutputsImageRefTo != "" {
outputs = append(outputs, customBuild.OutputsImageRefTo)
// this could be smarter and try to group by local path
ignores = append(ignores, model.Dockerignore{
LocalPath: filepath.Dir(customBuild.OutputsImageRefTo),
Source: "outputs_image_ref_to",
Patterns: []string{filepath.Base(customBuild.OutputsImageRefTo)},
})
}
}
}

if len(outputs) > 0 {
ignores = append(ignores, model.Dockerignore{
LocalPath: filepath.Dir(es.TiltfilePath),
Source: "outputs_image_ref_to",
Patterns: outputs,
})
}

return ignores
}

Expand Down Expand Up @@ -189,7 +185,7 @@ func (w *WatchManager) OnChange(ctx context.Context, st store.RStore, _ store.Ch
for targetID, spec := range specsToProcess {
name := types.NamespacedName{Name: targetID.String()}
watchesToKeep[name] = true
res, fwStatus, err := w.addOrUpdate(ctx, st, name, spec)
res, fwStatus, err := w.addOrUpdate(ctx, st, name, *spec.DeepCopy())
if err != nil {
logger.Get(ctx).Debugf("Error adding/updating watch for %q: %v", name.String(), err)
continue
Expand Down Expand Up @@ -241,7 +237,9 @@ func (w *WatchManager) addOrUpdate(ctx context.Context, st store.RStore, name ty
var ignoreMatchers []model.PathMatcher
for _, ignoreDef := range spec.Ignores {
if len(ignoreDef.Patterns) != 0 {
m, err := dockerignore.NewDockerPatternMatcher(ignoreDef.BasePath, ignoreDef.Patterns)
m, err := dockerignore.NewDockerPatternMatcher(
ignoreDef.BasePath,
append([]string{}, ignoreDef.Patterns...))
if err != nil {
return resultUnknown, nil, fmt.Errorf("invalid ignore def: %v", err)
}
Expand All @@ -257,7 +255,10 @@ func (w *WatchManager) addOrUpdate(ctx context.Context, st store.RStore, name ty
// ephemeral OS/IDE stuff is not part of the spec but always included
ignoreMatchers = append(ignoreMatchers, ignore.EphemeralPathMatcher)

notify, err := w.fsWatcherMaker(spec.WatchedPaths, model.NewCompositeMatcher(ignoreMatchers), logger.Get(ctx))
notify, err := w.fsWatcherMaker(
append([]string{}, spec.WatchedPaths...),
model.NewCompositeMatcher(ignoreMatchers),
logger.Get(ctx))
if err != nil {
return resultUnknown, nil, fmt.Errorf("failed to initialize filesystem watch: %v", err)
}
Expand Down

0 comments on commit f382940

Please sign in to comment.