Merge branch 'main' into integrate-exp-show

iterative · Apr 17, 2023 · ead10f9 · ead10f9
2 parents 54eedd9 + 57295e2
commit ead10f9
Show file tree

Hide file tree

Showing 11 changed files with 213 additions and 58 deletions.
diff --git a/extension/package.json b/extension/package.json
@@ -1688,7 +1688,7 @@
     "fork-ts-checker-webpack-plugin": "8.0.0",
     "jest": "29.5.0",
     "jest-environment-node": "29.5.0",
-    "lint-staged": "13.2.0",
+    "lint-staged": "13.2.1",
     "mocha": "10.2.0",
     "mock-require": "3.0.3",
     "process-exists": "4.1.0",

diff --git a/extension/src/experiments/workspace.test.ts b/extension/src/experiments/workspace.test.ts
@@ -19,6 +19,7 @@ import {
   hasDvcYamlFile
 } from '../fileSystem'
 import { Toast } from '../vscode/toast'
+import { pickFile } from '../vscode/resourcePicker'
 
 const mockedShowWebview = jest.fn()
 const mockedDisposable = jest.mocked(Disposable)
@@ -35,12 +36,14 @@ const mockedListStages = jest.fn()
 const mockedFindOrCreateDvcYamlFile = jest.mocked(findOrCreateDvcYamlFile)
 const mockedGetFileExtension = jest.mocked(getFileExtension)
 const mockedHasDvcYamlFile = jest.mocked(hasDvcYamlFile)
+const mockedPickFile = jest.mocked(pickFile)
 
 jest.mock('vscode')
 jest.mock('@hediet/std/disposable')
 jest.mock('../vscode/quickPick')
 jest.mock('../vscode/inputBox')
 jest.mock('../fileSystem')
+jest.mock('../vscode/resourcePicker')
 
 beforeEach(() => {
   jest.resetAllMocks()
@@ -442,7 +445,8 @@ describe('Experiments', () => {
         mockedDvcRoot,
         trainingScript,
         'train',
-        scriptCommand.PYTHON
+        scriptCommand.PYTHON,
+        false
       )
     })
 
@@ -482,7 +486,8 @@ describe('Experiments', () => {
         mockedDvcRoot,
         'path/to/training_script.py',
         'train',
-        scriptCommand.PYTHON
+        scriptCommand.PYTHON,
+        false
       )
     })
 
@@ -501,7 +506,8 @@ describe('Experiments', () => {
         mockedDvcRoot,
         'path/to/training_script.ipynb',
         'train',
-        scriptCommand.JUPYTER
+        scriptCommand.JUPYTER,
+        false
       )
     })
 
@@ -553,7 +559,53 @@ describe('Experiments', () => {
         mockedDvcRoot,
         'path/to/training_script.js',
         'train',
-        customCommand
+        customCommand,
+        false
+      )
+    })
+
+    it('should not convert the script path to relative if the path was entered manually', async () => {
+      const customCommand = 'node'
+
+      mockedGetValidInput.mockResolvedValueOnce('train')
+      mockedListStages.mockResolvedValueOnce('')
+      mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot)
+      mockedQuickPickOneOrInput.mockResolvedValueOnce(
+        'path/to/training_script.js'
+      )
+      mockedGetFileExtension.mockReturnValueOnce('.js')
+      mockedGetInput.mockResolvedValueOnce(customCommand)
+
+      await workspaceExperiments.getCwdThenRun(mockedCommandId)
+
+      expect(mockedFindOrCreateDvcYamlFile).toHaveBeenCalledWith(
+        mockedDvcRoot,
+        'path/to/training_script.js',
+        'train',
+        customCommand,
+        false
+      )
+    })
+
+    it('should convert the script path to relative if the path was not entered manually', async () => {
+      const customCommand = 'node'
+
+      mockedGetValidInput.mockResolvedValueOnce('train')
+      mockedListStages.mockResolvedValueOnce('')
+      mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot)
+      mockedQuickPickOneOrInput.mockResolvedValueOnce('select')
+      mockedPickFile.mockResolvedValueOnce('path/to/training_script.js')
+      mockedGetFileExtension.mockReturnValueOnce('.js')
+      mockedGetInput.mockResolvedValueOnce(customCommand)
+
+      await workspaceExperiments.getCwdThenRun(mockedCommandId)
+
+      expect(mockedFindOrCreateDvcYamlFile).toHaveBeenCalledWith(
+        mockedDvcRoot,
+        'path/to/training_script.js',
+        'train',
+        customCommand,
+        true
       )
     })
 
