Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add regression test for .git file watcher #3259

Merged
merged 5 commits into from
Feb 11, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 70 additions & 9 deletions extension/src/test/suite/experiments/data/index.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -24,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()
Expand All @@ -37,7 +41,12 @@ suite('Experiments Data Test Suite', () => {
return disposable.disposeAndFlush()
})

// 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)

Expand Down Expand Up @@ -67,22 +76,69 @@ 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

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: (command: CommandId) => {
if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) {
return Promise.resolve(gitRoot)
}
}
} as unknown as InternalCommands,
disposable.track(new EventEmitter<boolean>())
)
)

await data.isReady()
bypassProcessManagerDebounce(mockNow)

const managedUpdateSpy = spy(data, 'managedUpdate')
const dataUpdatedEvent = new Promise(resolve =>
data.onDidUpdate(() => resolve(undefined))
)

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 the directory is inside workspace', async () => {
const mockNow = getMockNow()
const gitRoot = dvcDemoPath
const mockDotGitFilePath = join(
MOCK_WORKSPACE_GIT_FOLDER,
gitPath.DOT_GIT_HEAD
)
const mockDotGitNestedFilePath = join(
MOCK_WORKSPACE_GIT_FOLDER,
EXPERIMENTS_GIT_LOGS_REFS,
'index'
)

ensureFileSync(mockDotGitFilePath)
ensureFileSync(mockDotGitNestedFilePath)

stub(FileSystem, 'getGitPath').returns(MOCK_WORKSPACE_GIT_FOLDER)

const data = disposable.track(
new ExperimentsData(
dvcDemoPath,
{
dispose: stub(),
executeCommand: mockExecuteCommand
executeCommand: (command: CommandId) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up moving the executeCommand to be inline to avoid the no-identical-function eslint rule. I could be wrong, but when I tried to create a single function for the two to share it looked like I was overcomplicating things 🤔

if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) {
return Promise.resolve(gitRoot)
}
}
} as unknown as InternalCommands,
disposable.track(new EventEmitter<boolean>())
)
Expand All @@ -96,7 +152,12 @@ suite('Experiments Data Test Suite', () => {
data.onDidUpdate(() => resolve(undefined))
)

await Watcher.fireWatcher(getGitPath(gitRoot, gitPath.DOT_GIT_HEAD))
await Watcher.fireWatcher(mockDotGitFilePath)
await dataUpdatedEvent

expect(managedUpdateSpy).to.be.called

await Watcher.fireWatcher(mockDotGitNestedFilePath)
await dataUpdatedEvent

expect(managedUpdateSpy).to.be.called
Expand Down