Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix extension initialization on Windows (esm imports broken) #3937

Merged
merged 1 commit into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1628,15 +1628,14 @@
"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": ""
},
"dependencies": {
"@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",
Expand All @@ -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",
Expand Down
12 changes: 5 additions & 7 deletions extension/scripts/coverIntegrationTests.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
})
Expand All @@ -43,4 +39,6 @@ getExeca().then(async execa => {
if (failed) {
process.exit(1)
}
})
}

runCover()
13 changes: 0 additions & 13 deletions extension/scripts/virtualenv-install.js

This file was deleted.

4 changes: 1 addition & 3 deletions extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -304,8 +303,7 @@ class Extension extends Disposable {

let extension: undefined | Extension

export async function activate(context: ExtensionContext): Promise<void> {
await esmPackagesImported
export function activate(context: ExtensionContext): void {
extension = new Extension(context)
context.subscriptions.push(extension)
}
Expand Down
32 changes: 32 additions & 0 deletions extension/src/process/execution.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
3 changes: 2 additions & 1 deletion extension/src/process/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion extension/src/python/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions extension/src/python/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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],
Expand Down
6 changes: 2 additions & 4 deletions extension/src/test/suite/cli/child.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
36 changes: 0 additions & 36 deletions extension/src/test/suite/process/execution.test.ts

This file was deleted.

29 changes: 0 additions & 29 deletions extension/src/util/esm.ts

This file was deleted.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion renovate.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"ignoreDeps": ["@types/node", "@types/vscode"],
"ignoreDeps": ["@types/node", "@types/vscode", "execa", "process-exists"],
"extends": ["config:base"],
"packageRules": [
{
Expand Down
14 changes: 14 additions & 0 deletions scripts/virtualenv-install.ts
Original file line number Diff line number Diff line change
@@ -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'))
50 changes: 25 additions & 25 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10667,22 +10667,7 @@ evp_bytestokey@^1.0.0, evp_bytestokey@^1.0.3:
md5.js "^1.3.4"
safe-buffer "^5.1.1"

[email protected], 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:
[email protected], 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==
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down