From a8e982f612f679562854ac5e74d2518a982f5e87 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Tue, 6 Apr 2021 17:33:38 -0400 Subject: [PATCH 1/9] Refactor commands to use Config object instead of destructuring --- extension/src/__mocks__/vscode.ts | 6 +- extension/src/cli/commands.ts | 7 +- extension/src/cli/index.test.ts | 7 +- extension/src/cli/index.ts | 104 ++++++++++++++++++--- extension/src/cli/reader.test.ts | 64 +++++++------ extension/src/cli/reader.ts | 70 -------------- extension/src/extension.ts | 50 ++-------- extension/src/fileSystem.test.ts | 28 ++++-- extension/src/fileSystem.ts | 24 +++-- extension/src/git.test.ts | 4 +- extension/src/test/suite/extension.test.ts | 2 +- package.json | 6 +- yarn.lock | 5 + 13 files changed, 182 insertions(+), 195 deletions(-) delete mode 100644 extension/src/cli/reader.ts diff --git a/extension/src/__mocks__/vscode.ts b/extension/src/__mocks__/vscode.ts index 4e3790fecf..76db3d441a 100644 --- a/extension/src/__mocks__/vscode.ts +++ b/extension/src/__mocks__/vscode.ts @@ -1,4 +1,5 @@ import { join } from 'path' +import { URI, Utils } from 'vscode-uri' export const Extension = jest.fn() export const extensions = jest.fn() @@ -13,4 +14,7 @@ export const workspace = { } ] } -export const Uri = { file: jest.fn().mockImplementation(file => file) } +export const Uri = { + file: jest.fn(URI.file), + joinPath: jest.fn(Utils.joinPath) +} diff --git a/extension/src/cli/commands.ts b/extension/src/cli/commands.ts index f7d926363d..fec4347cca 100644 --- a/extension/src/cli/commands.ts +++ b/extension/src/cli/commands.ts @@ -4,7 +4,8 @@ export enum Commands { initialize_subdirectory = 'init --subdir', add = 'add', checkout = 'checkout', - checkout_recursive = 'checkout --recursive' + checkout_recursive = 'checkout --recursive', + root = 'root' } const getCliCommand = (command: string, ...options: string[]): string => { @@ -14,7 +15,3 @@ const getCliCommand = (command: string, ...options: string[]): string => { export const getRunExperimentCommand = (): string => { return getCliCommand(Commands.experiment_run) } - -export const getAddCommand = (toAdd: string): string => { - return `${Commands.add} ${toAdd}` -} diff --git a/extension/src/cli/index.test.ts b/extension/src/cli/index.test.ts index e0d2bb5d34..d57e74f09b 100644 --- a/extension/src/cli/index.test.ts +++ b/extension/src/cli/index.test.ts @@ -2,6 +2,7 @@ import { mocked } from 'ts-jest/utils' import { execPromise } from '../util' import { basename, resolve } from 'path' import { add } from '.' +import { Config } from '../Config' jest.mock('fs') jest.mock('../util') @@ -16,6 +17,7 @@ describe('add', () => { it('should call execPromise with the correct parameters', async () => { const fsPath = __filename const dir = resolve(fsPath, '..') + const mockConfig = { dvcPath: 'dvc', workspaceRoot: dir } as Config const file = basename(__filename) const stdout = `100% Add|████████████████████████████████████████████████` + @@ -30,10 +32,7 @@ describe('add', () => { stderr: '' }) - const output = await add({ - cliPath: 'dvc', - fsPath - }) + const output = await add(mockConfig, fsPath) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith(`dvc add ${file}`, { diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index d829569a13..9d673b9927 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -1,18 +1,96 @@ -import { basename, dirname } from 'path' -import { getAddCommand } from './commands' -import { execCommand } from './reader' +import { basename } from 'path' +import { Commands } from './commands' +import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' +import { getPythonExecutionDetails } from '../extensions/python' +import { commands } from 'vscode' +import { Disposer } from '@hediet/std/disposable' +import { Config } from '../Config' +import { execPromise } from '../util' -export const add = async (options: { - fsPath: string - cliPath: string | undefined -}): Promise => { - const { fsPath, cliPath } = options - - const cwd = dirname(fsPath) +export const getDvcInvocation = async (config: Config) => { + const dvcPath = config.dvcPath + if (dvcPath) { + return dvcPath + } + const executionDetails = await getPythonExecutionDetails() + return executionDetails ? `${executionDetails.join(' ')} -m dvc` : 'dvc' +} +export const execCommand = async ( + config: Config, + command: string, + execPromiseConfig = { + cwd: config.workspaceRoot + } +): Promise => { + const fullCommandString = `${await getDvcInvocation(config)} ${command}` + return (await execPromise(fullCommandString, execPromiseConfig)).stdout +} +export const add = (config: Config, fsPath: string): Promise => { const toAdd = basename(fsPath) - const addCommand = getAddCommand(toAdd) - const { stdout } = await execCommand({ cwd, cliPath }, addCommand) - return stdout + return execCommand(config, `add ${toAdd}`) +} + +export const getExperiments = async ( + config: Config +): Promise => { + const output = await execCommand(config, Commands.experiment_show) + return JSON.parse(output) +} + +export const initializeDirectory = async (config: Config): Promise => { + return execCommand(config, Commands.initialize_subdirectory) +} + +export const checkout = async (config: Config): Promise => { + return execCommand(config, Commands.checkout) +} + +export const checkoutRecursive = async (config: Config): Promise => { + return execCommand(config, Commands.checkout_recursive) +} + +export const getRoot = async (config: Config, cwd: string): Promise => { + const output = await execCommand(config, Commands.root, { cwd }) + return output.trim() +} + +export const listDvcOnlyRecursive = async ( + config: Config +): Promise => { + const output = await execCommand(config, `list . --dvc-only -R`) + return output.trim().split('\n') +} + +export const registerDvcCommands = ({ + dispose, + config +}: { + dispose: Disposer + config: Config +}) => { + dispose.track( + commands.registerCommand('dvc.initializeDirectory', () => { + initializeDirectory(config) + }) + ) + + dispose.track( + commands.registerCommand('dvc.add', ({ resourceUri }) => + add(config, resourceUri) + ) + ) + + dispose.track( + commands.registerCommand('dvc.checkout', () => { + checkout(config) + }) + ) + + dispose.track( + commands.registerCommand('dvc.checkoutRecursive', () => { + checkoutRecursive(config) + }) + ) } diff --git a/extension/src/cli/reader.test.ts b/extension/src/cli/reader.test.ts index f722d32743..7bb6c79d7d 100644 --- a/extension/src/cli/reader.test.ts +++ b/extension/src/cli/reader.test.ts @@ -7,11 +7,12 @@ import { getRoot, listDvcOnlyRecursive, getDvcInvocation -} from './reader' +} from './index' import { execPromise } from '../util' import complexExperimentsOutput from '../webviews/experiments/complex-output-example.json' import { join, resolve } from 'path' import { getPythonExecutionDetails } from '../extensions/python' +import { Config } from '../Config' jest.mock('fs') jest.mock('../util') @@ -28,40 +29,45 @@ describe('getDvcInvocation', () => { it('should utilize an interpreter path from the Python extension by default', async () => { const testPythonBin = '/custom/path/to/python' mockedGetPythonExecutionDetails.mockResolvedValue([testPythonBin]) - expect(await getDvcInvocation({ cliPath: '', cwd: './' })).toEqual( - `${testPythonBin} -m dvc` - ) + expect( + await getDvcInvocation({ dvcPath: '', workspaceRoot: './' } as Config) + ).toEqual(`${testPythonBin} -m dvc`) }) - it('should ignore a path from the Python extension when cliPath is defined', async () => { + it('should ignore a path from the Python extension when dvcPath is defined', async () => { const testPythonBin = '/custom/path/to/python' mockedGetPythonExecutionDetails.mockResolvedValue(['/wrong/python/bin']) expect( - await getDvcInvocation({ cliPath: testPythonBin, cwd: './' }) + await getDvcInvocation({ + dvcPath: testPythonBin, + workspaceRoot: './' + } as Config) ).toEqual(testPythonBin) }) it('should return a simple dvc call when no Python extension is present', async () => { mockedGetPythonExecutionDetails.mockResolvedValue(undefined) - expect(await getDvcInvocation({ cliPath: '', cwd: './' })).toEqual('dvc') + expect( + await getDvcInvocation({ dvcPath: '', workspaceRoot: './' } as Config) + ).toEqual('dvc') }) }) describe('getExperiments', () => { it('should match a snapshot when parsed', async () => { - const cwd = resolve() + const workspaceRoot = resolve() mockedExecPromise.mockResolvedValueOnce({ stdout: JSON.stringify(complexExperimentsOutput), stderr: '' }) const experiments = await getExperiments({ - cliPath: 'dvc', - cwd - }) + dvcPath: 'dvc', + workspaceRoot + } as Config) expect(experiments).toMatchSnapshot() expect(mockedExecPromise).toBeCalledWith('dvc exp show --show-json', { - cwd + cwd: workspaceRoot }) }) }) @@ -94,9 +100,9 @@ describe('initializeDirectory', () => { }) const output = await initializeDirectory({ - cliPath: 'dvc', - cwd: fsPath - }) + dvcPath: 'dvc', + workspaceRoot: fsPath + } as Config) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc init --subdir', { @@ -114,9 +120,9 @@ describe('checkout', () => { }) const output = await checkout({ - cliPath: 'dvc', - cwd: fsPath - }) + dvcPath: 'dvc', + workspaceRoot: fsPath + } as Config) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc checkout', { @@ -135,9 +141,9 @@ describe('checkoutRecursive', () => { }) const output = await checkoutRecursive({ - cliPath: 'dvc', - cwd: fsPath - }) + dvcPath: 'dvc', + workspaceRoot: fsPath + } as Config) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc checkout --recursive', { @@ -148,6 +154,7 @@ describe('checkoutRecursive', () => { describe('getRoot', () => { it('should return the root relative to the cwd', async () => { + const mockConfig = { dvcPath: 'dvc' } as Config const mockRelativeRoot = join('..', '..') const mockStdout = mockRelativeRoot + '\n\r' const cwd = resolve() @@ -155,10 +162,7 @@ describe('getRoot', () => { stdout: mockStdout, stderr: '' }) - const relativeRoot = await getRoot({ - cwd, - cliPath: 'dvc' - }) + const relativeRoot = await getRoot(mockConfig, cwd) expect(relativeRoot).toEqual(mockRelativeRoot) expect(mockedExecPromise).toBeCalledWith('dvc root', { cwd @@ -180,15 +184,15 @@ describe('getTracked', () => { `logs/acc.tsv\n` + `logs/loss.tsv\n` + `model.pt` - const cwd = resolve() + const workspaceRoot = resolve() mockedExecPromise.mockResolvedValueOnce({ stdout: stdout, stderr: '' }) const tracked = await listDvcOnlyRecursive({ - cwd, - cliPath: 'dvc' - }) + workspaceRoot, + dvcPath: 'dvc' + } as Config) expect(tracked).toEqual([ 'data/MNIST/raw/t10k-images-idx3-ubyte', @@ -205,7 +209,7 @@ describe('getTracked', () => { ]) expect(mockedExecPromise).toBeCalledWith('dvc list . --dvc-only -R', { - cwd + cwd: workspaceRoot }) }) }) diff --git a/extension/src/cli/reader.ts b/extension/src/cli/reader.ts deleted file mode 100644 index 4675a65911..0000000000 --- a/extension/src/cli/reader.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { Commands } from './commands' -import { execPromise } from '../util' -import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' -import { getPythonExecutionDetails } from '../extensions/python' - -interface ReaderOptions { - cliPath: string | undefined - cwd: string -} - -export const getDvcInvocation = async (options: ReaderOptions) => { - const { cliPath } = options - if (cliPath) { - return cliPath - } - const executionDetails = await getPythonExecutionDetails() - return executionDetails ? `${executionDetails.join(' ')} -m dvc` : 'dvc' -} - -export const execCommand = async ( - options: ReaderOptions, - command: string -): Promise<{ stdout: string; stderr: string }> => { - const { cwd } = options - const fullCommandString = `${await getDvcInvocation(options)} ${command}` - return execPromise(fullCommandString, { - cwd - }) -} - -export const getExperiments = async ( - options: ReaderOptions -): Promise => { - const { stdout } = await execCommand(options, Commands.experiment_show) - return JSON.parse(stdout) -} - -export const initializeDirectory = async ( - options: ReaderOptions -): Promise => { - const { stdout } = await execCommand( - options, - Commands.initialize_subdirectory - ) - return stdout -} - -export const checkout = async (options: ReaderOptions): Promise => { - const { stdout } = await execCommand(options, Commands.checkout) - return stdout -} - -export const checkoutRecursive = async ( - options: ReaderOptions -): Promise => { - const { stdout } = await execCommand(options, Commands.checkout_recursive) - return stdout -} - -export const getRoot = async (options: ReaderOptions): Promise => { - const { stdout } = await execCommand(options, 'root') - return stdout.trim() -} - -export const listDvcOnlyRecursive = async ( - options: ReaderOptions -): Promise => { - const { stdout } = await execCommand(options, `list . --dvc-only -R`) - return stdout.trim().split('\n') -} diff --git a/extension/src/extension.ts b/extension/src/extension.ts index bd159eafc5..67f7c4e956 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -10,13 +10,7 @@ import { IntegratedTerminal, runExperiment } from './IntegratedTerminal' import { SourceControlManagement } from './views/SourceControlManagement' import { Config } from './Config' import { WebviewManager } from './webviews/WebviewManager' -import { - getExperiments, - initializeDirectory, - checkout, - checkoutRecursive -} from './cli/reader' -import { add } from './cli' +import { getExperiments } from './cli' import { addFileChangeHandler, @@ -60,10 +54,7 @@ export class Extension { } private refreshExperimentsWebview = async () => { - const experiments = await getExperiments({ - cwd: this.config.workspaceRoot, - cliPath: this.config.dvcPath - }) + const experiments = await getExperiments(this.config) return this.webviewManager.refreshExperiments(experiments) } @@ -86,11 +77,9 @@ export class Extension { this.decorationProvider = this.dispose.track(new DecorationProvider()) - findDvcTrackedPaths(this.config.workspaceRoot, this.config.dvcPath).then( - files => { - this.decorationProvider.setTrackedFiles(files) - } - ) + findDvcTrackedPaths(this.config).then(files => { + this.decorationProvider.setTrackedFiles(files) + }) this.webviewManager = this.dispose.track( new WebviewManager(this.config, this.resourceLocator) @@ -118,33 +107,6 @@ export class Extension { }) ) - this.dispose.track( - commands.registerCommand('dvc.initializeDirectory', ({ fsPath }) => { - initializeDirectory({ - cwd: fsPath, - cliPath: this.config.dvcPath - }) - }) - ) - - this.dispose.track( - commands.registerCommand('dvc.add', ({ resourceUri }) => - add({ fsPath: resourceUri.fsPath, cliPath: this.config.dvcPath }) - ) - ) - - this.dispose.track( - commands.registerCommand('dvc.checkout', ({ fsPath }) => { - checkout({ cwd: fsPath, cliPath: this.config.dvcPath }) - }) - ) - - this.dispose.track( - commands.registerCommand('dvc.checkoutRecursive', ({ fsPath }) => { - checkoutRecursive({ cwd: fsPath, cliPath: this.config.dvcPath }) - }) - ) - this.gitExtension = this.dispose.track(new GitExtension()) this.gitExtension.ready.then(() => { @@ -155,7 +117,7 @@ export class Extension { this.dispose.track(disposable) ) - const dvcRoots = await findDvcRootPaths(gitRoot, this.config.dvcPath) + const dvcRoots = await findDvcRootPaths(this.config, gitRoot) dvcRoots.forEach(async dvcRoot => { const untracked = await getAllUntracked(dvcRoot) const scm = this.dispose.track( diff --git a/extension/src/fileSystem.test.ts b/extension/src/fileSystem.test.ts index d687182dd1..6220350b34 100644 --- a/extension/src/fileSystem.test.ts +++ b/extension/src/fileSystem.test.ts @@ -4,7 +4,8 @@ import { mocked } from 'ts-jest/utils' import debounce from 'lodash.debounce' import { dirname, join, resolve } from 'path' import { mkdirSync, rmdir } from 'fs-extra' -import { getRoot, listDvcOnlyRecursive } from './cli/reader' +import { getRoot, listDvcOnlyRecursive } from './cli' +import { Config } from './Config' const { addFileChangeHandler, @@ -16,7 +17,7 @@ const { jest.mock('chokidar') jest.mock('lodash.debounce') -jest.mock('./cli/reader') +jest.mock('./cli') const mockedWatch = mocked(watch) const mockedDebounce = mocked(debounce) @@ -96,11 +97,13 @@ describe('getWatcher', () => { describe('findDvcRootPaths', () => { const dataRoot = resolve(dvcDemoPath, 'data') - const mockCliPath = 'dvc' + const mockConfig = { + dvcPath: 'dvc' + } as Config it('should find the dvc root if it exists in the given folder', async () => { mockGetRoot.mockResolvedValueOnce('.') - const dvcRoots = await findDvcRootPaths(dvcDemoPath, mockCliPath) + const dvcRoots = await findDvcRootPaths(mockConfig, dvcDemoPath) expect(mockGetRoot).toBeCalledTimes(1) expect(dvcRoots).toEqual([dvcDemoPath]) @@ -111,7 +114,7 @@ describe('findDvcRootPaths', () => { const mockDvcRoot = join(dataRoot, '.dvc') mkdirSync(mockDvcRoot) - const dvcRoots = await findDvcRootPaths(dvcDemoPath, mockCliPath) + const dvcRoots = await findDvcRootPaths(mockConfig, dvcDemoPath) rmdir(mockDvcRoot) @@ -124,7 +127,7 @@ describe('findDvcRootPaths', () => { const mockDvcRoot = join(dataRoot, 'MNIST', '.dvc') mkdirSync(mockDvcRoot) - const dvcRoots = await findDvcRootPaths(dataRoot, mockCliPath) + const dvcRoots = await findDvcRootPaths(mockConfig, dataRoot) rmdir(mockDvcRoot) @@ -134,13 +137,13 @@ describe('findDvcRootPaths', () => { it('should find the dvc root if it exists above the given folder', async () => { mockGetRoot.mockResolvedValueOnce('..') - const dvcRoots = await findDvcRootPaths(dataRoot, mockCliPath) + const dvcRoots = await findDvcRootPaths(mockConfig, dataRoot) expect(mockGetRoot).toBeCalledTimes(1) expect(dvcRoots).toEqual([dvcDemoPath]) }) it('should return an empty array given no dvc root in or above the given directory', async () => { - const dvcRoots = await findDvcRootPaths(__dirname, mockCliPath) + const dvcRoots = await findDvcRootPaths(mockConfig, __dirname) expect(dvcRoots).toEqual([]) }) }) @@ -156,9 +159,14 @@ describe('getAbsoluteTrackedPath', () => { }) describe('findDvcTrackedPaths', () => { + const mockConfig = { + dvcPath: 'dvc', + workspaceRoot: dvcDemoPath + } as Config + it('should find the paths in the workspace corresponding to .dvc files and return them in a Set', async () => { mockListDvcOnlyRecursive.mockResolvedValueOnce([]) - const tracked = await findDvcTrackedPaths(dvcDemoPath, 'dvc') + const tracked = await findDvcTrackedPaths(mockConfig) expect(tracked).toEqual( new Set([resolve(dvcDemoPath, 'data', 'MNIST', 'raw')]) @@ -171,7 +179,7 @@ describe('findDvcTrackedPaths', () => { const logLoss = join(logFolder, 'loss.tsv') const model = 'model.pt' mockListDvcOnlyRecursive.mockResolvedValueOnce([logAcc, logLoss, model]) - const tracked = await findDvcTrackedPaths(dvcDemoPath, 'dvc') + const tracked = await findDvcTrackedPaths(mockConfig) expect(tracked).toEqual( new Set([ diff --git a/extension/src/fileSystem.ts b/extension/src/fileSystem.ts index 7bdaca1e44..050a14984e 100644 --- a/extension/src/fileSystem.ts +++ b/extension/src/fileSystem.ts @@ -3,7 +3,8 @@ import chokidar from 'chokidar' import debounce from 'lodash.debounce' import { dirname, join, resolve, basename } from 'path' import glob from 'tiny-glob' -import { getRoot, listDvcOnlyRecursive } from './cli/reader' +import { getRoot, listDvcOnlyRecursive } from './cli' +import { Config } from './Config' export const getWatcher = (handler: () => void) => (path: string): void => { if (path) { @@ -40,14 +41,11 @@ const filterRootDir = (dirs: string[], rootDir: string) => dirs.filter(dir => dir !== rootDir) const findDvcAbsoluteRootPath = async ( - cwd: string, - cliPath: string | undefined + config: Config, + cwd: string ): Promise => { try { - const root = await getRoot({ - cliPath, - cwd - }) + const root = await getRoot(config, cwd) return resolve(cwd, root) } catch (e) {} } @@ -66,12 +64,12 @@ const findDvcSubRootPaths = async (cwd: string): Promise => { } export const findDvcRootPaths = async ( - cwd: string, - cliPath: string | undefined + config: Config, + cwd: string ): Promise => { const [subRoots, absoluteRoot] = await Promise.all([ findDvcSubRootPaths(cwd), - findDvcAbsoluteRootPath(cwd, cliPath) + findDvcAbsoluteRootPath(config, cwd) ]) const roots = [...subRoots, absoluteRoot].filter(v => v).sort() as string[] @@ -93,9 +91,9 @@ const getAbsoluteParentPath = (rootDir: string, files: string[]): string[] => { } export const findDvcTrackedPaths = async ( - cwd: string, - cliPath: string | undefined + config: Config ): Promise> => { + const cwd = config.workspaceRoot const [dotDvcFiles, dvcListFiles] = await Promise.all([ glob(join('**', '*.dvc'), { absolute: true, @@ -104,7 +102,7 @@ export const findDvcTrackedPaths = async ( filesOnly: true }), - listDvcOnlyRecursive({ cwd, cliPath }) + listDvcOnlyRecursive(config) ]) return new Set([ diff --git a/extension/src/git.test.ts b/extension/src/git.test.ts index 7994e5403a..c81de026d9 100644 --- a/extension/src/git.test.ts +++ b/extension/src/git.test.ts @@ -28,7 +28,7 @@ describe('getAllUntracked', () => { await Promise.all([remove(untrackedDir), remove(untrackedPython)]) - expect(gitUntracked).toEqual( + expect(gitUntracked.map(x => x.path)).toEqual( expect.arrayContaining([ untrackedDir, untrackedPerl, @@ -37,7 +37,7 @@ describe('getAllUntracked', () => { ]) ) - expect(dvcUntracked).toEqual( + expect(dvcUntracked.map(x => x.path)).toEqual( expect.arrayContaining([untrackedDir, untrackedPerl, untrackedText]) ) expect(dvcUntracked).not.toEqual(expect.arrayContaining([untrackedPython])) diff --git a/extension/src/test/suite/extension.test.ts b/extension/src/test/suite/extension.test.ts index 1fa698f39d..b001b5fae9 100644 --- a/extension/src/test/suite/extension.test.ts +++ b/extension/src/test/suite/extension.test.ts @@ -11,7 +11,7 @@ import { ConfigurationChangeEvent } from 'vscode' import { Disposable } from '../../extension' -import * as DvcReader from '../../cli/reader' +import * as DvcReader from '../../cli' import complexExperimentsOutput from '../../webviews/experiments/complex-output-example.json' import { ExperimentsWebview } from '../../webviews/experiments/ExperimentsWebview' diff --git a/package.json b/package.json index 72eac652d9..c8436fe5c9 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "extension-dev-server": "yarn workspace dvc dev", "webview-dev-server": "yarn workspace dvc-vscode-webview dev", "dev-server": "run-p extension-dev-server webview-dev-server", - "dev-ui": "HOT_RELOAD=true USE_DEV_UI=true code --extensionDevelopmentPath $PWD/extension demo", + "dev-ui": "HOT_RELOAD=true USE_DEV_UI=true DVCPATH='.env/bin/dvc' code --extensionDevelopmentPath $PWD/extension demo", + "dev-ui-win": "HOT_RELOAD=true USE_DEV_UI=true DVCPATH='.env\\Scripts\\dvc.exe' code --extensionDevelopmentPath $PWD/extension demo", "postinstall": "husky install", "storybook": "yarn workspace dvc-vscode-webview storybook", "build-storybook": "yarn workspace dvc-vscode-webview build-storybook", @@ -52,6 +53,7 @@ "prettier": "^1.19.1", "prettier-config-standard": "^4.0.0", "ts-jest": "^26.4.4", - "typescript": "^4.1.3" + "typescript": "^4.1.3", + "vscode-uri": "^3.0.2" } } diff --git a/yarn.lock b/yarn.lock index 18662b3b2c..53a5460e69 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14129,6 +14129,11 @@ vscode-test@^1.5.1: rimraf "^3.0.2" unzipper "^0.10.11" +vscode-uri@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/vscode-uri/-/vscode-uri-3.0.2.tgz#ecfd1d066cb8ef4c3a208decdbab9a8c23d055d0" + integrity sha512-jkjy6pjU1fxUvI51P+gCsxg1u2n8LSt0W6KrCNQceaziKzff74GoWmjVG46KieVzybO1sttPQmYfrwSHey7GUA== + w3c-hr-time@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/w3c-hr-time/-/w3c-hr-time-1.0.2.tgz#0a89cdf5cc15822df9c360543676963e0cc308cd" From bdf4dd8cc8c25283b4bcbf7be7020c1a066d9e6c Mon Sep 17 00:00:00 2001 From: rogermparent Date: Tue, 6 Apr 2021 17:53:58 -0400 Subject: [PATCH 2/9] Register commands in `extension` and resolve `resourceUri` properly in `add` --- extension/src/cli/index.ts | 7 ++++--- extension/src/extension.ts | 7 ++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 9d673b9927..38dce87b38 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -2,7 +2,7 @@ import { basename } from 'path' import { Commands } from './commands' import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' import { getPythonExecutionDetails } from '../extensions/python' -import { commands } from 'vscode' +import { commands, Uri } from 'vscode' import { Disposer } from '@hediet/std/disposable' import { Config } from '../Config' import { execPromise } from '../util' @@ -77,8 +77,9 @@ export const registerDvcCommands = ({ ) dispose.track( - commands.registerCommand('dvc.add', ({ resourceUri }) => - add(config, resourceUri) + commands.registerCommand( + 'dvc.add', + ({ resourceUri }: { resourceUri: Uri }) => add(config, resourceUri.fsPath) ) ) diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 67f7c4e956..bccb6db384 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -10,7 +10,7 @@ import { IntegratedTerminal, runExperiment } from './IntegratedTerminal' import { SourceControlManagement } from './views/SourceControlManagement' import { Config } from './Config' import { WebviewManager } from './webviews/WebviewManager' -import { getExperiments } from './cli' +import { getExperiments, registerDvcCommands } from './cli' import { addFileChangeHandler, @@ -107,6 +107,11 @@ export class Extension { }) ) + registerDvcCommands({ + dispose: this.dispose, + config: this.config + }) + this.gitExtension = this.dispose.track(new GitExtension()) this.gitExtension.ready.then(() => { From 0309e58a0aa81ee7d1452eaf0a86ab0613b8d19f Mon Sep 17 00:00:00 2001 From: rogermparent Date: Tue, 6 Apr 2021 19:04:14 -0400 Subject: [PATCH 3/9] Remove Commands module Without using this data in multiple places, the wrapper module didn't serve much purpose and hampered code readability a bit. --- extension/src/IntegratedTerminal.test.ts | 2 +- extension/src/IntegratedTerminal.ts | 4 +--- extension/src/cli/commands.ts | 17 ----------------- extension/src/cli/index.ts | 13 ++++++------- 4 files changed, 8 insertions(+), 28 deletions(-) delete mode 100644 extension/src/cli/commands.ts diff --git a/extension/src/IntegratedTerminal.test.ts b/extension/src/IntegratedTerminal.test.ts index 1d750cfc98..047e87b01b 100644 --- a/extension/src/IntegratedTerminal.test.ts +++ b/extension/src/IntegratedTerminal.test.ts @@ -9,6 +9,6 @@ describe('runExperiment', () => { const undef = await runExperiment() expect(undef).toBeUndefined() - expect(terminalSpy).toBeCalledWith('dvc exp run ') + expect(terminalSpy).toBeCalledWith('dvc exp run') }) }) diff --git a/extension/src/IntegratedTerminal.ts b/extension/src/IntegratedTerminal.ts index 116573d52a..8f908f71e0 100644 --- a/extension/src/IntegratedTerminal.ts +++ b/extension/src/IntegratedTerminal.ts @@ -1,5 +1,4 @@ import { Terminal, window, workspace } from 'vscode' -import { getRunExperimentCommand } from './cli/commands' import { getReadyPythonExtension } from './extensions/python' import { delay } from './util' @@ -65,6 +64,5 @@ export class IntegratedTerminal { } export const runExperiment = (): Promise => { - const runExperimentCommand = getRunExperimentCommand() - return IntegratedTerminal.run(runExperimentCommand) + return IntegratedTerminal.run('dvc exp run') } diff --git a/extension/src/cli/commands.ts b/extension/src/cli/commands.ts deleted file mode 100644 index fec4347cca..0000000000 --- a/extension/src/cli/commands.ts +++ /dev/null @@ -1,17 +0,0 @@ -export enum Commands { - experiment_run = 'exp run', - experiment_show = 'exp show --show-json', - initialize_subdirectory = 'init --subdir', - add = 'add', - checkout = 'checkout', - checkout_recursive = 'checkout --recursive', - root = 'root' -} - -const getCliCommand = (command: string, ...options: string[]): string => { - return `dvc ${command} ${options.join(' ')}` -} - -export const getRunExperimentCommand = (): string => { - return getCliCommand(Commands.experiment_run) -} diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 38dce87b38..28e48a1148 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -1,5 +1,4 @@ import { basename } from 'path' -import { Commands } from './commands' import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' import { getPythonExecutionDetails } from '../extensions/python' import { commands, Uri } from 'vscode' @@ -35,31 +34,31 @@ export const add = (config: Config, fsPath: string): Promise => { export const getExperiments = async ( config: Config ): Promise => { - const output = await execCommand(config, Commands.experiment_show) + const output = await execCommand(config, 'exp show --show-json') return JSON.parse(output) } export const initializeDirectory = async (config: Config): Promise => { - return execCommand(config, Commands.initialize_subdirectory) + return execCommand(config, 'init --subdir') } export const checkout = async (config: Config): Promise => { - return execCommand(config, Commands.checkout) + return execCommand(config, 'checkout') } export const checkoutRecursive = async (config: Config): Promise => { - return execCommand(config, Commands.checkout_recursive) + return execCommand(config, 'checkout --recursive') } export const getRoot = async (config: Config, cwd: string): Promise => { - const output = await execCommand(config, Commands.root, { cwd }) + const output = await execCommand(config, 'root', { cwd }) return output.trim() } export const listDvcOnlyRecursive = async ( config: Config ): Promise => { - const output = await execCommand(config, `list . --dvc-only -R`) + const output = await execCommand(config, 'list . --dvc-only -R') return output.trim().split('\n') } From f25817d7f87d773a90a1ced19fe13d32081b7355 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 17:02:37 -0400 Subject: [PATCH 4/9] Revert more destructive parts of the branch Most commands are returned to their original state, proper Uri mocks are kept. --- extension/src/cli/commands.ts | 20 ++++ extension/src/cli/index.test.ts | 7 +- extension/src/cli/index.ts | 104 +++------------------ extension/src/cli/reader.test.ts | 64 ++++++------- extension/src/cli/reader.ts | 70 ++++++++++++++ extension/src/extension.ts | 53 +++++++++-- extension/src/fileSystem.test.ts | 28 ++---- extension/src/fileSystem.ts | 24 ++--- extension/src/git.test.ts | 13 ++- extension/src/test/suite/extension.test.ts | 2 +- 10 files changed, 214 insertions(+), 171 deletions(-) create mode 100644 extension/src/cli/commands.ts create mode 100644 extension/src/cli/reader.ts diff --git a/extension/src/cli/commands.ts b/extension/src/cli/commands.ts new file mode 100644 index 0000000000..f7d926363d --- /dev/null +++ b/extension/src/cli/commands.ts @@ -0,0 +1,20 @@ +export enum Commands { + experiment_run = 'exp run', + experiment_show = 'exp show --show-json', + initialize_subdirectory = 'init --subdir', + add = 'add', + checkout = 'checkout', + checkout_recursive = 'checkout --recursive' +} + +const getCliCommand = (command: string, ...options: string[]): string => { + return `dvc ${command} ${options.join(' ')}` +} + +export const getRunExperimentCommand = (): string => { + return getCliCommand(Commands.experiment_run) +} + +export const getAddCommand = (toAdd: string): string => { + return `${Commands.add} ${toAdd}` +} diff --git a/extension/src/cli/index.test.ts b/extension/src/cli/index.test.ts index d57e74f09b..e0d2bb5d34 100644 --- a/extension/src/cli/index.test.ts +++ b/extension/src/cli/index.test.ts @@ -2,7 +2,6 @@ import { mocked } from 'ts-jest/utils' import { execPromise } from '../util' import { basename, resolve } from 'path' import { add } from '.' -import { Config } from '../Config' jest.mock('fs') jest.mock('../util') @@ -17,7 +16,6 @@ describe('add', () => { it('should call execPromise with the correct parameters', async () => { const fsPath = __filename const dir = resolve(fsPath, '..') - const mockConfig = { dvcPath: 'dvc', workspaceRoot: dir } as Config const file = basename(__filename) const stdout = `100% Add|████████████████████████████████████████████████` + @@ -32,7 +30,10 @@ describe('add', () => { stderr: '' }) - const output = await add(mockConfig, fsPath) + const output = await add({ + cliPath: 'dvc', + fsPath + }) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith(`dvc add ${file}`, { diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 28e48a1148..d829569a13 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -1,96 +1,18 @@ -import { basename } from 'path' -import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' -import { getPythonExecutionDetails } from '../extensions/python' -import { commands, Uri } from 'vscode' -import { Disposer } from '@hediet/std/disposable' -import { Config } from '../Config' -import { execPromise } from '../util' +import { basename, dirname } from 'path' +import { getAddCommand } from './commands' +import { execCommand } from './reader' -export const getDvcInvocation = async (config: Config) => { - const dvcPath = config.dvcPath - if (dvcPath) { - return dvcPath - } - const executionDetails = await getPythonExecutionDetails() - return executionDetails ? `${executionDetails.join(' ')} -m dvc` : 'dvc' -} - -export const execCommand = async ( - config: Config, - command: string, - execPromiseConfig = { - cwd: config.workspaceRoot - } -): Promise => { - const fullCommandString = `${await getDvcInvocation(config)} ${command}` - return (await execPromise(fullCommandString, execPromiseConfig)).stdout -} -export const add = (config: Config, fsPath: string): Promise => { - const toAdd = basename(fsPath) - - return execCommand(config, `add ${toAdd}`) -} - -export const getExperiments = async ( - config: Config -): Promise => { - const output = await execCommand(config, 'exp show --show-json') - return JSON.parse(output) -} - -export const initializeDirectory = async (config: Config): Promise => { - return execCommand(config, 'init --subdir') -} +export const add = async (options: { + fsPath: string + cliPath: string | undefined +}): Promise => { + const { fsPath, cliPath } = options -export const checkout = async (config: Config): Promise => { - return execCommand(config, 'checkout') -} - -export const checkoutRecursive = async (config: Config): Promise => { - return execCommand(config, 'checkout --recursive') -} - -export const getRoot = async (config: Config, cwd: string): Promise => { - const output = await execCommand(config, 'root', { cwd }) - return output.trim() -} + const cwd = dirname(fsPath) -export const listDvcOnlyRecursive = async ( - config: Config -): Promise => { - const output = await execCommand(config, 'list . --dvc-only -R') - return output.trim().split('\n') -} - -export const registerDvcCommands = ({ - dispose, - config -}: { - dispose: Disposer - config: Config -}) => { - dispose.track( - commands.registerCommand('dvc.initializeDirectory', () => { - initializeDirectory(config) - }) - ) - - dispose.track( - commands.registerCommand( - 'dvc.add', - ({ resourceUri }: { resourceUri: Uri }) => add(config, resourceUri.fsPath) - ) - ) - - dispose.track( - commands.registerCommand('dvc.checkout', () => { - checkout(config) - }) - ) + const toAdd = basename(fsPath) + const addCommand = getAddCommand(toAdd) - dispose.track( - commands.registerCommand('dvc.checkoutRecursive', () => { - checkoutRecursive(config) - }) - ) + const { stdout } = await execCommand({ cwd, cliPath }, addCommand) + return stdout } diff --git a/extension/src/cli/reader.test.ts b/extension/src/cli/reader.test.ts index 7bb6c79d7d..f722d32743 100644 --- a/extension/src/cli/reader.test.ts +++ b/extension/src/cli/reader.test.ts @@ -7,12 +7,11 @@ import { getRoot, listDvcOnlyRecursive, getDvcInvocation -} from './index' +} from './reader' import { execPromise } from '../util' import complexExperimentsOutput from '../webviews/experiments/complex-output-example.json' import { join, resolve } from 'path' import { getPythonExecutionDetails } from '../extensions/python' -import { Config } from '../Config' jest.mock('fs') jest.mock('../util') @@ -29,45 +28,40 @@ describe('getDvcInvocation', () => { it('should utilize an interpreter path from the Python extension by default', async () => { const testPythonBin = '/custom/path/to/python' mockedGetPythonExecutionDetails.mockResolvedValue([testPythonBin]) - expect( - await getDvcInvocation({ dvcPath: '', workspaceRoot: './' } as Config) - ).toEqual(`${testPythonBin} -m dvc`) + expect(await getDvcInvocation({ cliPath: '', cwd: './' })).toEqual( + `${testPythonBin} -m dvc` + ) }) - it('should ignore a path from the Python extension when dvcPath is defined', async () => { + it('should ignore a path from the Python extension when cliPath is defined', async () => { const testPythonBin = '/custom/path/to/python' mockedGetPythonExecutionDetails.mockResolvedValue(['/wrong/python/bin']) expect( - await getDvcInvocation({ - dvcPath: testPythonBin, - workspaceRoot: './' - } as Config) + await getDvcInvocation({ cliPath: testPythonBin, cwd: './' }) ).toEqual(testPythonBin) }) it('should return a simple dvc call when no Python extension is present', async () => { mockedGetPythonExecutionDetails.mockResolvedValue(undefined) - expect( - await getDvcInvocation({ dvcPath: '', workspaceRoot: './' } as Config) - ).toEqual('dvc') + expect(await getDvcInvocation({ cliPath: '', cwd: './' })).toEqual('dvc') }) }) describe('getExperiments', () => { it('should match a snapshot when parsed', async () => { - const workspaceRoot = resolve() + const cwd = resolve() mockedExecPromise.mockResolvedValueOnce({ stdout: JSON.stringify(complexExperimentsOutput), stderr: '' }) const experiments = await getExperiments({ - dvcPath: 'dvc', - workspaceRoot - } as Config) + cliPath: 'dvc', + cwd + }) expect(experiments).toMatchSnapshot() expect(mockedExecPromise).toBeCalledWith('dvc exp show --show-json', { - cwd: workspaceRoot + cwd }) }) }) @@ -100,9 +94,9 @@ describe('initializeDirectory', () => { }) const output = await initializeDirectory({ - dvcPath: 'dvc', - workspaceRoot: fsPath - } as Config) + cliPath: 'dvc', + cwd: fsPath + }) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc init --subdir', { @@ -120,9 +114,9 @@ describe('checkout', () => { }) const output = await checkout({ - dvcPath: 'dvc', - workspaceRoot: fsPath - } as Config) + cliPath: 'dvc', + cwd: fsPath + }) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc checkout', { @@ -141,9 +135,9 @@ describe('checkoutRecursive', () => { }) const output = await checkoutRecursive({ - dvcPath: 'dvc', - workspaceRoot: fsPath - } as Config) + cliPath: 'dvc', + cwd: fsPath + }) expect(output).toEqual(stdout) expect(mockedExecPromise).toBeCalledWith('dvc checkout --recursive', { @@ -154,7 +148,6 @@ describe('checkoutRecursive', () => { describe('getRoot', () => { it('should return the root relative to the cwd', async () => { - const mockConfig = { dvcPath: 'dvc' } as Config const mockRelativeRoot = join('..', '..') const mockStdout = mockRelativeRoot + '\n\r' const cwd = resolve() @@ -162,7 +155,10 @@ describe('getRoot', () => { stdout: mockStdout, stderr: '' }) - const relativeRoot = await getRoot(mockConfig, cwd) + const relativeRoot = await getRoot({ + cwd, + cliPath: 'dvc' + }) expect(relativeRoot).toEqual(mockRelativeRoot) expect(mockedExecPromise).toBeCalledWith('dvc root', { cwd @@ -184,15 +180,15 @@ describe('getTracked', () => { `logs/acc.tsv\n` + `logs/loss.tsv\n` + `model.pt` - const workspaceRoot = resolve() + const cwd = resolve() mockedExecPromise.mockResolvedValueOnce({ stdout: stdout, stderr: '' }) const tracked = await listDvcOnlyRecursive({ - workspaceRoot, - dvcPath: 'dvc' - } as Config) + cwd, + cliPath: 'dvc' + }) expect(tracked).toEqual([ 'data/MNIST/raw/t10k-images-idx3-ubyte', @@ -209,7 +205,7 @@ describe('getTracked', () => { ]) expect(mockedExecPromise).toBeCalledWith('dvc list . --dvc-only -R', { - cwd: workspaceRoot + cwd }) }) }) diff --git a/extension/src/cli/reader.ts b/extension/src/cli/reader.ts new file mode 100644 index 0000000000..4675a65911 --- /dev/null +++ b/extension/src/cli/reader.ts @@ -0,0 +1,70 @@ +import { Commands } from './commands' +import { execPromise } from '../util' +import { ExperimentsRepoJSONOutput } from '../webviews/experiments/contract' +import { getPythonExecutionDetails } from '../extensions/python' + +interface ReaderOptions { + cliPath: string | undefined + cwd: string +} + +export const getDvcInvocation = async (options: ReaderOptions) => { + const { cliPath } = options + if (cliPath) { + return cliPath + } + const executionDetails = await getPythonExecutionDetails() + return executionDetails ? `${executionDetails.join(' ')} -m dvc` : 'dvc' +} + +export const execCommand = async ( + options: ReaderOptions, + command: string +): Promise<{ stdout: string; stderr: string }> => { + const { cwd } = options + const fullCommandString = `${await getDvcInvocation(options)} ${command}` + return execPromise(fullCommandString, { + cwd + }) +} + +export const getExperiments = async ( + options: ReaderOptions +): Promise => { + const { stdout } = await execCommand(options, Commands.experiment_show) + return JSON.parse(stdout) +} + +export const initializeDirectory = async ( + options: ReaderOptions +): Promise => { + const { stdout } = await execCommand( + options, + Commands.initialize_subdirectory + ) + return stdout +} + +export const checkout = async (options: ReaderOptions): Promise => { + const { stdout } = await execCommand(options, Commands.checkout) + return stdout +} + +export const checkoutRecursive = async ( + options: ReaderOptions +): Promise => { + const { stdout } = await execCommand(options, Commands.checkout_recursive) + return stdout +} + +export const getRoot = async (options: ReaderOptions): Promise => { + const { stdout } = await execCommand(options, 'root') + return stdout.trim() +} + +export const listDvcOnlyRecursive = async ( + options: ReaderOptions +): Promise => { + const { stdout } = await execCommand(options, `list . --dvc-only -R`) + return stdout.trim().split('\n') +} diff --git a/extension/src/extension.ts b/extension/src/extension.ts index bccb6db384..bd159eafc5 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -10,7 +10,13 @@ import { IntegratedTerminal, runExperiment } from './IntegratedTerminal' import { SourceControlManagement } from './views/SourceControlManagement' import { Config } from './Config' import { WebviewManager } from './webviews/WebviewManager' -import { getExperiments, registerDvcCommands } from './cli' +import { + getExperiments, + initializeDirectory, + checkout, + checkoutRecursive +} from './cli/reader' +import { add } from './cli' import { addFileChangeHandler, @@ -54,7 +60,10 @@ export class Extension { } private refreshExperimentsWebview = async () => { - const experiments = await getExperiments(this.config) + const experiments = await getExperiments({ + cwd: this.config.workspaceRoot, + cliPath: this.config.dvcPath + }) return this.webviewManager.refreshExperiments(experiments) } @@ -77,9 +86,11 @@ export class Extension { this.decorationProvider = this.dispose.track(new DecorationProvider()) - findDvcTrackedPaths(this.config).then(files => { - this.decorationProvider.setTrackedFiles(files) - }) + findDvcTrackedPaths(this.config.workspaceRoot, this.config.dvcPath).then( + files => { + this.decorationProvider.setTrackedFiles(files) + } + ) this.webviewManager = this.dispose.track( new WebviewManager(this.config, this.resourceLocator) @@ -107,10 +118,32 @@ export class Extension { }) ) - registerDvcCommands({ - dispose: this.dispose, - config: this.config - }) + this.dispose.track( + commands.registerCommand('dvc.initializeDirectory', ({ fsPath }) => { + initializeDirectory({ + cwd: fsPath, + cliPath: this.config.dvcPath + }) + }) + ) + + this.dispose.track( + commands.registerCommand('dvc.add', ({ resourceUri }) => + add({ fsPath: resourceUri.fsPath, cliPath: this.config.dvcPath }) + ) + ) + + this.dispose.track( + commands.registerCommand('dvc.checkout', ({ fsPath }) => { + checkout({ cwd: fsPath, cliPath: this.config.dvcPath }) + }) + ) + + this.dispose.track( + commands.registerCommand('dvc.checkoutRecursive', ({ fsPath }) => { + checkoutRecursive({ cwd: fsPath, cliPath: this.config.dvcPath }) + }) + ) this.gitExtension = this.dispose.track(new GitExtension()) @@ -122,7 +155,7 @@ export class Extension { this.dispose.track(disposable) ) - const dvcRoots = await findDvcRootPaths(this.config, gitRoot) + const dvcRoots = await findDvcRootPaths(gitRoot, this.config.dvcPath) dvcRoots.forEach(async dvcRoot => { const untracked = await getAllUntracked(dvcRoot) const scm = this.dispose.track( diff --git a/extension/src/fileSystem.test.ts b/extension/src/fileSystem.test.ts index 6220350b34..d687182dd1 100644 --- a/extension/src/fileSystem.test.ts +++ b/extension/src/fileSystem.test.ts @@ -4,8 +4,7 @@ import { mocked } from 'ts-jest/utils' import debounce from 'lodash.debounce' import { dirname, join, resolve } from 'path' import { mkdirSync, rmdir } from 'fs-extra' -import { getRoot, listDvcOnlyRecursive } from './cli' -import { Config } from './Config' +import { getRoot, listDvcOnlyRecursive } from './cli/reader' const { addFileChangeHandler, @@ -17,7 +16,7 @@ const { jest.mock('chokidar') jest.mock('lodash.debounce') -jest.mock('./cli') +jest.mock('./cli/reader') const mockedWatch = mocked(watch) const mockedDebounce = mocked(debounce) @@ -97,13 +96,11 @@ describe('getWatcher', () => { describe('findDvcRootPaths', () => { const dataRoot = resolve(dvcDemoPath, 'data') - const mockConfig = { - dvcPath: 'dvc' - } as Config + const mockCliPath = 'dvc' it('should find the dvc root if it exists in the given folder', async () => { mockGetRoot.mockResolvedValueOnce('.') - const dvcRoots = await findDvcRootPaths(mockConfig, dvcDemoPath) + const dvcRoots = await findDvcRootPaths(dvcDemoPath, mockCliPath) expect(mockGetRoot).toBeCalledTimes(1) expect(dvcRoots).toEqual([dvcDemoPath]) @@ -114,7 +111,7 @@ describe('findDvcRootPaths', () => { const mockDvcRoot = join(dataRoot, '.dvc') mkdirSync(mockDvcRoot) - const dvcRoots = await findDvcRootPaths(mockConfig, dvcDemoPath) + const dvcRoots = await findDvcRootPaths(dvcDemoPath, mockCliPath) rmdir(mockDvcRoot) @@ -127,7 +124,7 @@ describe('findDvcRootPaths', () => { const mockDvcRoot = join(dataRoot, 'MNIST', '.dvc') mkdirSync(mockDvcRoot) - const dvcRoots = await findDvcRootPaths(mockConfig, dataRoot) + const dvcRoots = await findDvcRootPaths(dataRoot, mockCliPath) rmdir(mockDvcRoot) @@ -137,13 +134,13 @@ describe('findDvcRootPaths', () => { it('should find the dvc root if it exists above the given folder', async () => { mockGetRoot.mockResolvedValueOnce('..') - const dvcRoots = await findDvcRootPaths(mockConfig, dataRoot) + const dvcRoots = await findDvcRootPaths(dataRoot, mockCliPath) expect(mockGetRoot).toBeCalledTimes(1) expect(dvcRoots).toEqual([dvcDemoPath]) }) it('should return an empty array given no dvc root in or above the given directory', async () => { - const dvcRoots = await findDvcRootPaths(mockConfig, __dirname) + const dvcRoots = await findDvcRootPaths(__dirname, mockCliPath) expect(dvcRoots).toEqual([]) }) }) @@ -159,14 +156,9 @@ describe('getAbsoluteTrackedPath', () => { }) describe('findDvcTrackedPaths', () => { - const mockConfig = { - dvcPath: 'dvc', - workspaceRoot: dvcDemoPath - } as Config - it('should find the paths in the workspace corresponding to .dvc files and return them in a Set', async () => { mockListDvcOnlyRecursive.mockResolvedValueOnce([]) - const tracked = await findDvcTrackedPaths(mockConfig) + const tracked = await findDvcTrackedPaths(dvcDemoPath, 'dvc') expect(tracked).toEqual( new Set([resolve(dvcDemoPath, 'data', 'MNIST', 'raw')]) @@ -179,7 +171,7 @@ describe('findDvcTrackedPaths', () => { const logLoss = join(logFolder, 'loss.tsv') const model = 'model.pt' mockListDvcOnlyRecursive.mockResolvedValueOnce([logAcc, logLoss, model]) - const tracked = await findDvcTrackedPaths(mockConfig) + const tracked = await findDvcTrackedPaths(dvcDemoPath, 'dvc') expect(tracked).toEqual( new Set([ diff --git a/extension/src/fileSystem.ts b/extension/src/fileSystem.ts index 050a14984e..7bdaca1e44 100644 --- a/extension/src/fileSystem.ts +++ b/extension/src/fileSystem.ts @@ -3,8 +3,7 @@ import chokidar from 'chokidar' import debounce from 'lodash.debounce' import { dirname, join, resolve, basename } from 'path' import glob from 'tiny-glob' -import { getRoot, listDvcOnlyRecursive } from './cli' -import { Config } from './Config' +import { getRoot, listDvcOnlyRecursive } from './cli/reader' export const getWatcher = (handler: () => void) => (path: string): void => { if (path) { @@ -41,11 +40,14 @@ const filterRootDir = (dirs: string[], rootDir: string) => dirs.filter(dir => dir !== rootDir) const findDvcAbsoluteRootPath = async ( - config: Config, - cwd: string + cwd: string, + cliPath: string | undefined ): Promise => { try { - const root = await getRoot(config, cwd) + const root = await getRoot({ + cliPath, + cwd + }) return resolve(cwd, root) } catch (e) {} } @@ -64,12 +66,12 @@ const findDvcSubRootPaths = async (cwd: string): Promise => { } export const findDvcRootPaths = async ( - config: Config, - cwd: string + cwd: string, + cliPath: string | undefined ): Promise => { const [subRoots, absoluteRoot] = await Promise.all([ findDvcSubRootPaths(cwd), - findDvcAbsoluteRootPath(config, cwd) + findDvcAbsoluteRootPath(cwd, cliPath) ]) const roots = [...subRoots, absoluteRoot].filter(v => v).sort() as string[] @@ -91,9 +93,9 @@ const getAbsoluteParentPath = (rootDir: string, files: string[]): string[] => { } export const findDvcTrackedPaths = async ( - config: Config + cwd: string, + cliPath: string | undefined ): Promise> => { - const cwd = config.workspaceRoot const [dotDvcFiles, dvcListFiles] = await Promise.all([ glob(join('**', '*.dvc'), { absolute: true, @@ -102,7 +104,7 @@ export const findDvcTrackedPaths = async ( filesOnly: true }), - listDvcOnlyRecursive(config) + listDvcOnlyRecursive({ cwd, cliPath }) ]) return new Set([ diff --git a/extension/src/git.test.ts b/extension/src/git.test.ts index c81de026d9..f52139f8e1 100644 --- a/extension/src/git.test.ts +++ b/extension/src/git.test.ts @@ -1,6 +1,11 @@ import { getAllUntracked } from './git' import { ensureFile, remove } from 'fs-extra' import { join, resolve } from 'path' +import { Uri } from 'vscode' + +function mapPaths(uris: Uri[]): string[] { + return uris.map(x => x.path) +} describe('getAllUntracked', () => { it('should return a list of all untracked paths', async () => { @@ -28,7 +33,7 @@ describe('getAllUntracked', () => { await Promise.all([remove(untrackedDir), remove(untrackedPython)]) - expect(gitUntracked.map(x => x.path)).toEqual( + expect(mapPaths(gitUntracked)).toEqual( expect.arrayContaining([ untrackedDir, untrackedPerl, @@ -37,9 +42,11 @@ describe('getAllUntracked', () => { ]) ) - expect(dvcUntracked.map(x => x.path)).toEqual( + expect(mapPaths(dvcUntracked)).toEqual( expect.arrayContaining([untrackedDir, untrackedPerl, untrackedText]) ) - expect(dvcUntracked).not.toEqual(expect.arrayContaining([untrackedPython])) + expect(mapPaths(dvcUntracked)).not.toEqual( + expect.arrayContaining([untrackedPython]) + ) }) }) diff --git a/extension/src/test/suite/extension.test.ts b/extension/src/test/suite/extension.test.ts index b001b5fae9..1fa698f39d 100644 --- a/extension/src/test/suite/extension.test.ts +++ b/extension/src/test/suite/extension.test.ts @@ -11,7 +11,7 @@ import { ConfigurationChangeEvent } from 'vscode' import { Disposable } from '../../extension' -import * as DvcReader from '../../cli' +import * as DvcReader from '../../cli/reader' import complexExperimentsOutput from '../../webviews/experiments/complex-output-example.json' import { ExperimentsWebview } from '../../webviews/experiments/ExperimentsWebview' From 9c92848320298a39cf786820c2e750ba6936ae0a Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 17:08:49 -0400 Subject: [PATCH 5/9] Revert IntegratedTerminal changes --- extension/src/IntegratedTerminal.test.ts | 2 +- extension/src/IntegratedTerminal.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/extension/src/IntegratedTerminal.test.ts b/extension/src/IntegratedTerminal.test.ts index 047e87b01b..1d750cfc98 100644 --- a/extension/src/IntegratedTerminal.test.ts +++ b/extension/src/IntegratedTerminal.test.ts @@ -9,6 +9,6 @@ describe('runExperiment', () => { const undef = await runExperiment() expect(undef).toBeUndefined() - expect(terminalSpy).toBeCalledWith('dvc exp run') + expect(terminalSpy).toBeCalledWith('dvc exp run ') }) }) diff --git a/extension/src/IntegratedTerminal.ts b/extension/src/IntegratedTerminal.ts index 8f908f71e0..116573d52a 100644 --- a/extension/src/IntegratedTerminal.ts +++ b/extension/src/IntegratedTerminal.ts @@ -1,4 +1,5 @@ import { Terminal, window, workspace } from 'vscode' +import { getRunExperimentCommand } from './cli/commands' import { getReadyPythonExtension } from './extensions/python' import { delay } from './util' @@ -64,5 +65,6 @@ export class IntegratedTerminal { } export const runExperiment = (): Promise => { - return IntegratedTerminal.run('dvc exp run') + const runExperimentCommand = getRunExperimentCommand() + return IntegratedTerminal.run(runExperimentCommand) } From b77bf9676244e4c9314d97f6afcea52cdc22ae45 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 17:24:36 -0400 Subject: [PATCH 6/9] Move mapPaths to a shared util module --- extension/src/git.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/extension/src/git.test.ts b/extension/src/git.test.ts index f52139f8e1..ab2ee3fb2f 100644 --- a/extension/src/git.test.ts +++ b/extension/src/git.test.ts @@ -1,11 +1,7 @@ import { getAllUntracked } from './git' import { ensureFile, remove } from 'fs-extra' import { join, resolve } from 'path' -import { Uri } from 'vscode' - -function mapPaths(uris: Uri[]): string[] { - return uris.map(x => x.path) -} +import { mapPaths } from './test/util' describe('getAllUntracked', () => { it('should return a list of all untracked paths', async () => { From 01428cf47b918ec74ee636479e2997ca24b97090 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 17:28:55 -0400 Subject: [PATCH 7/9] Actually add the new shared module --- extension/src/test/util.ts | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 extension/src/test/util.ts diff --git a/extension/src/test/util.ts b/extension/src/test/util.ts new file mode 100644 index 0000000000..6112e96ebb --- /dev/null +++ b/extension/src/test/util.ts @@ -0,0 +1,5 @@ +import { Uri } from 'vscode' + +export function mapPaths(uris: Uri[]): string[] { + return uris.map(x => x.path) +} From 315f67a4d89086b772ccc39ce5011afddabee544 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 18:50:39 -0400 Subject: [PATCH 8/9] Remove old dev scripts --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index c8436fe5c9..3c8fac58f7 100644 --- a/package.json +++ b/package.json @@ -22,8 +22,7 @@ "extension-dev-server": "yarn workspace dvc dev", "webview-dev-server": "yarn workspace dvc-vscode-webview dev", "dev-server": "run-p extension-dev-server webview-dev-server", - "dev-ui": "HOT_RELOAD=true USE_DEV_UI=true DVCPATH='.env/bin/dvc' code --extensionDevelopmentPath $PWD/extension demo", - "dev-ui-win": "HOT_RELOAD=true USE_DEV_UI=true DVCPATH='.env\\Scripts\\dvc.exe' code --extensionDevelopmentPath $PWD/extension demo", + "dev-ui": "HOT_RELOAD=true USE_DEV_UI=true code --extensionDevelopmentPath $PWD/extension demo", "postinstall": "husky install", "storybook": "yarn workspace dvc-vscode-webview storybook", "build-storybook": "yarn workspace dvc-vscode-webview build-storybook", From 7b6c096b3623d3c1aedf15f7e8a41c6986eb6bcd Mon Sep 17 00:00:00 2001 From: rogermparent Date: Wed, 7 Apr 2021 18:58:07 -0400 Subject: [PATCH 9/9] Rename test util module and map argument inside mapPaths --- extension/src/git.test.ts | 2 +- extension/src/{test/util.ts => util/testHelpers.ts} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename extension/src/{test/util.ts => util/testHelpers.ts} (70%) diff --git a/extension/src/git.test.ts b/extension/src/git.test.ts index ab2ee3fb2f..83958b9317 100644 --- a/extension/src/git.test.ts +++ b/extension/src/git.test.ts @@ -1,7 +1,7 @@ import { getAllUntracked } from './git' import { ensureFile, remove } from 'fs-extra' import { join, resolve } from 'path' -import { mapPaths } from './test/util' +import { mapPaths } from './util/testHelpers' describe('getAllUntracked', () => { it('should return a list of all untracked paths', async () => { diff --git a/extension/src/test/util.ts b/extension/src/util/testHelpers.ts similarity index 70% rename from extension/src/test/util.ts rename to extension/src/util/testHelpers.ts index 6112e96ebb..c41d622b4d 100644 --- a/extension/src/test/util.ts +++ b/extension/src/util/testHelpers.ts @@ -1,5 +1,5 @@ import { Uri } from 'vscode' export function mapPaths(uris: Uri[]): string[] { - return uris.map(x => x.path) + return uris.map(uri => uri.path) }