From 7f21fb616aadcdfdcf42751b4a101c71de13c014 Mon Sep 17 00:00:00 2001 From: Matt Seddon <37993418+mattseddon@users.noreply.github.com> Date: Fri, 19 May 2023 14:43:59 +1000 Subject: [PATCH 1/4] Fix extension initialization on Windows (downgrade from esm only packages) (#3937) --- extension/package.json | 5 +- extension/scripts/coverIntegrationTests.js | 12 ++--- extension/scripts/virtualenv-install.js | 13 ----- extension/src/extension.ts | 4 +- extension/src/process/execution.test.ts | 32 ++++++++++++ extension/src/process/execution.ts | 3 +- extension/src/python/index.test.ts | 1 - extension/src/python/index.ts | 2 - extension/src/test/suite/cli/child.ts | 6 +-- .../src/test/suite/process/execution.test.ts | 36 ------------- extension/src/util/esm.ts | 29 ----------- package.json | 2 +- renovate.json | 2 +- scripts/virtualenv-install.ts | 14 ++++++ yarn.lock | 50 +++++++++---------- 15 files changed, 85 insertions(+), 126 deletions(-) delete mode 100644 extension/scripts/virtualenv-install.js create mode 100644 extension/src/process/execution.test.ts delete mode 100644 extension/src/test/suite/process/execution.test.ts delete mode 100644 extension/src/util/esm.ts create mode 100644 scripts/virtualenv-install.ts diff --git a/extension/package.json b/extension/package.json index 5c30da8e11..9de8252b04 100644 --- a/extension/package.json +++ b/extension/package.json @@ -1628,7 +1628,6 @@ "test-vscode": "node ./dist/test/runTest.js", "test-e2e": "wdio run ./src/test/e2e/wdio.conf.ts", "test": "jest --collect-coverage", - "setup-venv": "node ./scripts/virtualenv-install.js", "cover-vscode-run": "node ./scripts/coverIntegrationTests.js", "vscode:prepublish": "" }, @@ -1636,7 +1635,7 @@ "@hediet/std": "0.6.0", "@vscode/extension-telemetry": "0.7.7", "appdirs": "1.1.0", - "execa": "7.1.1", + "execa": "5.1.1", "fs-extra": "11.1.1", "js-yaml": "4.1.0", "json5": "2.2.3", @@ -1646,7 +1645,7 @@ "lodash.isequal": "4.5.0", "lodash.merge": "4.6.2", "lodash.omit": "4.5.0", - "process-exists": "5.0.0", + "process-exists": "4.1.0", "tree-kill": "1.2.2", "uuid": "9.0.0", "vega-util": "1.17.2", diff --git a/extension/scripts/coverIntegrationTests.js b/extension/scripts/coverIntegrationTests.js index 7703fb5a33..d5fe8ed840 100644 --- a/extension/scripts/coverIntegrationTests.js +++ b/extension/scripts/coverIntegrationTests.js @@ -1,10 +1,6 @@ const { resolve, join } = require('path') const { writeFileSync } = require('fs-extra') - -const getExeca = async () => { - const { execa } = await import('execa') - return execa -} +const execa = require('execa') let activationEvents = [] let failed @@ -22,7 +18,7 @@ activationEvents = packageJson.activationEvents packageJson.activationEvents = ['onStartupFinished'] writeFileSync(packageJsonPath, JSON.stringify(packageJson)) -getExeca().then(async execa => { +const runCover = async () => { const tests = execa('node', [join(cwd, 'dist', 'test', 'runTest.js')], { cwd }) @@ -43,4 +39,6 @@ getExeca().then(async execa => { if (failed) { process.exit(1) } -}) +} + +runCover() diff --git a/extension/scripts/virtualenv-install.js b/extension/scripts/virtualenv-install.js deleted file mode 100644 index a3399329e0..0000000000 --- a/extension/scripts/virtualenv-install.js +++ /dev/null @@ -1,13 +0,0 @@ -const { join, resolve } = require('path') -require('../dist/vscode/mockModule') - -const importModuleAfterMockingVsCode = async () => { - const { setupTestVenv } = require('../dist/python') - return setupTestVenv -} - -importModuleAfterMockingVsCode().then(setupTestVenv => { - const cwd = resolve(__dirname, '..', '..', 'demo') - - setupTestVenv(cwd, '.env', '-r', join('.', 'requirements.txt')) -}) diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 7229f6320d..66bc19f6c8 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -50,7 +50,6 @@ import { DvcViewer } from './cli/dvc/viewer' import { registerSetupCommands } from './setup/register' import { Status } from './status' import { registerPersistenceCommands } from './persistence/register' -import { esmPackagesImported } from './util/esm' class Extension extends Disposable { protected readonly internalCommands: InternalCommands @@ -304,8 +303,7 @@ class Extension extends Disposable { let extension: undefined | Extension -export async function activate(context: ExtensionContext): Promise { - await esmPackagesImported +export function activate(context: ExtensionContext): void { extension = new Extension(context) context.subscriptions.push(extension) } diff --git a/extension/src/process/execution.test.ts b/extension/src/process/execution.test.ts new file mode 100644 index 0000000000..f8fb1e471d --- /dev/null +++ b/extension/src/process/execution.test.ts @@ -0,0 +1,32 @@ +import process from 'process' +import { executeProcess, processExists } from './execution' + +describe('executeProcess', () => { + it('should be able to run a process', async () => { + const output = await executeProcess({ + args: ['some', 'text'], + cwd: __dirname, + executable: 'echo' + }) + expect(output).toMatch(/some.*text/) + }) + + it('should return the stderr if the process throws with stderr', async () => { + await expect( + executeProcess({ + args: ['me', 'outside'], + cwd: __dirname, + executable: 'find' + }) + ).rejects.toBeTruthy() + }) +}) + +describe('processExists', () => { + it('should return true if the process exists', async () => { + expect(await processExists(process.pid)).toBe(true) + }) + it('should return false if it does not', async () => { + expect(await processExists(-123.321)).toBe(false) + }) +}) diff --git a/extension/src/process/execution.ts b/extension/src/process/execution.ts index ce6b4a7fa7..54c3d603da 100644 --- a/extension/src/process/execution.ts +++ b/extension/src/process/execution.ts @@ -2,9 +2,10 @@ import { ChildProcess } from 'child_process' import { Readable } from 'stream' import { Event, EventEmitter } from 'vscode' import { Disposable } from '@hediet/std/disposable' +import execa from 'execa' +import doesProcessExist from 'process-exists' import kill from 'tree-kill' import { getProcessPlatform } from '../env' -import { doesProcessExist, execa } from '../util/esm' interface RunningProcess extends ChildProcess { all?: Readable diff --git a/extension/src/python/index.test.ts b/extension/src/python/index.test.ts index 03e37df88b..c2563cb40a 100644 --- a/extension/src/python/index.test.ts +++ b/extension/src/python/index.test.ts @@ -5,7 +5,6 @@ import { getProcessPlatform } from '../env' jest.mock('../env') jest.mock('../process/execution') -jest.mock('../util/esm') const mockedGetProcessPlatform = jest.mocked(getProcessPlatform) const mockedCreateProcess = jest.mocked(createProcess) diff --git a/extension/src/python/index.ts b/extension/src/python/index.ts index 8e09dbfe89..86ecfa3b83 100644 --- a/extension/src/python/index.ts +++ b/extension/src/python/index.ts @@ -4,7 +4,6 @@ import { getProcessPlatform } from '../env' import { exists } from '../fileSystem' import { Logger } from '../common/logger' import { createProcess, executeProcess, Process } from '../process/execution' -import { esmPackagesImported } from '../util/esm' const sendOutput = (process: Process) => { process.all?.on('data', chunk => @@ -35,7 +34,6 @@ export const setupTestVenv = async ( envDir: string, ...installArgs: string[] ) => { - await esmPackagesImported if (!exists(join(cwd, envDir))) { const initVenv = createProcess({ args: ['-m', 'venv', envDir], diff --git a/extension/src/test/suite/cli/child.ts b/extension/src/test/suite/cli/child.ts index 6867cb91b3..757d159ae8 100644 --- a/extension/src/test/suite/cli/child.ts +++ b/extension/src/test/suite/cli/child.ts @@ -4,15 +4,13 @@ import { delay } from '../../../util/time' require('../../../vscode/mockModule') -const importModuleAfterMockingVsCode = async () => { +const importModuleAfterMockingVsCode = () => { const { Cli } = require('../../../cli') - const { esmPackagesImported } = require('../../../util/esm') - await esmPackagesImported return { Cli } } const main = async () => { - const { Cli } = await importModuleAfterMockingVsCode() + const { Cli } = importModuleAfterMockingVsCode() const cli = new Cli() diff --git a/extension/src/test/suite/process/execution.test.ts b/extension/src/test/suite/process/execution.test.ts deleted file mode 100644 index ae9a00e3e2..0000000000 --- a/extension/src/test/suite/process/execution.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import process from 'process' -import { describe, it, suite } from 'mocha' -import { expect } from 'chai' -import { executeProcess, processExists } from '../../../process/execution' - -suite('Process Manager Test Suite', () => { - describe('executeProcess', () => { - it('should be able to run a process', async () => { - const output = await executeProcess({ - args: ['some', 'text'], - cwd: __dirname, - executable: 'echo' - }) - expect(output).to.match(/some.*text/) - }) - - it('should return the stderr if the process throws with stderr', async () => { - await expect( - executeProcess({ - args: ['me', 'outside'], - cwd: __dirname, - executable: 'find' - }) - ).to.be.eventually.rejected - }) - }) - - describe('processExists', () => { - it('should return true if the process exists', async () => { - expect(await processExists(process.pid)).to.be.true - }) - it('should return false if it does not', async () => { - expect(await processExists(-123.321)).to.be.false - }) - }) -}) diff --git a/extension/src/util/esm.ts b/extension/src/util/esm.ts deleted file mode 100644 index 012ad90a5f..0000000000 --- a/extension/src/util/esm.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Deferred } from '@hediet/std/synchronization' - -const deferred = new Deferred() -export const esmPackagesImported = deferred.promise - -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -type EsmExeca = typeof import('execa').execa -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -type EsmProcessExists = typeof import('process-exists').processExists - -const shouldImportEsm = !process.env.JEST_WORKER_ID - -let execa: EsmExeca -let doesProcessExist: EsmProcessExists -const importEsmPackages = async () => { - const [{ execa: esmExeca }, { processExists: esmProcessExists }] = - await Promise.all([import('execa'), import('process-exists')]) - execa = esmExeca - doesProcessExist = esmProcessExists - deferred.resolve() -} - -if (shouldImportEsm) { - void importEsmPackages() -} - -export { execa, doesProcessExist } diff --git a/package.json b/package.json index b5badc564c..39c34a6d28 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "postinstall": "husky install && git submodule init && git submodule update && yarn svgr", "storybook": "yarn workspace dvc-vscode-webview storybook", "build-storybook": "yarn turbo run build-storybook --filter=dvc-vscode-webview", - "setup:venv": "yarn turbo run lint:build && yarn workspace dvc run setup-venv", + "setup:venv": "ts-node ./scripts/virtualenv-install.ts", "scheduled:cli:test": "ts-node ./extension/src/test/cli/index.ts", "create-svgs": "ts-node ./scripts/create-svgs.ts", "svgr": "yarn workspace dvc-vscode-webview svgr" diff --git a/renovate.json b/renovate.json index d70aa3c0f5..4e6cb10b27 100644 --- a/renovate.json +++ b/renovate.json @@ -1,5 +1,5 @@ { - "ignoreDeps": ["@types/node", "@types/vscode"], + "ignoreDeps": ["@types/node", "@types/vscode", "execa", "process-exists"], "extends": ["config:base"], "packageRules": [ { diff --git a/scripts/virtualenv-install.ts b/scripts/virtualenv-install.ts new file mode 100644 index 0000000000..e756cd683f --- /dev/null +++ b/scripts/virtualenv-install.ts @@ -0,0 +1,14 @@ +import { join, resolve } from 'path' +require('dvc/src/vscode/mockModule') + +const importModuleAfterMockingVsCode = () => { + const { setupTestVenv } = require('dvc/src/python') + + return setupTestVenv +} + +const setupTestVenv = importModuleAfterMockingVsCode() + +const cwd = resolve(__dirname, '..', 'demo') + +setupTestVenv(cwd, '.env', '-r', join('.', 'requirements.txt')) diff --git a/yarn.lock b/yarn.lock index 8014f97edf..ebee0dc4b8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10667,22 +10667,7 @@ evp_bytestokey@^1.0.0, evp_bytestokey@^1.0.3: md5.js "^1.3.4" safe-buffer "^5.1.1" -execa@7.1.1, execa@^7.1.1: - version "7.1.1" - resolved "https://registry.yarnpkg.com/execa/-/execa-7.1.1.tgz#3eb3c83d239488e7b409d48e8813b76bb55c9c43" - integrity sha512-wH0eMf/UXckdUYnO21+HDztteVv05rq2GXksxT4fCGeHkBhw1DROXh40wcjMcRqDOWE7iPJ4n3M7e2+YFP+76Q== - dependencies: - cross-spawn "^7.0.3" - get-stream "^6.0.1" - human-signals "^4.3.0" - is-stream "^3.0.0" - merge-stream "^2.0.0" - npm-run-path "^5.1.0" - onetime "^6.0.0" - signal-exit "^3.0.7" - strip-final-newline "^3.0.0" - -execa@^5.0.0, execa@^5.1.1: +execa@5.1.1, execa@^5.0.0, execa@^5.1.1: version "5.1.1" resolved "https://registry.yarnpkg.com/execa/-/execa-5.1.1.tgz#f80ad9cbf4298f7bd1d4c9555c21e93741c411dd" integrity sha512-8uSpZZocAZRBAPIEINJj3Lo9HyGitllczc27Eh5YYojjMFMn8yHMDMaUHE2Jqfq05D/wucwI4JGURyXt1vchyg== @@ -10712,6 +10697,21 @@ execa@^7.0.0: signal-exit "^3.0.7" strip-final-newline "^3.0.0" +execa@^7.1.1: + version "7.1.1" + resolved "https://registry.yarnpkg.com/execa/-/execa-7.1.1.tgz#3eb3c83d239488e7b409d48e8813b76bb55c9c43" + integrity sha512-wH0eMf/UXckdUYnO21+HDztteVv05rq2GXksxT4fCGeHkBhw1DROXh40wcjMcRqDOWE7iPJ4n3M7e2+YFP+76Q== + dependencies: + cross-spawn "^7.0.3" + get-stream "^6.0.1" + human-signals "^4.3.0" + is-stream "^3.0.0" + merge-stream "^2.0.0" + npm-run-path "^5.1.0" + onetime "^6.0.0" + signal-exit "^3.0.7" + strip-final-newline "^3.0.0" + exenv-es6@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/exenv-es6/-/exenv-es6-1.1.1.tgz#80b7a8c5af24d53331f755bac07e84abb1f6de67" @@ -16550,12 +16550,12 @@ prismjs@^1.27.0, prismjs@~1.27.0: resolved "https://registry.yarnpkg.com/prismjs/-/prismjs-1.27.0.tgz#bb6ee3138a0b438a3653dd4d6ce0cc6510a45057" integrity sha512-t13BGPUlFDR7wRB5kQDG4jjl7XeuH6jbJGt11JHPL96qwsEHNX2+68tFXqc1/k+/jALsbSWJKUOT/hcYAZ5LkA== -process-exists@5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/process-exists/-/process-exists-5.0.0.tgz#0b6dcd3d19e85e1f72c633f56d38e498196e2855" - integrity sha512-6QPRh5fyHD8MaXr4GYML8K/YY0Sq5dKHGIOrAKS3cYpHQdmygFCcijIu1dVoNKAZ0TWAMoeh8KDK9dF8auBkJA== +process-exists@4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/process-exists/-/process-exists-4.1.0.tgz#4132c516324c1da72d65896851cdbd8bbdf5b9d8" + integrity sha512-BBJoiorUKoP2AuM5q/yKwIfT1YWRHsaxjW+Ayu9erLhqKOfnXzzVVML0XTYoQZuI1YvcWKmc1dh06DEy4+KzfA== dependencies: - ps-list "^8.0.0" + ps-list "^6.3.0" process-nextick-args@~2.0.0: version "2.0.1" @@ -16652,10 +16652,10 @@ prr@~1.0.1: resolved "https://registry.yarnpkg.com/prr/-/prr-1.0.1.tgz#d3fc114ba06995a45ec6893f484ceb1d78f5f476" integrity sha1-0/wRS6BplaRexok/SEzrHXj19HY= -ps-list@^8.0.0: - version "8.1.1" - resolved "https://registry.yarnpkg.com/ps-list/-/ps-list-8.1.1.tgz#9ff1952b26a9a07fcc05270407e60544237ae581" - integrity sha512-OPS9kEJYVmiO48u/B9qneqhkMvgCxT+Tm28VCEJpheTpl8cJ0ffZRRNgS5mrQRTrX5yRTpaJ+hRDeefXYmmorQ== +ps-list@^6.3.0: + version "6.3.0" + resolved "https://registry.yarnpkg.com/ps-list/-/ps-list-6.3.0.tgz#a2b775c2db7d547a28fbaa3a05e4c281771259be" + integrity sha512-qau0czUSB0fzSlBOQt0bo+I2v6R+xiQdj78e1BR/Qjfl5OHWJ/urXi8+ilw1eHe+5hSeDI1wrwVTgDp2wst4oA== pseudomap@^1.0.2: version "1.0.2" From 2558f1fe1f76cc8ad8efb38f9edea754955855a5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 19 May 2023 05:00:43 +0000 Subject: [PATCH 2/4] Update version and CHANGELOG for release (#3938) Co-authored-by: Olivaw[bot] --- CHANGELOG.md | 11 +++++++++++ extension/package.json | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56dc5815a4..fa4b5c480d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to this project will be documented in this file. +## [0.8.17] - 2023-05-19 + +### 🐛 Bug Fixes + +- Fix extension initialization on Windows (esm imports broken) [#3937](https://github.com/iterative/vscode-dvc/pull/3937) by [@mattseddon](https://github.com/mattseddon) + +### 🔨 Maintenance + +- Fix test console errors (add tbody) [#3927](https://github.com/iterative/vscode-dvc/pull/3927) by [@mattseddon](https://github.com/mattseddon) +- Increase timeout of flaky test [#3923](https://github.com/iterative/vscode-dvc/pull/3923) by [@mattseddon](https://github.com/mattseddon) + ## [0.8.16] - 2023-05-18 ### 🚀 New Features and Enhancements diff --git a/extension/package.json b/extension/package.json index 9de8252b04..9b611999b5 100644 --- a/extension/package.json +++ b/extension/package.json @@ -9,7 +9,7 @@ "extensionDependencies": [ "vscode.git" ], - "version": "0.8.16", + "version": "0.8.17", "license": "Apache-2.0", "readme": "./README.md", "repository": { From ddce5b1544ac62e650878cb514ecfbd176068e74 Mon Sep 17 00:00:00 2001 From: Matt Seddon <37993418+mattseddon@users.noreply.github.com> Date: Fri, 19 May 2023 16:05:43 +1000 Subject: [PATCH 3/4] Add command to add remote (#3929) * add add remote command * add tests * refactor * apply review feedback --- .eslintrc.js | 4 +- demo | 2 +- extension/package.json | 10 ++ extension/src/cli/dvc/constants.ts | 5 +- extension/src/cli/dvc/executor.ts | 7 +- extension/src/commands/external.ts | 2 + extension/src/extension.ts | 2 +- extension/src/setup/commands/index.ts | 86 ++++++++++++++++ .../src/setup/{ => commands}/register.ts | 21 ++-- extension/src/setup/webview/messages.ts | 2 + extension/src/telemetry/constants.ts | 2 + extension/src/test/suite/setup/index.test.ts | 98 +++++++++++++++++++ extension/src/test/suite/setup/util.ts | 3 +- extension/src/vscode/title.ts | 5 +- extension/src/webview/contract.ts | 2 + extension/src/workspace/index.ts | 14 +-- extension/src/workspace/util.ts | 14 +++ scripts/virtualenv-install.ts | 1 + webview/src/setup/components/App.test.tsx | 28 +++++- webview/src/setup/components/messages.ts | 3 + .../src/setup/components/remote/Connect.tsx | 3 + webview/src/stories/Setup.stories.tsx | 13 +++ 22 files changed, 297 insertions(+), 30 deletions(-) create mode 100644 extension/src/setup/commands/index.ts rename extension/src/setup/{ => commands}/register.ts (82%) create mode 100644 extension/src/workspace/util.ts diff --git a/.eslintrc.js b/.eslintrc.js index 1a67e932df..ca685b45e1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -3,6 +3,7 @@ module.exports = { env: { 'jest/globals': true }, + extends: [ 'prettier-standard/prettier-file', 'plugin:@typescript-eslint/eslint-recommended', @@ -22,7 +23,8 @@ module.exports = { '**/*.js', '*.d.ts', 'tsconfig.json', - 'webpack.config.ts' + 'webpack.config.ts', + 'scripts/virtualenv-install.ts' ], overrides: [ { diff --git a/demo b/demo index a20953baeb..5f06c3734d 160000 --- a/demo +++ b/demo @@ -1 +1 @@ -Subproject commit a20953baeb985bdfa41490d220da32942345864f +Subproject commit 5f06c3734d76cb7ca894895e89d6d06dd878f8c4 diff --git a/extension/package.json b/extension/package.json index 9b611999b5..187fd9afa3 100644 --- a/extension/package.json +++ b/extension/package.json @@ -100,6 +100,12 @@ "category": "DVC", "icon": "$(add)" }, + { + "title": "Add Remote", + "command": "dvc.addRemote", + "category": "DVC", + "icon": "$(add)" + }, { "title": "Filter Experiments Table to Starred", "command": "dvc.addStarredExperimentsTableFilter", @@ -654,6 +660,10 @@ "command": "dvc.addExperimentsTableSort", "when": "dvc.commands.available && dvc.project.available" }, + { + "command": "dvc.addRemote", + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running.workspace" + }, { "command": "dvc.addStarredExperimentsTableFilter", "when": "dvc.commands.available && dvc.project.available" diff --git a/extension/src/cli/dvc/constants.ts b/extension/src/cli/dvc/constants.ts index 2759f0ff5a..f2c03f7716 100644 --- a/extension/src/cli/dvc/constants.ts +++ b/extension/src/cli/dvc/constants.ts @@ -38,6 +38,7 @@ export enum Command { } export enum SubCommand { + ADD = 'add', DIFF = 'diff', LIST = 'list', STATUS = 'status', @@ -47,13 +48,15 @@ export enum SubCommand { export enum Flag { ALL_COMMITS = '-A', FOLLOW = '-f', + DEFAULT = '-d', FORCE = '-f', GLOBAL = '--global', GRANULAR = '--granular', - LOCAL = '--local', JOBS = '-j', JSON = '--json', KILL = '--kill', + LOCAL = '--local', + PROJECT = '--project', NUM_COMMIT = '-n', OUTPUT_PATH = '-o', SUBDIRECTORY = '--subdir', diff --git a/extension/src/cli/dvc/executor.ts b/extension/src/cli/dvc/executor.ts index 8136837ab5..c0a9a2ef50 100644 --- a/extension/src/cli/dvc/executor.ts +++ b/extension/src/cli/dvc/executor.ts @@ -7,8 +7,7 @@ import { ExperimentSubCommand, Flag, GcPreserveFlag, - QueueSubCommand, - SubCommand + QueueSubCommand } from './constants' import { addStudioAccessToken } from './options' import { CliResult, CliStarted, typeCheckCommands } from '..' @@ -198,8 +197,8 @@ export class DvcExecutor extends DvcCli { return this.blockAndExecuteProcess(cwd, Command.REMOVE, ...args) } - public remote(cwd: string, arg: typeof SubCommand.LIST) { - return this.executeDvcProcess(cwd, Command.REMOTE, arg) + public remote(cwd: string, ...args: Args) { + return this.executeDvcProcess(cwd, Command.REMOTE, ...args) } private executeExperimentProcess(cwd: string, ...args: Args) { diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index 151f106098..8e9ea99f03 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -41,6 +41,8 @@ export enum RegisteredCliCommands { REMOVE_TARGET = 'dvc.removeTarget', RENAME_TARGET = 'dvc.renameTarget', + REMOTE_ADD = 'dvc.addRemote', + GIT_STAGE_ALL = 'dvc.gitStageAll', GIT_UNSTAGE_ALL = 'dvc.gitUnstageAll' } diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 66bc19f6c8..4a55541d39 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -47,7 +47,7 @@ import { Flag } from './cli/dvc/constants' import { LanguageClient } from './languageClient' import { collectRunningExperimentPids } from './experiments/processExecution/collect' import { DvcViewer } from './cli/dvc/viewer' -import { registerSetupCommands } from './setup/register' +import { registerSetupCommands } from './setup/commands/register' import { Status } from './status' import { registerPersistenceCommands } from './persistence/register' diff --git a/extension/src/setup/commands/index.ts b/extension/src/setup/commands/index.ts new file mode 100644 index 0000000000..b4245c89d3 --- /dev/null +++ b/extension/src/setup/commands/index.ts @@ -0,0 +1,86 @@ +import { Setup } from '..' +import { Flag, SubCommand } from '../../cli/dvc/constants' +import { AvailableCommands, InternalCommands } from '../../commands/internal' +import { definedAndNonEmpty } from '../../util/array' +import { getInput } from '../../vscode/inputBox' +import { quickPickYesOrNo } from '../../vscode/quickPick' +import { Title } from '../../vscode/title' +import { Toast } from '../../vscode/toast' +import { getOnlyOrPickProject } from '../../workspace/util' + +const noExistingOrUserConfirms = async ( + internalCommands: InternalCommands, + dvcRoot: string +): Promise => { + const remoteList = await internalCommands.executeCommand( + AvailableCommands.REMOTE, + dvcRoot, + SubCommand.LIST + ) + + if (!remoteList) { + return true + } + + return await quickPickYesOrNo( + 'make this new remote the default', + 'keep the current default', + { + placeHolder: 'Would you like to set this new remote as the default?', + title: Title.SET_REMOTE_AS_DEFAULT + } + ) +} + +const addRemoteToProject = async ( + internalCommands: InternalCommands, + dvcRoot: string +): Promise => { + const name = await getInput(Title.ENTER_REMOTE_NAME) + if (!name) { + return + } + + const url = await getInput(Title.ENTER_REMOTE_URL) + if (!url) { + return + } + + const args = [Flag.PROJECT, name, url] + + const shouldSetAsDefault = await noExistingOrUserConfirms( + internalCommands, + dvcRoot + ) + if (shouldSetAsDefault === undefined) { + return + } + + if (shouldSetAsDefault) { + args.unshift(Flag.DEFAULT) + } + + return await Toast.showOutput( + internalCommands.executeCommand( + AvailableCommands.REMOTE, + dvcRoot, + SubCommand.ADD, + ...args + ) + ) +} + +export const getAddRemoteCommand = + (setup: Setup, internalCommands: InternalCommands) => + async (): Promise => { + const dvcRoots = setup.getRoots() + if (!definedAndNonEmpty(dvcRoots)) { + return Toast.showError('Cannot add a remote without a DVC project') + } + const dvcRoot = await getOnlyOrPickProject(dvcRoots) + + if (!dvcRoot) { + return + } + return addRemoteToProject(internalCommands, dvcRoot) + } diff --git a/extension/src/setup/register.ts b/extension/src/setup/commands/register.ts similarity index 82% rename from extension/src/setup/register.ts rename to extension/src/setup/commands/register.ts index f0e8468abc..9efd2c852e 100644 --- a/extension/src/setup/register.ts +++ b/extension/src/setup/commands/register.ts @@ -1,10 +1,14 @@ import { commands } from 'vscode' -import { Setup } from '.' -import { run } from './runner' -import { SetupSection } from './webview/contract' -import { AvailableCommands, InternalCommands } from '../commands/internal' -import { RegisteredCliCommands, RegisteredCommands } from '../commands/external' -import { getFirstWorkspaceFolder } from '../vscode/workspaceFolders' +import { getAddRemoteCommand } from '.' +import { Setup } from '..' +import { run } from '../runner' +import { SetupSection } from '../webview/contract' +import { AvailableCommands, InternalCommands } from '../../commands/internal' +import { + RegisteredCliCommands, + RegisteredCommands +} from '../../commands/external' +import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders' const registerSetupConfigCommands = ( setup: Setup, @@ -100,6 +104,11 @@ export const registerSetupCommands = ( } ) + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.REMOTE_ADD, + getAddRemoteCommand(setup, internalCommands) + ) + registerSetupConfigCommands(setup, internalCommands) registerSetupShowCommands(setup, internalCommands) registerSetupStudioCommands(setup, internalCommands) diff --git a/extension/src/setup/webview/messages.ts b/extension/src/setup/webview/messages.ts index 2c6f767478..0a91f48b92 100644 --- a/extension/src/setup/webview/messages.ts +++ b/extension/src/setup/webview/messages.ts @@ -104,6 +104,8 @@ export class WebviewMessages { ) case MessageFromWebviewType.OPEN_EXPERIMENTS_WEBVIEW: return commands.executeCommand(RegisteredCommands.EXPERIMENT_SHOW) + case MessageFromWebviewType.REMOTE_ADD: + return commands.executeCommand(RegisteredCliCommands.REMOTE_ADD) default: Logger.error(`Unexpected message: ${JSON.stringify(message)}`) diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index da8ef8a87b..7fc37368c9 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -187,6 +187,8 @@ export interface IEventNamePropertyMapping { [EventName.REMOVE_TARGET]: undefined [EventName.RENAME_TARGET]: undefined + [EventName.REMOTE_ADD]: undefined + [EventName.GIT_STAGE_ALL]: undefined [EventName.GIT_UNSTAGE_ALL]: undefined diff --git a/extension/src/test/suite/setup/index.test.ts b/extension/src/test/suite/setup/index.test.ts index 9ef5143756..e823b30192 100644 --- a/extension/src/test/suite/setup/index.test.ts +++ b/extension/src/test/suite/setup/index.test.ts @@ -69,6 +69,7 @@ suite('Setup Test Suite', () => { ]) }) + // eslint-disable-next-line sonarjs/cognitive-complexity describe('Setup', () => { it('should handle an initialize git message from the webview', async () => { const { messageSpy, setup, mockInitializeGit } = buildSetup(disposable) @@ -843,6 +844,103 @@ suite('Setup Test Suite', () => { expect(mockShowWebview).to.be.calledOnce }).timeout(WEBVIEW_TEST_TIMEOUT) + it('should handle a message to add a remote', async () => { + const { messageSpy, setup, mockExecuteCommand } = buildSetup(disposable) + + const webview = await setup.showWebview() + await webview.isReady() + mockExecuteCommand.restore() + + const mockMessageReceived = getMessageReceivedEmitter(webview) + + const mockRemote = stub(DvcExecutor.prototype, 'remote') + + const remoteAdded = new Promise(resolve => + mockRemote.callsFake((cwd, ...args) => { + if (args.includes('add')) { + resolve(undefined) + } + return Promise.resolve('') + }) + ) + + const mockShowInputBox = stub(window, 'showInputBox') + .onFirstCall() + .resolves('storage') + .onSecondCall() + .resolves('s3://my-bucket') + + messageSpy.resetHistory() + mockMessageReceived.fire({ + type: MessageFromWebviewType.REMOTE_ADD + }) + + await remoteAdded + + expect(mockShowInputBox).to.be.calledTwice + expect( + mockRemote, + 'new remote is set as the default' + ).to.be.calledWithExactly( + dvcDemoPath, + 'add', + '-d', + '--project', + 'storage', + 's3://my-bucket' + ) + }).timeout(WEBVIEW_TEST_TIMEOUT) + + it('should be able to add a remote', async () => { + const mockRemote = stub(DvcExecutor.prototype, 'remote') + + const remoteAdded = new Promise(resolve => + mockRemote.callsFake((cwd, ...args) => { + if (args.includes('list')) { + return Promise.resolve('storage s3://my-bucket') + } + + if (args.includes('add')) { + resolve(undefined) + } + return Promise.resolve('') + }) + ) + + const mockShowInputBox = stub(window, 'showInputBox') + .onFirstCall() + .resolves('backup') + .onSecondCall() + .resolves('s3://my-backup-bucket') + + const mockShowQuickPick = ( + stub(window, 'showQuickPick') as SinonStub< + [items: readonly QuickPickItem[], options: QuickPickOptionsWithTitle], + Thenable | undefined> + > + ).resolves({ + label: 'No', + value: false + }) + + await commands.executeCommand(RegisteredCliCommands.REMOTE_ADD) + + await remoteAdded + + expect(mockShowInputBox).to.be.calledTwice + expect(mockShowQuickPick).to.be.calledOnce + expect( + mockRemote, + 'should not set a remote as the default unless the user explicitly chooses to' + ).to.be.calledWithExactly( + dvcDemoPath, + 'add', + '--project', + 'backup', + 's3://my-backup-bucket' + ) + }).timeout(WEBVIEW_TEST_TIMEOUT) + it('should send the appropriate messages to the webview to focus different sections', async () => { const { setup, messageSpy } = buildSetup(disposable) messageSpy.restore() diff --git a/extension/src/test/suite/setup/util.ts b/extension/src/test/suite/setup/util.ts index fd0d8a2405..f113dfde7c 100644 --- a/extension/src/test/suite/setup/util.ts +++ b/extension/src/test/suite/setup/util.ts @@ -44,7 +44,7 @@ export const buildSetup = ( const mockEmitter = disposer.track(new EventEmitter()) stub(dvcReader, 'root').resolves(mockDvcRoot) - stub(dvcExecutor, 'remote').resolves('') + const mockRemote = stub(dvcExecutor, 'remote').resolves('') const mockVersion = stub(dvcReader, 'version').resolves(MIN_CLI_VERSION) const mockGlobalVersion = stub(dvcReader, 'globalVersion').resolves( MIN_CLI_VERSION @@ -112,6 +112,7 @@ export const buildSetup = ( mockGlobalVersion, mockInitializeGit, mockOpenExternal, + mockRemote, mockRunSetup, mockShowWebview, mockVersion, diff --git a/extension/src/vscode/title.ts b/extension/src/vscode/title.ts index f5eb35800a..694ceea7ab 100644 --- a/extension/src/vscode/title.ts +++ b/extension/src/vscode/title.ts @@ -6,6 +6,8 @@ export enum Title { ENTER_EXPERIMENT_WORKER_COUNT = 'Enter the Number of Queue Workers', ENTER_FILTER_VALUE = 'Enter a Filter Value', ENTER_RELATIVE_DESTINATION = 'Enter a Destination Relative to the Root', + ENTER_REMOTE_NAME = 'Enter a Name for the Remote', + ENTER_REMOTE_URL = 'Enter the URL for the Remote', ENTER_PATH_OR_CHOOSE_FILE = 'Enter the path to your training script or select it', ENTER_STUDIO_USERNAME = 'Enter your Studio username', ENTER_STUDIO_TOKEN = 'Enter your Studio access token', @@ -36,7 +38,8 @@ export enum Title { SELECT_SORTS_TO_REMOVE = 'Select Sort(s) to Remove', SELECT_TRAINING_SCRIPT = 'Select your training script', SETUP_WORKSPACE = 'Setup the Workspace', - SET_EXPERIMENTS_HEADER_HEIGHT = 'Set Maximum Experiment Table Header Height' + SET_EXPERIMENTS_HEADER_HEIGHT = 'Set Maximum Experiment Table Header Height', + SET_REMOTE_AS_DEFAULT = 'Set Default Remote' } export const getEnterValueTitle = (path: string): Title => diff --git a/extension/src/webview/contract.ts b/extension/src/webview/contract.ts index fbf109194b..1f98ea74cf 100644 --- a/extension/src/webview/contract.ts +++ b/extension/src/webview/contract.ts @@ -51,6 +51,7 @@ export enum MessageFromWebviewType { SET_EXPERIMENTS_AND_OPEN_PLOTS = 'set-experiments-and-open-plots', SET_STUDIO_SHARE_EXPERIMENTS_LIVE = 'set-studio-share-experiments-live', TOGGLE_PLOTS_SECTION = 'toggle-plots-section', + REMOTE_ADD = 'remote-add', REMOVE_CUSTOM_PLOTS = 'remove-custom-plots', REMOVE_STUDIO_TOKEN = 'remove-studio-token', MODIFY_EXPERIMENT_PARAMS_AND_QUEUE = 'modify-experiment-params-and-queue', @@ -160,6 +161,7 @@ export type MessageFromWebview = type: MessageFromWebviewType.REMOVE_COLUMN_SORT payload: string } + | { type: MessageFromWebviewType.REMOTE_ADD } | { type: MessageFromWebviewType.REMOVE_CUSTOM_PLOTS } diff --git a/extension/src/workspace/index.ts b/extension/src/workspace/index.ts index f04f607355..207cc6e827 100644 --- a/extension/src/workspace/index.ts +++ b/extension/src/workspace/index.ts @@ -1,6 +1,6 @@ +import { getOnlyOrPickProject } from './util' import { InternalCommands } from '../commands/internal' import { Disposables, reset } from '../util/disposable' -import { quickPickOne } from '../vscode/quickPick' import { DeferredDisposable } from '../class/deferred' export abstract class BaseWorkspace< @@ -39,17 +39,9 @@ export abstract class BaseWorkspace< this.resetDeferred() } - public async getOnlyOrPickProject() { + public getOnlyOrPickProject() { const dvcRoots = this.getDvcRoots() - - if (dvcRoots.length === 1) { - return dvcRoots[0] - } - - return await quickPickOne( - dvcRoots, - 'Select which project to run command against' - ) + return getOnlyOrPickProject(dvcRoots) } public getRepository(dvcRoot: string) { diff --git a/extension/src/workspace/util.ts b/extension/src/workspace/util.ts new file mode 100644 index 0000000000..1ca5036298 --- /dev/null +++ b/extension/src/workspace/util.ts @@ -0,0 +1,14 @@ +import { quickPickOne } from '../vscode/quickPick' + +export const getOnlyOrPickProject = async ( + dvcRoots: string[] +): Promise => { + if (dvcRoots.length === 1) { + return dvcRoots[0] + } + + return await quickPickOne( + dvcRoots, + 'Select which project to run command against' + ) +} diff --git a/scripts/virtualenv-install.ts b/scripts/virtualenv-install.ts index e756cd683f..f2b64af35a 100644 --- a/scripts/virtualenv-install.ts +++ b/scripts/virtualenv-install.ts @@ -1,3 +1,4 @@ +/* eslint-disable */ import { join, resolve } from 'path' require('dvc/src/vscode/mockModule') diff --git a/webview/src/setup/components/App.test.tsx b/webview/src/setup/components/App.test.tsx index 672b161cc4..03beb10a26 100644 --- a/webview/src/setup/components/App.test.tsx +++ b/webview/src/setup/components/App.test.tsx @@ -792,10 +792,32 @@ describe('App', () => { } }) - const setupDVCButton = screen.getByText('Connect to Remote Storage') + const title = screen.getByText('Connect to Remote Storage') - expect(setupDVCButton).toBeInTheDocument() - expect(setupDVCButton).toBeVisible() + expect(title).toBeInTheDocument() + expect(title).toBeVisible() + }) + + it('should allow the user to connect a remote if they do not already have one', () => { + renderApp({ + remoteList: { demo: undefined, 'example-get-started': undefined }, + sectionCollapsed: { + [SetupSection.DVC]: true, + [SetupSection.EXPERIMENTS]: true, + [SetupSection.REMOTES]: false, + [SetupSection.STUDIO]: true + } + }) + mockPostMessage.mockReset() + const startButton = screen.getByText('Add Remote') + + expect(startButton).toBeInTheDocument() + expect(startButton).toBeVisible() + fireEvent.click(startButton) + expect(mockPostMessage).toHaveBeenCalledTimes(1) + expect(mockPostMessage).toHaveBeenCalledWith({ + type: MessageFromWebviewType.REMOTE_ADD + }) }) it('should show the list of remotes if there is only one project in the workspace', () => { diff --git a/webview/src/setup/components/messages.ts b/webview/src/setup/components/messages.ts index 6f58caf0cc..698ac972f1 100644 --- a/webview/src/setup/components/messages.ts +++ b/webview/src/setup/components/messages.ts @@ -52,3 +52,6 @@ export const saveStudioToken = () => export const removeStudioToken = () => sendMessage({ type: MessageFromWebviewType.REMOVE_STUDIO_TOKEN }) + +export const addRemote = () => + sendMessage({ type: MessageFromWebviewType.REMOTE_ADD }) diff --git a/webview/src/setup/components/remote/Connect.tsx b/webview/src/setup/components/remote/Connect.tsx index eea4acf0a1..9f5b68ca5c 100644 --- a/webview/src/setup/components/remote/Connect.tsx +++ b/webview/src/setup/components/remote/Connect.tsx @@ -1,5 +1,7 @@ import React from 'react' import { EmptyState } from '../../../shared/components/emptyState/EmptyState' +import { StartButton } from '../../../shared/components/button/StartButton' +import { addRemote } from '../messages' export const Connect: React.FC = () => ( @@ -18,5 +20,6 @@ export const Connect: React.FC = () => ( {' '} for details on how to connect to a remote

+ addRemote()} text="Add Remote" /> ) diff --git a/webview/src/stories/Setup.stories.tsx b/webview/src/stories/Setup.stories.tsx index 0e15735914..5be17782fc 100644 --- a/webview/src/stories/Setup.stories.tsx +++ b/webview/src/stories/Setup.stories.tsx @@ -154,6 +154,19 @@ CliAboveLatestTested.args = getUpdatedArgs({ isAboveLatestTestedVersion: true }) +export const NoRemoteSetup = Template.bind({}) +NoRemoteSetup.args = getUpdatedArgs({ + remoteList: { + '/Users/thatguy/projects/vscode-dvc/rootB': undefined + }, + sectionCollapsed: { + [SetupSection.DVC]: true, + [SetupSection.EXPERIMENTS]: true, + [SetupSection.REMOTES]: false, + [SetupSection.STUDIO]: true + } +}) + export const ProjectRemoteSetup = Template.bind({}) ProjectRemoteSetup.args = getUpdatedArgs({ remoteList: { From 53b234640dc74c2e3728e1922f366ab4a58c4551 Mon Sep 17 00:00:00 2001 From: Matt Seddon <37993418+mattseddon@users.noreply.github.com> Date: Fri, 19 May 2023 16:25:39 +1000 Subject: [PATCH 4/4] Rename remote folder to remotes (#3930) --- webview/src/setup/components/App.tsx | 2 +- webview/src/setup/components/{remote => remotes}/Connect.tsx | 0 .../setup/components/{remote => remotes}/DvcUninitialized.tsx | 0 .../components/{remote => remotes}/MultiProjectRemotes.tsx | 0 .../src/setup/components/{remote => remotes}/ProjectRemotes.tsx | 0 .../src/setup/components/{remote => remotes}/RemoteDetails.tsx | 0 webview/src/setup/components/{remote => remotes}/Remotes.tsx | 0 .../src/setup/components/{remote => remotes}/styles.module.scss | 0 8 files changed, 1 insertion(+), 1 deletion(-) rename webview/src/setup/components/{remote => remotes}/Connect.tsx (100%) rename webview/src/setup/components/{remote => remotes}/DvcUninitialized.tsx (100%) rename webview/src/setup/components/{remote => remotes}/MultiProjectRemotes.tsx (100%) rename webview/src/setup/components/{remote => remotes}/ProjectRemotes.tsx (100%) rename webview/src/setup/components/{remote => remotes}/RemoteDetails.tsx (100%) rename webview/src/setup/components/{remote => remotes}/Remotes.tsx (100%) rename webview/src/setup/components/{remote => remotes}/styles.module.scss (100%) diff --git a/webview/src/setup/components/App.tsx b/webview/src/setup/components/App.tsx index 369408b0cd..bcf4b123b0 100644 --- a/webview/src/setup/components/App.tsx +++ b/webview/src/setup/components/App.tsx @@ -9,7 +9,7 @@ import { Dvc } from './dvc/Dvc' import { Experiments } from './experiments/Experiments' import { Studio } from './studio/Studio' import { SetupContainer } from './SetupContainer' -import { Remotes } from './remote/Remotes' +import { Remotes } from './remotes/Remotes' import { useVsCodeMessaging } from '../../shared/hooks/useVsCodeMessaging' import { sendMessage } from '../../shared/vscode' import { TooltipIconType } from '../../shared/components/sectionContainer/InfoTooltip' diff --git a/webview/src/setup/components/remote/Connect.tsx b/webview/src/setup/components/remotes/Connect.tsx similarity index 100% rename from webview/src/setup/components/remote/Connect.tsx rename to webview/src/setup/components/remotes/Connect.tsx diff --git a/webview/src/setup/components/remote/DvcUninitialized.tsx b/webview/src/setup/components/remotes/DvcUninitialized.tsx similarity index 100% rename from webview/src/setup/components/remote/DvcUninitialized.tsx rename to webview/src/setup/components/remotes/DvcUninitialized.tsx diff --git a/webview/src/setup/components/remote/MultiProjectRemotes.tsx b/webview/src/setup/components/remotes/MultiProjectRemotes.tsx similarity index 100% rename from webview/src/setup/components/remote/MultiProjectRemotes.tsx rename to webview/src/setup/components/remotes/MultiProjectRemotes.tsx diff --git a/webview/src/setup/components/remote/ProjectRemotes.tsx b/webview/src/setup/components/remotes/ProjectRemotes.tsx similarity index 100% rename from webview/src/setup/components/remote/ProjectRemotes.tsx rename to webview/src/setup/components/remotes/ProjectRemotes.tsx diff --git a/webview/src/setup/components/remote/RemoteDetails.tsx b/webview/src/setup/components/remotes/RemoteDetails.tsx similarity index 100% rename from webview/src/setup/components/remote/RemoteDetails.tsx rename to webview/src/setup/components/remotes/RemoteDetails.tsx diff --git a/webview/src/setup/components/remote/Remotes.tsx b/webview/src/setup/components/remotes/Remotes.tsx similarity index 100% rename from webview/src/setup/components/remote/Remotes.tsx rename to webview/src/setup/components/remotes/Remotes.tsx diff --git a/webview/src/setup/components/remote/styles.module.scss b/webview/src/setup/components/remotes/styles.module.scss similarity index 100% rename from webview/src/setup/components/remote/styles.module.scss rename to webview/src/setup/components/remotes/styles.module.scss