Skip to content

Commit

Permalink
test and expand
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Jul 17, 2023
1 parent ed8cfad commit 740ba9f
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 38 deletions.
28 changes: 15 additions & 13 deletions extension/src/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,23 @@ export abstract class BaseData<
return createFileSystemWatcher(
disposable => this.dispose.track(disposable),
getRelativePattern(this.dvcRoot, '**'),
path => {
const relPath = relative(this.dvcRoot, path)
if (
this.getWatchedFiles().some(
watchedRelPath =>
path.endsWith(watchedRelPath) ||
isSameOrChild(relPath, watchedRelPath)
) &&
!isPathInSubProject(relPath, this.relSubProjects)
) {
void this.managedUpdate(path)
}
}
path => this.listener(path)
)
}

private listener(path: string) {
const relPath = relative(this.dvcRoot, path)
if (
this.getWatchedFiles().some(
watchedRelPath =>
path.endsWith(watchedRelPath) ||
isSameOrChild(relPath, watchedRelPath)
) &&
!isPathInSubProject(relPath, this.relSubProjects)
) {
void this.managedUpdate(path)
}
}

abstract managedUpdate(path?: string): Promise<void>
}
2 changes: 1 addition & 1 deletion extension/src/fileSystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const isPathInProject = (
path: string | undefined,
dvcRoot: string,
subProjects: string[]
): path is string =>
): boolean =>
!!path?.startsWith(dvcRoot) &&
path !== dvcRoot &&
!isPathInSubProject(path, subProjects)
Expand Down
2 changes: 1 addition & 1 deletion extension/src/pipeline/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const setContextForEditorTitleIcons = (

const activePath = window.activeTextEditor?.document?.fileName
if (isPathInProject(activePath, dvcRoot, subProjects)) {
setActiveEditorContext(activePath)
setActiveEditorContext(activePath as string)
}

disposer.track(
Expand Down
2 changes: 2 additions & 0 deletions extension/src/pipeline/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ export class PipelineData extends BaseData<{
])

const dvcYamlsDirs = new Set<string>()
this.collectedFiles = []
for (const file of fileList) {
if (isPathInProject(file, this.dvcRoot, this.subProjects)) {
this.collectedFiles.push(file)
dvcYamlsDirs.add(dirname(file))
}
}
Expand Down
24 changes: 17 additions & 7 deletions extension/src/repository/data/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { isExcluded } from '.'
describe('isExcluded', () => {
it('should exclude empty paths', () => {
const mockedDvcRoot = resolve('some', 'dvc', 'root')
const excluded = isExcluded(mockedDvcRoot, '')
const excluded = isExcluded(mockedDvcRoot, '', [])

expect(excluded).toBe(true)
})
Expand All @@ -13,7 +13,7 @@ describe('isExcluded', () => {
const mockedDvcRoot = __dirname
const path = join(mockedDvcRoot, '.git', 'refs', 'exps', '0F')

const excluded = isExcluded(mockedDvcRoot, path)
const excluded = isExcluded(mockedDvcRoot, path, [])

expect(excluded).toBe(true)
})
Expand All @@ -22,23 +22,33 @@ describe('isExcluded', () => {
const mockedDvcRoot = resolve('some', 'dvc', 'repo')
const path = join(mockedDvcRoot, 'data', 'placeholder.dvc')

const excluded = isExcluded(mockedDvcRoot, path)
const excluded = isExcluded(mockedDvcRoot, path, [])

expect(excluded).toBe(false)
})

it('should exclude paths from inside the sub-projects', () => {
const mockedDvcRoot = resolve('some', 'dvc', 'repo')
const subProject = join(mockedDvcRoot, 'data')
const path = join(subProject, 'placeholder.dvc')

const excluded = isExcluded(mockedDvcRoot, path, [subProject])

expect(excluded).toBe(true)
})

it('should not exclude .git/index (that is above the dvc root)', () => {
const path = resolve(__dirname, '..', '..', '.git', 'index')

const excluded = isExcluded(__dirname, path)
const excluded = isExcluded(__dirname, path, [])

expect(excluded).toBe(false)
})

it('should not exclude .git/ORIG_HEAD (that is above the dvc root)', () => {
const path = resolve(__dirname, '..', '..', '.git', 'ORIG_HEAD')

const excluded = isExcluded(__dirname, path)
const excluded = isExcluded(__dirname, path, [])

expect(excluded).toBe(false)
})
Expand All @@ -56,15 +66,15 @@ describe('isExcluded', () => {
'ref'
)

const excluded = isExcluded(__dirname, path)
const excluded = isExcluded(__dirname, path, [])

expect(excluded).toBe(true)
})

it('should exclude paths that are above the dvc root and not in the .git folder', () => {
const path = resolve(__dirname, '..', '..', 'other', 'refs')

const excluded = isExcluded(__dirname, path)
const excluded = isExcluded(__dirname, path, [])

expect(excluded).toBe(true)
})
Expand Down
20 changes: 15 additions & 5 deletions extension/src/repository/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ import {
import { DeferredDisposable } from '../../class/deferred'
import { gitPath } from '../../cli/git/constants'
import { DataStatusOutput, DvcError } from '../../cli/dvc/contract'
import { getGitPath } from '../../fileSystem'
import { getGitPath, isPathInProject } from '../../fileSystem'

export type Data = {
dataStatus: DataStatusOutput | DvcError
hasGitChanges: boolean
untracked: Set<string>
}

export const isExcluded = (dvcRoot: string, path: string) =>
export const isExcluded = (
dvcRoot: string,
path: string,
subProjects: string[]
) =>
!path ||
!(
path.includes(dvcRoot) ||
isPathInProject(path, dvcRoot, subProjects) ||
(path.includes('.git') && (path.includes('HEAD') || path.includes('index')))
) ||
path.includes(EXPERIMENTS_GIT_REFS) ||
Expand All @@ -35,6 +39,7 @@ export class RepositoryData extends DeferredDisposable {
public readonly onDidUpdate: Event<Data>

private readonly dvcRoot: string
private readonly subProjects: string[]

private readonly processManager: ProcessManager
private readonly internalCommands: InternalCommands
Expand All @@ -43,10 +48,15 @@ export class RepositoryData extends DeferredDisposable {
new EventEmitter()
)

constructor(dvcRoot: string, internalCommands: InternalCommands) {
constructor(
dvcRoot: string,
internalCommands: InternalCommands,
subProjects: string[]
) {
super()

this.dvcRoot = dvcRoot
this.subProjects = subProjects
this.processManager = this.dispose.track(
new ProcessManager({
name: 'update',
Expand Down Expand Up @@ -114,7 +124,7 @@ export class RepositoryData extends DeferredDisposable {
disposable => this.dispose.track(disposable),
getRelativePattern(this.dvcRoot, '**'),
(path: string) => {
if (isExcluded(this.dvcRoot, path)) {
if (isExcluded(this.dvcRoot, path, this.subProjects)) {
return
}
return this.managedUpdate()
Expand Down
5 changes: 3 additions & 2 deletions extension/src/repository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ export class Repository extends DeferredDisposable {
constructor(
dvcRoot: string,
internalCommands: InternalCommands,
treeDataChanged: EventEmitter<void>
treeDataChanged: EventEmitter<void>,
subProjects: string[]
) {
super()

this.dvcRoot = dvcRoot
this.model = this.dispose.track(new RepositoryModel(dvcRoot))
this.data = this.dispose.track(
new RepositoryData(dvcRoot, internalCommands)
new RepositoryData(dvcRoot, internalCommands, subProjects)
)

this.errorDecorationProvider = this.dispose.track(
Expand Down
9 changes: 7 additions & 2 deletions extension/src/repository/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ export class WorkspaceRepositories extends BaseWorkspace<Repository> {
return cwd
}

public createRepository(dvcRoot: string): Repository {
public createRepository(dvcRoot: string, subProjects: string[]): Repository {
const repository = this.dispose.track(
new Repository(dvcRoot, this.internalCommands, this.treeDataChanged)
new Repository(
dvcRoot,
this.internalCommands,
this.treeDataChanged,
subProjects
)
)

this.setRepository(dvcRoot, repository)
Expand Down
97 changes: 97 additions & 0 deletions extension/src/test/suite/pipeline/data.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { join } from 'path'
import { afterEach, beforeEach, describe, it, suite } from 'mocha'
import { restore, stub } from 'sinon'
import { expect } from 'chai'
import { Disposable } from '../../../extension'
import { PipelineData } from '../../../pipeline/data'
import { buildDependencies } from '../util'
import { dvcDemoPath } from '../../util'

suite('Pipeline Data Test Suite', () => {
const disposable = Disposable.fn()

beforeEach(() => {
restore()
})

afterEach(function () {
this.timeout(6000)
return disposable.dispose()
})

const buildPipelineData = async (
dvcYamls: string[],
subProjects: string[] = []
) => {
const { internalCommands } = buildDependencies({
disposer: disposable
})

const data = disposable.track(
new PipelineData(dvcDemoPath, internalCommands, subProjects)
)

const mockMangedUpdate = stub(data, 'managedUpdate').resolves(undefined)

// eslint-disable-next-line @typescript-eslint/no-explicit-any
stub(data as any, 'findDvcYamls').resolves(dvcYamls)

await data.update()

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const listener = (path: string) => (data as any).listener(path)

return {
data,
listener,
mockMangedUpdate
}
}

describe('PipelineData', () => {
it('should call managedUpdate for the found dvc.yaml files', async () => {
const dvcYaml = join(dvcDemoPath, 'dvc.yaml')
const nestedDvcYaml = join(dvcDemoPath, 'data', 'dvc.yaml')

const { listener, mockMangedUpdate } = await buildPipelineData([
dvcYaml,
nestedDvcYaml
])

listener(dvcYaml)
expect(
mockMangedUpdate,
'should call managedUpdate for a dvc.yaml inside of the project'
).to.be.calledOnce
mockMangedUpdate.resetHistory()

listener(nestedDvcYaml)
expect(
mockMangedUpdate,
'should call managedUpdate for a nested dvc.yaml'
).to.be.calledOnce
})
})

it('should not call managedUpdate for a dvc.yaml in a sub-project', async () => {
const dvcYaml = join(dvcDemoPath, 'dvc.yaml')
const subProject = join(dvcDemoPath, 'data')
const nestedDvcYaml = join(subProject, 'dvc.yaml')

const { listener, mockMangedUpdate } = await buildPipelineData(
[dvcYaml, nestedDvcYaml],
[subProject]
)

listener(dvcYaml)
expect(
mockMangedUpdate,
'should call managedUpdate for a dvc.yaml inside of the project'
).to.be.calledOnce
mockMangedUpdate.resetHistory()

listener(nestedDvcYaml)
expect(mockMangedUpdate, 'should call managedUpdate for a nested dvc.yaml')
.not.to.be.calledOnce
})
})
12 changes: 8 additions & 4 deletions extension/src/test/suite/repository/data/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@ suite('Repository Data Test Suite', () => {
}

const data = disposable.track(
new RepositoryData(dvcDemoPath, {
dispose: stub(),
executeCommand: mockExecuteCommand
} as unknown as InternalCommands)
new RepositoryData(
dvcDemoPath,
{
dispose: stub(),
executeCommand: mockExecuteCommand
} as unknown as InternalCommands,
[]
)
)
await data.isReady()

Expand Down
3 changes: 2 additions & 1 deletion extension/src/test/suite/repository/model/tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ suite('Repositories Tree Test Suite', () => {
new Repository(
dvcDemoPath,
internalCommands,
disposable.track(new EventEmitter<void>())
disposable.track(new EventEmitter<void>()),
[]
)
)

Expand Down
6 changes: 4 additions & 2 deletions extension/src/test/suite/repository/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ export const buildRepositoryData = async (disposer: SafeWatcherDisposer) => {
mockGetAllUntracked.resolves(new Set())
mockNow.returns(FIRST_TRUTHY_TIME)

const data = disposer.track(new RepositoryData(dvcDemoPath, internalCommands))
const data = disposer.track(
new RepositoryData(dvcDemoPath, internalCommands, [])
)
await data.isReady()

mockDataStatus.resetHistory()
Expand All @@ -77,7 +79,7 @@ export const buildRepository = async (
dvcRoot = dvcDemoPath
) => {
const repository = disposer.track(
new Repository(dvcRoot, internalCommands, treeDataChanged)
new Repository(dvcRoot, internalCommands, treeDataChanged, [])
)

const setErrorDecorationStateSpy = spyOnPrivateMemberMethod(
Expand Down

0 comments on commit 740ba9f

Please sign in to comment.