@@ -573,7 +625,8 @@ describe('Experiments', () => {
         mockedDvcRoot,
         'path/to/training_script.js',
         'train',
-        ''
+        '',
+        false
       )
     })
 

diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts
@@ -452,11 +452,18 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
       return false
     }
 
-    const { trainingScript, command } = await this.askForTrainingScript()
+    const { trainingScript, command, enteredManually } =
+      await this.askForTrainingScript()
     if (!trainingScript) {
       return false
     }
-    void findOrCreateDvcYamlFile(cwd, trainingScript, stageName, command)
+    void findOrCreateDvcYamlFile(
+      cwd,
+      trainingScript,
+      stageName,
+      command,
+      !enteredManually
+    )
     return true
   }
 
@@ -495,14 +502,19 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
         : pathOrSelect
 
     if (!trainingScript) {
-      return { command: undefined, trainingScript: undefined }
+      return {
+        command: undefined,
+        enteredManually: false,
+        trainingScript: undefined
+      }
     }
 
     const command =
       getScriptCommand(trainingScript) ||
       (await getInput(Title.ENTER_COMMAND_TO_RUN)) ||
       ''
-    return { command, trainingScript }
+    const enteredManually = pathOrSelect !== selectValue
+    return { command, enteredManually, trainingScript }
   }
 
   private async pickExpThenRun(

diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts
@@ -187,7 +187,8 @@ describe('findOrCreateDvcYamlFile', () => {
       cwd,
       '/my/training/script.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedEnsureFileSync).toHaveBeenCalledWith(`${cwd}/dvc.yaml`)
@@ -200,7 +201,8 @@ describe('findOrCreateDvcYamlFile', () => {
       cwd,
       '/script.py',
       uniqueStageName,
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -215,7 +217,8 @@ describe('findOrCreateDvcYamlFile', () => {
       cwd,
       '/my/training/script.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -230,7 +233,8 @@ describe('findOrCreateDvcYamlFile', () => {
       cwd,
       '/my/training/script.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -247,7 +251,8 @@ describe('findOrCreateDvcYamlFile', () => {
       '/dir/my_project/',
       '/dir/my_project/src/training/train.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -259,7 +264,8 @@ describe('findOrCreateDvcYamlFile', () => {
       '/dir/my_project/',
       '/dir/my_other_project/train.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -268,12 +274,32 @@ describe('findOrCreateDvcYamlFile', () => {
     )
   })
 
+  it('should not convert the script path to a relative path if applyRelativePath is set to false', () => {
+    mockedOpenTextDocument.mockResolvedValue({} as TextDocument)
+
+    void findOrCreateDvcYamlFile(
+      '/dir/my_project/',
+      join('dir', 'my_project', 'src', 'training', 'train.py'),
+      'train',
+      scriptCommand.PYTHON,
+      false
+    )
+
+    expect(mockedAppendFileSync).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.stringContaining(
+        join('dir', 'my_project', 'src', 'training', 'train.py')
+      )
+    )
+  })
+
   it('should use the jupyter nbconvert command if the training script is a Jupyter notebook', () => {
     void findOrCreateDvcYamlFile(
       '/',
       '/train.ipynb',
       'train',
-      scriptCommand.JUPYTER
+      scriptCommand.JUPYTER,
+      true
     )
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
@@ -291,7 +317,8 @@ describe('findOrCreateDvcYamlFile', () => {
       '/',
       '/train.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedAppendFileSync).not.toHaveBeenCalledWith(
@@ -306,7 +333,7 @@ describe('findOrCreateDvcYamlFile', () => {
 
   it('should use the custom command received', () => {
     const command = 'specialCommand'
-    void findOrCreateDvcYamlFile('/', '/train.other', 'train', command)
+    void findOrCreateDvcYamlFile('/', '/train.other', 'train', command, true)
 
     expect(mockedAppendFileSync).toHaveBeenCalledWith(
       expect.anything(),
@@ -321,7 +348,8 @@ describe('findOrCreateDvcYamlFile', () => {
       '/',
       '/train.py',
       'train',
-      scriptCommand.PYTHON
+      scriptCommand.PYTHON,
+      true
     )
 
     expect(mockedOpenTextDocument).toHaveBeenCalledWith(

diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts
@@ -1,4 +1,13 @@
-import { basename, extname, join, parse, relative, resolve, sep } from 'path'
+import {
+  basename,
+  extname,
+  join,
+  parse,
+  relative,
+  resolve,
+  sep,
+  format
+} from 'path'
 import {
   appendFileSync,
   ensureFileSync,
@@ -153,21 +162,24 @@ export const findOrCreateDvcYamlFile = (
   cwd: string,
   trainingScript: string,
   stageName: string,
-  command: string
+  command: string,
+  applyRelativePath: boolean
 ) => {
   const dvcYamlPath = `${cwd}/dvc.yaml`
   ensureFileSync(dvcYamlPath)
 
-  const relativeScript = relative(cwd, trainingScript)
+  const scriptPath = applyRelativePath
+    ? relative(cwd, trainingScript)
+    : format(parse(trainingScript))
 
   const pipeline = `
 # Read about DVC pipeline configuration (https://dvc.org/doc/user-guide/project-structure/dvcyaml-files#stages)
 # to customize your stages even more
 stages:
   ${stageName}:
-    cmd: ${command} ${relativeScript}
+    cmd: ${command} ${scriptPath}
     deps:
-      - ${relativeScript}`
+      - ${scriptPath}`
 
   void openFileInEditor(dvcYamlPath)
   return appendFileSync(dvcYamlPath, pipeline)

diff --git a/languageServer/package.json b/languageServer/package.json
@@ -32,7 +32,7 @@
     "copy-webpack-plugin": "11.0.0",
     "fork-ts-checker-webpack-plugin": "8.0.0",
     "ts-loader": "9.4.2",
-    "lint-staged": "13.2.0",
+    "lint-staged": "13.2.1",
     "jest": "29.5.0",
     "webpack": "5.78.0",
     "webpack-cli": "5.0.1",

diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
     "@typescript-eslint/eslint-plugin": "5.57.1",
     "@typescript-eslint/parser": "5.57.1",
     "@vscode/codicons": "0.0.32",
-    "eslint": "8.37.0",
+    "eslint": "8.38.0",
     "eslint-config-prettier": "8.8.0",
     "eslint-config-prettier-standard": "4.0.1",
     "eslint-config-standard": "17.0.0",
@@ -58,14 +58,14 @@
     "eslint-plugin-unicorn": "46.0.0",
     "husky": "8.0.3",
     "jest": "29.5.0",
-    "lint-staged": "13.2.0",
+    "lint-staged": "13.2.1",
     "npm-run-all": "4.1.5",
     "nyc": "15.1.0",
     "prettier": "2.8.7",
     "prettier-config-standard": "5.0.0",
     "ts-node": "10.9.1",
     "turbo": "1.8.8",
-    "typescript": "5.0.3"
+    "typescript": "5.0.4"
   },
   "resolutions": {
     "decode-uri-component": "0.2.2",

diff --git a/webview/package.json b/webview/package.json
@@ -33,7 +33,7 @@
     "react-intersection-observer": "9.4.3",
     "react-redux": "8.0.5",
     "react-vega": "7.6.0",
-    "react-virtualized": "9.22.3",
+    "react-virtualized": "9.22.4",
     "vega-lite": "5.6.1",
     "vega-util": "1.17.1"
   },
@@ -68,9 +68,9 @@
     "jest": "29.5.0",
     "jest-canvas-mock": "2.5.0",
     "jest-environment-jsdom": "29.5.0",
-    "lint-staged": "13.2.0",
+    "lint-staged": "13.2.1",
     "raw-loader": "4.0.2",
-    "sass": "1.60.0",
+    "sass": "1.61.0",
     "sass-loader": "13.2.2",
     "storybook-addon-designs": "6.3.1",
     "storybook-addon-themes": "6.1.0",