remove jonboulle/clockwork for testing/synctest

.golangci.yml

@@ -1,6 +1,8 @@
 version: "2"
 run:
   concurrency: 2
+  build-tags:
+    - e2e # Avoid "unused" linter issues for code only used in e2e.
 linters:
   default: none
   disable:

go.mod

@@ -24,7 +24,6 @@ require (
 	github.com/google/go-cmp v0.7.0
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/go-version v1.9.0
-	github.com/jonboulle/clockwork v0.5.0
 	github.com/mattn/go-shellwords v1.0.14
 	github.com/mitchellh/go-ps v1.0.0
 	github.com/moby/buildkit v0.33.0

go.sum

@@ -139,8 +139,6 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf h1:FtEj8sfIcaaBfAKrE1Cwb61YDtYq9JxChK1c7AKce7s=
 github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf/go.mod h1:yrqSXGoD/4EKfF26AOGzscPOgTTJcyAwM2rpixWT+t4=
-github.com/jonboulle/clockwork v0.5.0 h1:Hyh9A8u51kptdkR+cqRpT1EebBwTn1oK9YfGYbdFz6I=
-github.com/jonboulle/clockwork v0.5.0/go.mod h1:3mZlmanh0g2NDKO5TWZVJAfofYk64M7XN3SzBPjZF60=
 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/klauspost/compress v1.19.2 h1:hMRETovs/pu/dVWN7zIT1PGG8t509MwT6bO7XSi26R8=

pkg/compose/compose.go

@@ -30,7 +30,6 @@ import (
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/flags"
 	"github.com/docker/cli/cli/streams"
-	"github.com/jonboulle/clockwork"
 	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/api/types/swarm"
 	"github.com/moby/moby/client"
@@ -65,7 +64,6 @@ type Option func(service *composeService) error
 func NewComposeService(dockerCli command.Cli, options ...Option) (api.Compose, error) {
 	s := &composeService{
 		dockerCli:      dockerCli,
-		clock:          clockwork.NewRealClock(),
 		maxConcurrency: -1,
 		dryRun:         false,
 	}
@@ -211,7 +209,6 @@ type composeService struct {
 	contextInfo api.ContextInfo
 	proxyConfig map[string]string
 
-	clock          clockwork.Clock
 	maxConcurrency int
 	dryRun         bool
 

pkg/compose/watch.go

@@ -376,7 +376,7 @@ func (s *composeService) watchEvents(ctx context.Context, project *types.Project
 	defer cancel()
 
 	// debounce and group filesystem events so that we capture IDE saving many files as one "batch" event
-	batchEvents := watch.BatchDebounceEvents(ctx, s.clock, watcher.Events())
+	batchEvents := watch.BatchDebounceEvents(ctx, watcher.Events())
 
 	for {
 		select {

pkg/compose/watch_test.go

@@ -22,11 +22,11 @@ import (
 	"path/filepath"
 	"slices"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"github.com/compose-spec/compose-go/v2/types"
 	"github.com/docker/cli/cli/streams"
-	"github.com/jonboulle/clockwork"
 	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/api/types/image"
 	"github.com/moby/moby/client"
@@ -75,87 +75,81 @@ func (s stdLogger) Status(containerName, msg string) {
 }
 
 func TestWatch_Sync(t *testing.T) {
-	mockCtrl := gomock.NewController(t)
-	cli := mocks.NewMockCli(mockCtrl)
-	cli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
-	apiClient := mocks.NewMockAPIClient(mockCtrl)
-	apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(client.ContainerListResult{
-		Items: []container.Summary{
-			testContainer("test", "123", false),
-		},
-	}, nil).AnyTimes()
-	// we expect the image to be pruned
-	apiClient.EXPECT().ImageList(gomock.Any(), client.ImageListOptions{
-		Filters: make(client.Filters).
-			Add("dangling", "true").
-			Add("label", api.ProjectLabel+"=myProjectName"),
-	}).Return(client.ImageListResult{
-		Items: []image.Summary{
-			{ID: "123"},
-			{ID: "456"},
-		},
-	}, nil).Times(1)
-	apiClient.EXPECT().ImageRemove(gomock.Any(), "123", client.ImageRemoveOptions{}).Times(1)
-	apiClient.EXPECT().ImageRemove(gomock.Any(), "456", client.ImageRemoveOptions{}).Times(1)
-	//
-	cli.EXPECT().Client().Return(apiClient).AnyTimes()
-
-	ctx, cancelFunc := context.WithCancel(t.Context())
-	t.Cleanup(cancelFunc)
-
-	proj := types.Project{
-		Name: "myProjectName",
-		Services: types.Services{
-			"test": {
-				Name: "test",
+	synctest.Test(t, func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		cli := mocks.NewMockCli(mockCtrl)
+		cli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
+		apiClient := mocks.NewMockAPIClient(mockCtrl)
+		apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(client.ContainerListResult{
+			Items: []container.Summary{
+				testContainer("test", "123", false),
 			},
-		},
-	}
-
-	watcher := testWatcher{
-		events: make(chan watch.FileEvent),
-		errors: make(chan error),
-	}
+		}, nil).AnyTimes()
+		// we expect the image to be pruned
+		apiClient.EXPECT().ImageList(gomock.Any(), client.ImageListOptions{
+			Filters: make(client.Filters).
+				Add("dangling", "true").
+				Add("label", api.ProjectLabel+"=myProjectName"),
+		}).Return(client.ImageListResult{
+			Items: []image.Summary{
+				{ID: "123"},
+				{ID: "456"},
+			},
+		}, nil).Times(1)
+		apiClient.EXPECT().ImageRemove(gomock.Any(), "123", client.ImageRemoveOptions{}).Times(1)
+		apiClient.EXPECT().ImageRemove(gomock.Any(), "456", client.ImageRemoveOptions{}).Times(1)
+		//
+		cli.EXPECT().Client().Return(apiClient).AnyTimes()
+
+		proj := types.Project{
+			Name: "myProjectName",
+			Services: types.Services{
+				"test": {
+					Name: "test",
+				},
+			},
+		}
 
-	syncer := newFakeSyncer()
-	clock := clockwork.NewFakeClock()
-	go func() {
-		service := composeService{
-			dockerCli:      cli,
-			clock:          clock,
-			maxConcurrency: -1,
+		watcher := testWatcher{
+			events: make(chan watch.FileEvent),
+			errors: make(chan error),
 		}
-		rules, err := getWatchRules(&types.DevelopConfig{
-			Watch: []types.Trigger{
-				{
-					Path:   "/sync",
-					Action: "sync",
-					Target: "/work",
-					Ignore: []string{"ignore"},
-				},
-				{
-					Path:   "/rebuild",
-					Action: "rebuild",
+
+		syncer := newFakeSyncer()
+		go func() {
+			service := composeService{
+				dockerCli:      cli,
+				maxConcurrency: -1,
+			}
+			rules, err := getWatchRules(&types.DevelopConfig{
+				Watch: []types.Trigger{
+					{
+						Path:   "/sync",
+						Action: "sync",
+						Target: "/work",
+						Ignore: []string{"ignore"},
+					},
+					{
+						Path:   "/rebuild",
+						Action: "rebuild",
+					},
 				},
-			},
-		}, types.ServiceConfig{Name: "test"})
-		assert.NilError(t, err)
-
-		err = service.watchEvents(ctx, &proj, api.WatchOptions{
-			Build: &api.BuildOptions{},
-			LogTo: stdLogger{},
-			Prune: true,
-		}, watcher, syncer, rules)
-		assert.NilError(t, err)
-	}()
-
-	watcher.Events() <- watch.NewFileEvent("/sync/changed")
-	watcher.Events() <- watch.NewFileEvent("/sync/changed/sub")
-	err := clock.BlockUntilContext(ctx, 3)
-	assert.NilError(t, err)
-	clock.Advance(watch.QuietPeriod)
-	select {
-	case actual := <-syncer.synced:
+			}, types.ServiceConfig{Name: "test"})
+			assert.NilError(t, err)
+
+			err = service.watchEvents(t.Context(), &proj, api.WatchOptions{
+				Build: &api.BuildOptions{},
+				LogTo: stdLogger{},
+				Prune: true,
+			}, watcher, syncer, rules)
+			assert.NilError(t, err)
+		}()
+
+		watcher.Events() <- watch.NewFileEvent("/sync/changed")
+		watcher.Events() <- watch.NewFileEvent("/sync/changed/sub")
+		time.Sleep(watch.QuietPeriod)
+		synctest.Wait()
+		actual := <-syncer.synced
 		expected := []*sync.PathMapping{
 			{HostPath: "/sync/changed", ContainerPath: "/work/changed"},
 			{HostPath: "/sync/changed/sub", ContainerPath: "/work/changed/sub"},
@@ -164,22 +158,20 @@ func TestWatch_Sync(t *testing.T) {
 			return cmp.Compare(a.HostPath, b.HostPath)
 		})
 		assert.DeepEqual(t, expected, actual)
-	case <-time.After(100 * time.Millisecond):
-		t.Error("timeout")
-	}
 
-	watcher.Events() <- watch.NewFileEvent("/rebuild")
-	watcher.Events() <- watch.NewFileEvent("/sync/changed")
-	err = clock.BlockUntilContext(ctx, 4)
-	assert.NilError(t, err)
-	clock.Advance(watch.QuietPeriod)
-	select {
-	case batch := <-syncer.synced:
-		t.Fatalf("received unexpected events: %v", batch)
-	case <-time.After(100 * time.Millisecond):
-		// expected
-	}
-	// TODO: there's not a great way to assert that the rebuild attempt happened
+		// Rebuild fails before sync actions from the same batch are processed.
+		watcher.Events() <- watch.NewFileEvent("/rebuild")
+		watcher.Events() <- watch.NewFileEvent("/sync/changed")
+		time.Sleep(watch.QuietPeriod)
+		synctest.Wait()
+		select {
+		case batch := <-syncer.synced:
+			t.Fatalf("received unexpected events: %v", batch)
+		default:
+			// expected
+		}
+		// TODO: there's not a great way to assert that the rebuild attempt happened
+	})
 }
 
 type fakeSyncer struct {

pkg/watch/debounce.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/jonboulle/clockwork"
 	"github.com/sirupsen/logrus"
 
 	"github.com/docker/compose/v5/pkg/utils"
@@ -30,7 +29,7 @@ const QuietPeriod = 500 * time.Millisecond
 // channel.
 //
 // The returned channel is closed when the debouncer is stopped via context cancellation or by closing the input channel.
-func BatchDebounceEvents(ctx context.Context, clock clockwork.Clock, input <-chan FileEvent) <-chan []FileEvent {
+func BatchDebounceEvents(ctx context.Context, input <-chan FileEvent) <-chan []FileEvent {
 	out := make(chan []FileEvent)
 	go func() {
 		defer close(out)
@@ -49,13 +48,13 @@ func BatchDebounceEvents(ctx context.Context, clock clockwork.Clock, input <-cha
 			seen = utils.Set[FileEvent]{}
 		}
 
-		t := clock.NewTicker(QuietPeriod)
+		t := time.NewTicker(QuietPeriod)
 		defer t.Stop()
 		for {
 			select {
 			case <-ctx.Done():
 				return
-			case <-t.Chan():
+			case <-t.C:
 				flushEvents()
 			case e, ok := <-input:
 				if !ok {

pkg/watch/debounce_test.go

@@ -15,52 +15,44 @@
 package watch
 
 import (
-	"context"
 	"slices"
 	"testing"
+	"testing/synctest"
 	"time"
 
-	"github.com/jonboulle/clockwork"
 	"gotest.tools/v3/assert"
 )
 
 func Test_BatchDebounceEvents(t *testing.T) {
-	ch := make(chan FileEvent)
-	clock := clockwork.NewFakeClock()
-	ctx, stop := context.WithCancel(t.Context())
-	t.Cleanup(stop)
+	synctest.Test(t, func(t *testing.T) {
+		ch := make(chan FileEvent)
 
-	eventBatchCh := BatchDebounceEvents(ctx, clock, ch)
-	for i := range 100 {
-		path := "/a"
-		if i%2 == 0 {
-			path = "/b"
-		}
+		eventBatchCh := BatchDebounceEvents(t.Context(), ch)
+		for i := range 100 {
+			path := "/a"
+			if i%2 == 0 {
+				path = "/b"
+			}
 
-		ch <- FileEvent(path)
-	}
-	// we sent 100 events + the debouncer
-	err := clock.BlockUntilContext(ctx, 101)
-	assert.NilError(t, err)
-	clock.Advance(QuietPeriod)
-	select {
-	case batch := <-eventBatchCh:
+			ch <- FileEvent(path)
+		}
+		time.Sleep(QuietPeriod)
+		synctest.Wait()
+		batch := <-eventBatchCh
 		slices.Sort(batch)
 		assert.Equal(t, len(batch), 2)
 		assert.Equal(t, batch[0], FileEvent("/a"))
 		assert.Equal(t, batch[1], FileEvent("/b"))
-	case <-time.After(50 * time.Millisecond):
-		t.Fatal("timed out waiting for events")
-	}
-	err = clock.BlockUntilContext(ctx, 1)
-	assert.NilError(t, err)
-	clock.Advance(QuietPeriod)
 
-	// there should only be a single batch
-	select {
-	case batch := <-eventBatchCh:
-		t.Fatalf("unexpected events: %v", batch)
-	case <-time.After(50 * time.Millisecond):
-		// channel is empty
-	}
+		time.Sleep(QuietPeriod)
+		synctest.Wait()
+
+		// there should only be a single batch
+		select {
+		case batch := <-eventBatchCh:
+			t.Fatalf("unexpected events: %v", batch)
+		default:
+			// channel is empty
+		}
+	})
 }