From 9cc953d04202a2e00abae940f0d2f41795d26c56 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Thu, 9 Feb 2023 17:34:01 -0600 Subject: [PATCH 1/3] Add regression test for .git file watcher --- extension/src/experiments/data/index.ts | 9 ++- .../test/suite/experiments/data/index.test.ts | 57 ++++++++++++++++++- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/extension/src/experiments/data/index.ts b/extension/src/experiments/data/index.ts index 2028cce728..fee2b077de 100644 --- a/extension/src/experiments/data/index.ts +++ b/extension/src/experiments/data/index.ts @@ -18,10 +18,13 @@ import { getGitPath } from '../../fileSystem' export const QUEUED_EXPERIMENT_PATH = join(DOT_DVC, 'tmp', 'exps') export class ExperimentsData extends BaseData { + private dotGitPath: string | undefined + constructor( dvcRoot: string, internalCommands: InternalCommands, - updatesPaused: EventEmitter + updatesPaused: EventEmitter, + dotGitPath?: string ) { super( dvcRoot, @@ -39,6 +42,8 @@ export class ExperimentsData extends BaseData { void this.watchExpGitRefs() void this.managedUpdate(QUEUED_EXPERIMENT_PATH) + + this.dotGitPath = dotGitPath } public managedUpdate(path?: string) { @@ -74,7 +79,7 @@ export class ExperimentsData extends BaseData { this.dvcRoot ) - const dotGitPath = getGitPath(gitRoot, gitPath.DOT_GIT) + const dotGitPath = this.dotGitPath || getGitPath(gitRoot, gitPath.DOT_GIT) const watchedRelPaths = [ gitPath.DOT_GIT_HEAD, EXPERIMENTS_GIT_REFS, diff --git a/extension/src/test/suite/experiments/data/index.test.ts b/extension/src/test/suite/experiments/data/index.test.ts index 1814e48b77..d5d38f44b8 100644 --- a/extension/src/test/suite/experiments/data/index.test.ts +++ b/extension/src/test/suite/experiments/data/index.test.ts @@ -1,7 +1,9 @@ +import { join } from 'path' import { afterEach, beforeEach, describe, it, suite } from 'mocha' import { EventEmitter, RelativePattern } from 'vscode' import { expect } from 'chai' import { stub, restore, spy } from 'sinon' +import { ensureFileSync, removeSync } from 'fs-extra' import { bypassProcessManagerDebounce, getArgOfCall, @@ -37,6 +39,7 @@ suite('Experiments Data Test Suite', () => { return disposable.disposeAndFlush() }) + // eslint-disable-next-line sonarjs/cognitive-complexity describe('ExperimentsData', () => { it('should debounce all calls to update that are made within 200ms', async () => { const { data, mockExperimentShow } = buildExperimentsData(disposable) @@ -67,7 +70,7 @@ suite('Experiments Data Test Suite', () => { ) }) - it('should watch the .git directory for updates', async () => { + it('should watch the .git directory for updates when the directory is outside of the workspace', async () => { const mockNow = getMockNow() const gitRoot = dvcDemoPath @@ -102,6 +105,58 @@ suite('Experiments Data Test Suite', () => { expect(managedUpdateSpy).to.be.called }) + it('should watch the .git directory for updates when directory is inside workspace', async () => { + const mockNow = getMockNow() + const mockGitRoot = dvcDemoPath + const mockDotGitPath = join(dvcDemoPath, '.mock-git') + const mockDotGitFilePath = join(mockDotGitPath, gitPath.DOT_GIT_HEAD) + const mockDotGitNestedFilePath = join( + mockDotGitPath, + EXPERIMENTS_GIT_LOGS_REFS, + 'index' + ) + + ensureFileSync(mockDotGitFilePath) + ensureFileSync(mockDotGitNestedFilePath) + + const mockExecuteCommand = (command: CommandId) => { + if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { + return Promise.resolve(mockGitRoot) + } + } + const data = disposable.track( + new ExperimentsData( + dvcDemoPath, + { + dispose: stub(), + executeCommand: mockExecuteCommand + } as unknown as InternalCommands, + disposable.track(new EventEmitter()), + mockDotGitPath + ) + ) + + await data.isReady() + bypassProcessManagerDebounce(mockNow) + + const managedUpdateSpy = spy(data, 'managedUpdate') + const dataUpdatedEvent = new Promise(resolve => + data.onDidUpdate(() => resolve(undefined)) + ) + + await Watcher.fireWatcher(mockDotGitFilePath) + await dataUpdatedEvent + + expect(managedUpdateSpy).to.be.called + + await Watcher.fireWatcher(mockDotGitNestedFilePath) + await dataUpdatedEvent + + expect(managedUpdateSpy).to.be.called + + removeSync(mockDotGitPath) + }) + it('should not use exp show to fetch git refs external to the workspace if the path is not from a temp workspace', async () => { const mockNow = getMockNow() const { data, mockExperimentShow } = buildExperimentsData(disposable) From e3c9fcd5a90aa7884366899b6fd54ef5a9b7ff93 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Fri, 10 Feb 2023 12:51:25 -0600 Subject: [PATCH 2/3] Refactor --- extension/src/experiments/data/index.ts | 9 ++---- .../test/suite/experiments/data/index.test.ts | 29 ++++++++++++------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/extension/src/experiments/data/index.ts b/extension/src/experiments/data/index.ts index fee2b077de..2028cce728 100644 --- a/extension/src/experiments/data/index.ts +++ b/extension/src/experiments/data/index.ts @@ -18,13 +18,10 @@ import { getGitPath } from '../../fileSystem' export const QUEUED_EXPERIMENT_PATH = join(DOT_DVC, 'tmp', 'exps') export class ExperimentsData extends BaseData { - private dotGitPath: string | undefined - constructor( dvcRoot: string, internalCommands: InternalCommands, - updatesPaused: EventEmitter, - dotGitPath?: string + updatesPaused: EventEmitter ) { super( dvcRoot, @@ -42,8 +39,6 @@ export class ExperimentsData extends BaseData { void this.watchExpGitRefs() void this.managedUpdate(QUEUED_EXPERIMENT_PATH) - - this.dotGitPath = dotGitPath } public managedUpdate(path?: string) { @@ -79,7 +74,7 @@ export class ExperimentsData extends BaseData { this.dvcRoot ) - const dotGitPath = this.dotGitPath || getGitPath(gitRoot, gitPath.DOT_GIT) + const dotGitPath = getGitPath(gitRoot, gitPath.DOT_GIT) const watchedRelPaths = [ gitPath.DOT_GIT_HEAD, EXPERIMENTS_GIT_REFS, diff --git a/extension/src/test/suite/experiments/data/index.test.ts b/extension/src/test/suite/experiments/data/index.test.ts index d5d38f44b8..7c267c0df9 100644 --- a/extension/src/test/suite/experiments/data/index.test.ts +++ b/extension/src/test/suite/experiments/data/index.test.ts @@ -26,7 +26,9 @@ import { buildExperimentsData } from '../util' import { ExperimentFlag } from '../../../../cli/dvc/constants' import { EXPERIMENTS_GIT_LOGS_REFS } from '../../../../experiments/data/constants' import { gitPath } from '../../../../cli/git/constants' -import { getGitPath } from '../../../../fileSystem' +import * as FileSystem from '../../../../fileSystem' + +const MOCK_WORKSPACE_GIT_FOLDER = join(dvcDemoPath, '.mock-git') suite('Experiments Data Test Suite', () => { const disposable = getTimeSafeDisposer() @@ -41,6 +43,10 @@ suite('Experiments Data Test Suite', () => { // eslint-disable-next-line sonarjs/cognitive-complexity describe('ExperimentsData', () => { + afterEach(() => { + removeSync(MOCK_WORKSPACE_GIT_FOLDER) + }) + it('should debounce all calls to update that are made within 200ms', async () => { const { data, mockExperimentShow } = buildExperimentsData(disposable) @@ -99,19 +105,23 @@ suite('Experiments Data Test Suite', () => { data.onDidUpdate(() => resolve(undefined)) ) - await Watcher.fireWatcher(getGitPath(gitRoot, gitPath.DOT_GIT_HEAD)) + await Watcher.fireWatcher( + FileSystem.getGitPath(gitRoot, gitPath.DOT_GIT_HEAD) + ) await dataUpdatedEvent expect(managedUpdateSpy).to.be.called }) - it('should watch the .git directory for updates when directory is inside workspace', async () => { + it('should watch the .git directory for updates when the directory is inside workspace', async () => { const mockNow = getMockNow() const mockGitRoot = dvcDemoPath - const mockDotGitPath = join(dvcDemoPath, '.mock-git') - const mockDotGitFilePath = join(mockDotGitPath, gitPath.DOT_GIT_HEAD) + const mockDotGitFilePath = join( + MOCK_WORKSPACE_GIT_FOLDER, + gitPath.DOT_GIT_HEAD + ) const mockDotGitNestedFilePath = join( - mockDotGitPath, + MOCK_WORKSPACE_GIT_FOLDER, EXPERIMENTS_GIT_LOGS_REFS, 'index' ) @@ -124,6 +134,8 @@ suite('Experiments Data Test Suite', () => { return Promise.resolve(mockGitRoot) } } + stub(FileSystem, 'getGitPath').returns(MOCK_WORKSPACE_GIT_FOLDER) + const data = disposable.track( new ExperimentsData( dvcDemoPath, @@ -131,8 +143,7 @@ suite('Experiments Data Test Suite', () => { dispose: stub(), executeCommand: mockExecuteCommand } as unknown as InternalCommands, - disposable.track(new EventEmitter()), - mockDotGitPath + disposable.track(new EventEmitter()) ) ) @@ -153,8 +164,6 @@ suite('Experiments Data Test Suite', () => { await dataUpdatedEvent expect(managedUpdateSpy).to.be.called - - removeSync(mockDotGitPath) }) it('should not use exp show to fetch git refs external to the workspace if the path is not from a temp workspace', async () => { From a5bd0b43bca79b469b3c3f5b819340e34a60ddaf Mon Sep 17 00:00:00 2001 From: julieg18 Date: Fri, 10 Feb 2023 13:01:11 -0600 Subject: [PATCH 3/3] Fix typo --- .../test/suite/experiments/data/index.test.ts | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/extension/src/test/suite/experiments/data/index.test.ts b/extension/src/test/suite/experiments/data/index.test.ts index 7c267c0df9..99e59cd1f7 100644 --- a/extension/src/test/suite/experiments/data/index.test.ts +++ b/extension/src/test/suite/experiments/data/index.test.ts @@ -80,18 +80,16 @@ suite('Experiments Data Test Suite', () => { const mockNow = getMockNow() const gitRoot = dvcDemoPath - const mockExecuteCommand = (command: CommandId) => { - if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { - return Promise.resolve(gitRoot) - } - } - const data = disposable.track( new ExperimentsData( dvcDemoPath, { dispose: stub(), - executeCommand: mockExecuteCommand + executeCommand: (command: CommandId) => { + if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { + return Promise.resolve(gitRoot) + } + } } as unknown as InternalCommands, disposable.track(new EventEmitter()) ) @@ -115,7 +113,7 @@ suite('Experiments Data Test Suite', () => { it('should watch the .git directory for updates when the directory is inside workspace', async () => { const mockNow = getMockNow() - const mockGitRoot = dvcDemoPath + const gitRoot = dvcDemoPath const mockDotGitFilePath = join( MOCK_WORKSPACE_GIT_FOLDER, gitPath.DOT_GIT_HEAD @@ -129,11 +127,6 @@ suite('Experiments Data Test Suite', () => { ensureFileSync(mockDotGitFilePath) ensureFileSync(mockDotGitNestedFilePath) - const mockExecuteCommand = (command: CommandId) => { - if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { - return Promise.resolve(mockGitRoot) - } - } stub(FileSystem, 'getGitPath').returns(MOCK_WORKSPACE_GIT_FOLDER) const data = disposable.track( @@ -141,7 +134,11 @@ suite('Experiments Data Test Suite', () => { dvcDemoPath, { dispose: stub(), - executeCommand: mockExecuteCommand + executeCommand: (command: CommandId) => { + if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { + return Promise.resolve(gitRoot) + } + } } as unknown as InternalCommands, disposable.track(new EventEmitter()) )