diff --git a/core/repomanager/BUILD.bazel b/core/repomanager/BUILD.bazel index c10a11e0..eaf89e98 100644 --- a/core/repomanager/BUILD.bazel +++ b/core/repomanager/BUILD.bazel @@ -9,7 +9,6 @@ go_library( "//core/git", "//core/workspace", "//tangopb", - "@com_github_gofrs_flock//:flock", "@org_uber_go_zap//:zap", ], ) diff --git a/core/repomanager/repo_manager.go b/core/repomanager/repo_manager.go index 7076f742..3a6795ca 100644 --- a/core/repomanager/repo_manager.go +++ b/core/repomanager/repo_manager.go @@ -6,21 +6,17 @@ import ( "os" "path/filepath" "strings" - "time" + "sync" "github.com/uber/tango/core/git" "github.com/uber/tango/core/workspace" "github.com/uber/tango/tangopb" - "github.com/gofrs/flock" "go.uber.org/zap" ) -const ( - lockFileName = ".tango.lease.lock" - lockTimeout = 5 * time.Minute - lockRetryDelay = 15 * time.Second -) +const defaultPoolSize = 3 +// RepoManager manages repository workspaces with a pool of workers per repo. type RepoManager interface { Lease(ctx context.Context, desc tangopb.BuildDescription) (workspace.Workspace, error) } @@ -29,66 +25,162 @@ type repoManager struct { git git.Interface rootWorkspace string logger *zap.SugaredLogger + poolSize int + + mu sync.Mutex + pools map[string]*workerPool +} + +// workerPool manages a fixed set of worker slots for a single repo. +// The origin directory holds the initial clone; workers are cheap local copies. +type workerPool struct { + originDir string + originMu sync.Mutex // one lock per repo for orginal clone + cloned bool + + avail chan *workerSlot // available slots; = pool capacity } +// workerSlot is a pre-allocated workspace directory that may or may not +// have been cloned yet. Lazy creation on first use. +type workerSlot struct { + dir string + created bool +} + +// Params for creating a RepoManager. type Params struct { Git git.Interface Logger *zap.SugaredLogger RootWorkspace string + PoolSize int // number of worker workspaces per repo; 0 uses default (3) } -// NewRepoManager creates a new repo manager with the given git interface and root workspace. +// NewRepoManager creates a new repo manager with pooled worker workspaces. func NewRepoManager(p Params) RepoManager { - return &repoManager{git: p.Git, rootWorkspace: p.RootWorkspace, logger: p.Logger} + size := p.PoolSize + if size <= 0 { + size = defaultPoolSize + } + return &repoManager{ + git: p.Git, + rootWorkspace: p.RootWorkspace, + logger: p.Logger, + poolSize: size, + pools: make(map[string]*workerPool), + } } -// Lease tries to take an exclusive lease on the repo’s workspace. -// If another process holds it, wait with a timeout. -func (r *repoManager) Lease(ctx context.Context, desc tangopb.BuildDescription) (workspace.Workspace, error) { - repo := toShortRemote(desc.Remote) - repoDir := filepath.Join(r.rootWorkspace, repo) - // Lock file must be in the root workspace as git clone cannot be executed on a non-empty directory. - lockPath := filepath.Join(r.rootWorkspace, lockFileName) - if err := os.MkdirAll(r.rootWorkspace, 0o755); err != nil { - return nil, fmt.Errorf("mkdir repo dir: %w", err) +func (r *repoManager) poolFor(repo string) *workerPool { + r.mu.Lock() + defer r.mu.Unlock() + + if pool, ok := r.pools[repo]; ok { + return pool } - fl := flock.New(lockPath) + pool := &workerPool{ + originDir: filepath.Join(r.rootWorkspace, repo), + avail: make(chan *workerSlot, r.poolSize), + } - waitCtx, cancel := context.WithTimeout(ctx, lockTimeout) - defer cancel() + // Pre-allocate fixed worker slots. Existing directories from a previous + // run are detected and reused without re-cloning. + workersDir := filepath.Join(r.rootWorkspace, ".workers", repo) + for i := 1; i <= r.poolSize; i++ { + dir := filepath.Join(workersDir, fmt.Sprintf("worker-%d", i)) + slot := &workerSlot{dir: dir} + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + slot.created = true + r.logger.Debugf("discovered existing worker: %s", dir) + } + pool.avail <- slot + } + + r.pools[repo] = pool + return pool +} + +// Lease borrows a worker workspace from the pool. +// If all workers are leased, it blocks until one is returned or ctx is cancelled. +func (r *repoManager) Lease(ctx context.Context, desc tangopb.BuildDescription) (workspace.Workspace, error) { + repo := toShortRemote(desc.Remote) + pool := r.poolFor(repo) - // This will block until: - // 1) the lock is released by another process, OR - // 2) waitCtx expires/canceled - locked, err := fl.TryLockContext(waitCtx, lockRetryDelay) - if err != nil { - return nil, fmt.Errorf("failed to acquire lock for %s: %w", r.rootWorkspace, err) + if err := pool.ensureOrigin(ctx, r.git, desc.Remote); err != nil { + return nil, err } - if !locked { - return nil, fmt.Errorf("lock timeout after %s for %s", lockTimeout, r.rootWorkspace) + + // Acquire a worker slot (blocks if all slots are leased) + var slot *workerSlot + select { + case slot = <-pool.avail: + case <-ctx.Done(): + return nil, ctx.Err() } - if _, err := os.Stat(filepath.Join(repoDir, ".git")); os.IsNotExist(err) { - if err := r.git.Clone(ctx, desc.Remote, repoDir); err != nil { - flockErr := fl.Unlock() - if flockErr != nil { - r.logger.Errorf("unlock failed: %w", flockErr) - } - removeErr := os.RemoveAll(repoDir) - if removeErr != nil { - r.logger.Errorf("remove repo dir failed: %w", removeErr) - } - return nil, fmt.Errorf("clone failed: %w", err) + + // Lazily create the worker clone on first use + if !slot.created { + if err := r.createWorker(ctx, pool.originDir, slot.dir); err != nil { + pool.avail <- slot // return slot so others can retry + return nil, fmt.Errorf("create worker: %w", err) } + slot.created = true } - // Use a Git interface rooted at the repo directory so commands run in the correct working directory. - repoGit := git.New(repoDir) - return workspace.NewWorkspace(workspace.WorkspaceParams{ - Path: repoDir, - Lock: fl, + + repoGit := git.New(slot.dir) + ws := workspace.NewWorkspace(workspace.WorkspaceParams{ + Path: slot.dir, Git: repoGit, Logger: r.logger, - }), nil + }) + return &pooledWorkspace{Workspace: ws, pool: pool, slot: slot}, nil +} + +// ensureOrigin clones the origin repository if it doesn't exist yet. +func (p *workerPool) ensureOrigin(ctx context.Context, g git.Interface, remote string) error { + p.originMu.Lock() + defer p.originMu.Unlock() + + if p.cloned { + return nil + } + if _, err := os.Stat(filepath.Join(p.originDir, ".git")); err == nil { + p.cloned = true + return nil + } + + if err := os.MkdirAll(filepath.Dir(p.originDir), 0o755); err != nil { + return fmt.Errorf("mkdir origin dir: %w", err) + } + if err := g.Clone(ctx, remote, p.originDir); err != nil { + os.RemoveAll(p.originDir) + return fmt.Errorf("clone origin: %w", err) + } + p.cloned = true + return nil +} + +// createWorker creates a worker by cloning the origin with --local +// (fast and space-efficient). +func (r *repoManager) createWorker(ctx context.Context, originDir, workerDir string) error { + os.RemoveAll(workerDir) // clean up any partial/corrupted previous state + if err := os.MkdirAll(filepath.Dir(workerDir), 0o755); err != nil { + return err + } + return r.git.Clone(ctx, originDir, workerDir, "--local") +} + +// pooledWorkspace wraps a workspace and returns its slot to the pool on release. +type pooledWorkspace struct { + workspace.Workspace + pool *workerPool + slot *workerSlot +} + +func (pw *pooledWorkspace) Release() error { + pw.pool.avail <- pw.slot + return nil } func toShortRemote(remote string) string { diff --git a/core/repomanager/repo_manager_test.go b/core/repomanager/repo_manager_test.go index 55d92182..105d0a76 100644 --- a/core/repomanager/repo_manager_test.go +++ b/core/repomanager/repo_manager_test.go @@ -2,9 +2,11 @@ package repomanager import ( "context" + "fmt" "os" "path/filepath" "testing" + "time" gitmock "github.com/uber/tango/core/git/gitmock" "github.com/uber/tango/tangopb" @@ -14,79 +16,291 @@ import ( "go.uber.org/zap" ) -func TestLease_CreatesRepoDirAndClones_WhenGitMissing(t *testing.T) { +func TestLease_ClonesOriginAndCreatesWorker(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) g := gitmock.NewMockInterface(ctrl) root := t.TempDir() remote := "git@github.com:org/repo" - repoDir := filepath.Join(root, "org/repo") + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") - g.EXPECT().Clone(gomock.Any(), remote, repoDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil) - rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root}) - ctx := context.Background() - ws, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ws, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) require.NoError(t, err) - require.NotNil(t, ws) - defer func() { require.NoError(t, ws.Release()) }() - assert.Equal(t, repoDir, ws.Path()) + assert.Equal(t, workerDir, ws.Path()) + require.NoError(t, ws.Release()) } -func TestLease_SkipsClone_WhenGitPresent(t *testing.T) { +func TestLease_SkipsOriginClone_WhenExists(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) g := gitmock.NewMockInterface(ctrl) root := t.TempDir() remote := "git@github.com:org/repo" - repoDir := filepath.Join(root, "org/repo") - require.NoError(t, os.MkdirAll(filepath.Join(repoDir, ".git"), 0o755)) + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") + + require.NoError(t, os.MkdirAll(filepath.Join(originDir, ".git"), 0o755)) - // No expectation for Clone; if called, gomock will fail the test - rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root}) + // Only worker clone expected + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) ws, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) require.NoError(t, err) - require.NotNil(t, ws) - defer func() { require.NoError(t, ws.Release()) }() + assert.Equal(t, workerDir, ws.Path()) + require.NoError(t, ws.Release()) +} + +func TestLease_ReusesWorker_AfterRelease(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") + + // Exactly 1 origin + 1 worker clone total + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ctx := context.Background() + + ws1, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + require.NoError(t, ws1.Release()) + + // Second lease reuses the same worker — no new clones + ws2, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + assert.Equal(t, workerDir, ws2.Path()) + require.NoError(t, ws2.Release()) +} + +func TestLease_CreatesMultipleWorkers(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + for i := 1; i <= 2; i++ { + dir := filepath.Join(root, ".workers", "org/repo", fmt.Sprintf("worker-%d", i)) + g.EXPECT().Clone(gomock.Any(), originDir, dir, "--local").Return(nil) + } + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 2}) + ctx := context.Background() + + ws1, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + ws2, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + + assert.NotEqual(t, ws1.Path(), ws2.Path()) + require.NoError(t, ws1.Release()) + require.NoError(t, ws2.Release()) +} + +func TestLease_BlocksUntilReturn(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") + + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ctx := context.Background() + + ws1, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + + // Second lease blocks because pool size = 1 + done := make(chan error, 1) + go func() { + ws2, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + if err == nil { + ws2.Release() + } + done <- err + }() + + time.Sleep(50 * time.Millisecond) + select { + case <-done: + t.Fatal("second lease should block") + default: + } + + require.NoError(t, ws1.Release()) + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(5 * time.Second): + t.Fatal("second lease did not unblock") + } +} + +func TestLease_CtxCanceled(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") + + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + + ws1, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) - assert.Equal(t, repoDir, ws.Path()) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.Error(t, err) + + require.NoError(t, ws1.Release()) } -func TestLease_CloneFails_ReleasesLock(t *testing.T) { +func TestLease_OriginCloneFails(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) g := gitmock.NewMockInterface(ctrl) root := t.TempDir() remote := "git@github.com:org/repo" - repoDir := filepath.Join(root, "org/repo") + g.EXPECT().Clone(gomock.Any(), remote, filepath.Join(root, "org/repo")).Return(assert.AnError) - g.EXPECT().Clone(gomock.Any(), remote, repoDir).Return(assert.AnError) + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + _, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) + require.Error(t, err) + assert.Contains(t, err.Error(), "clone origin") +} - rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root}) +func TestLease_WorkerCloneFails(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") + + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(assert.AnError) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) _, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) require.Error(t, err) + assert.Contains(t, err.Error(), "create worker") +} + +func TestLease_DiscoversExistingWorker(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote := "git@github.com:org/repo" - // Repo dir should be removed on clone failure - _, statErr := os.Stat(repoDir) - require.Error(t, statErr) - assert.True(t, os.IsNotExist(statErr), "repo dir should be removed after clone failure") + // Pre-create origin and worker from a "previous run" + require.NoError(t, os.MkdirAll(filepath.Join(root, "org/repo", ".git"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(root, ".workers", "org/repo", "worker-1", ".git"), 0o755)) + + // No Clone calls — everything already exists + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ws, err := rm.Lease(context.Background(), tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + assert.Contains(t, ws.Path(), "worker-1") + require.NoError(t, ws.Release()) +} + +func TestLease_DifferentRepos_IndependentPools(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) + + root := t.TempDir() + remote1 := "git@github.com:org/repo1" + remote2 := "git@github.com:org/repo2" + + origin1 := filepath.Join(root, "org/repo1") + origin2 := filepath.Join(root, "org/repo2") + worker1 := filepath.Join(root, ".workers", "org/repo1", "worker-1") + worker2 := filepath.Join(root, ".workers", "org/repo2", "worker-1") + + g.EXPECT().Clone(gomock.Any(), remote1, origin1).Return(nil) + g.EXPECT().Clone(gomock.Any(), origin1, worker1, "--local").Return(nil) + g.EXPECT().Clone(gomock.Any(), remote2, origin2).Return(nil) + g.EXPECT().Clone(gomock.Any(), origin2, worker2, "--local").Return(nil) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ctx := context.Background() + + // Both repos can be leased concurrently even with pool size 1 + ws1, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote1}) + require.NoError(t, err) + ws2, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote2}) + require.NoError(t, err) + + assert.Contains(t, ws1.Path(), "repo1") + assert.Contains(t, ws2.Path(), "repo2") + + require.NoError(t, ws1.Release()) + require.NoError(t, ws2.Release()) } -func TestLease_LockUnavailable_CtxCanceled(t *testing.T) { +func TestLease_WorkerCloneFails_SlotReturnedToPool(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) + g := gitmock.NewMockInterface(ctrl) root := t.TempDir() remote := "git@github.com:org/repo" + originDir := filepath.Join(root, "org/repo") + workerDir := filepath.Join(root, ".workers", "org/repo", "worker-1") - rm := NewRepoManager(Params{Git: gitmock.NewMockInterface(ctrl), Logger: zap.NewNop().Sugar(), RootWorkspace: root}) - ctx, cancel := context.WithCancel(context.Background()) - cancel() // cancel immediately so TryLockContext returns quickly - ws, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + g.EXPECT().Clone(gomock.Any(), remote, originDir).Return(nil) + // First attempt fails, second succeeds + gomock.InOrder( + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(assert.AnError), + g.EXPECT().Clone(gomock.Any(), originDir, workerDir, "--local").Return(nil), + ) + + rm := NewRepoManager(Params{Git: g, Logger: zap.NewNop().Sugar(), RootWorkspace: root, PoolSize: 1}) + ctx := context.Background() + + // First attempt fails + _, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) require.Error(t, err) - require.Nil(t, ws) - assert.Contains(t, err.Error(), "failed to acquire lock") + + // Slot was returned to pool — retry succeeds + ws, err := rm.Lease(ctx, tangopb.BuildDescription{Remote: remote}) + require.NoError(t, err) + require.NoError(t, ws.Release()) } diff --git a/core/workspace/BUILD.bazel b/core/workspace/BUILD.bazel index 1792929c..d762ffb3 100644 --- a/core/workspace/BUILD.bazel +++ b/core/workspace/BUILD.bazel @@ -11,7 +11,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//core/git", - "@com_github_gofrs_flock//:flock", "@org_uber_go_zap//:zap", ], ) @@ -28,7 +27,6 @@ go_test( "//core/git", "//core/git/gitmock", "//core/workspace/requestmock", - "@com_github_gofrs_flock//:flock", "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", "@org_uber_go_mock//gomock", diff --git a/core/workspace/workspace.go b/core/workspace/workspace.go index 8314237d..9d297dc9 100644 --- a/core/workspace/workspace.go +++ b/core/workspace/workspace.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/uber/tango/core/git" - "github.com/gofrs/flock" "go.uber.org/zap" ) @@ -19,14 +18,12 @@ type Workspace interface { type workspace struct { path string - lock *flock.Flock git git.Interface logger *zap.SugaredLogger } type WorkspaceParams struct { Path string - Lock *flock.Flock Git git.Interface Logger *zap.SugaredLogger } @@ -35,7 +32,6 @@ type WorkspaceParams struct { func NewWorkspace(p WorkspaceParams) Workspace { return &workspace{ path: p.Path, - lock: p.Lock, git: p.Git, logger: p.Logger, } @@ -73,10 +69,7 @@ func (w *workspace) Checkout(ctx context.Context, remote string, ref string) err return w.git.Checkout(ctx, ref) } -// Release releases the workspace lock. +// Release is a no-op for the base workspace; pooled workspaces override this. func (w *workspace) Release() error { - if w.lock == nil { - return nil - } - return w.lock.Unlock() + return nil } diff --git a/core/workspace/workspace_test.go b/core/workspace/workspace_test.go index f2d9b903..4d09decf 100644 --- a/core/workspace/workspace_test.go +++ b/core/workspace/workspace_test.go @@ -3,12 +3,10 @@ package workspace import ( "context" "errors" - "path/filepath" "testing" gitmock "github.com/uber/tango/core/git/gitmock" requestmock "github.com/uber/tango/core/workspace/requestmock" - "github.com/gofrs/flock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -25,26 +23,21 @@ func TestNewWorkspace_SetsFields(t *testing.T) { defer ctrl.Finish() g := gitmock.NewMockInterface(ctrl) tmpDir := t.TempDir() - lockPath := filepath.Join(tmpDir, "ws.lock") - l := flock.New(lockPath) w := NewWorkspace(WorkspaceParams{ Path: tmpDir, - Lock: l, Git: g, }) iw, ok := w.(*workspace) require.True(t, ok) assert.Equal(t, tmpDir, iw.path) - assert.Equal(t, l, iw.lock) assert.Equal(t, g, iw.git) } func TestWorkspace_ApplyRequests_Success(t *testing.T) { w := NewWorkspace(WorkspaceParams{ Path: "/tmp/workspace", - Lock: flock.New("/tmp/workspace.lock"), Git: gitmock.NewMockInterface(gomock.NewController(t)), Logger: zap.NewNop().Sugar(), }) @@ -103,19 +96,8 @@ func TestWorkspace_Checkout_FetchError(t *testing.T) { require.Error(t, err) } -func TestWorkspace_Release_NoLock(t *testing.T) { - w := &workspace{lock: nil} - err := w.Release() - require.NoError(t, err) -} - -func TestWorkspace_Release_WithLock(t *testing.T) { - tmpDir := t.TempDir() - lockPath := filepath.Join(tmpDir, "ws.lock") - l := flock.New(lockPath) - require.NoError(t, l.Lock()) - w := &workspace{lock: l} - +func TestWorkspace_Release(t *testing.T) { + w := &workspace{} err := w.Release() require.NoError(t, err) }