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

Update blocked ESM only packages (execa, process-exists) #3856

Merged
merged 6 commits into from
May 11, 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: 3 additions & 2 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1628,14 +1628,15 @@
"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": "5.1.1",
"execa": "7.1.1",
"fs-extra": "11.1.1",
"js-yaml": "4.1.0",
"json5": "2.2.3",
Expand All @@ -1646,6 +1647,7 @@
"lodash.merge": "4.6.2",
"lodash.omit": "4.5.0",
"node-fetch": "2.6.9",
"process-exists": "5.0.0",
"tree-kill": "1.2.2",
"uuid": "9.0.0",
"vega-util": "1.17.2",
Expand Down Expand Up @@ -1690,7 +1692,6 @@
"lint-staged": "13.2.2",
"mocha": "10.2.0",
"mock-require": "3.0.3",
"process-exists": "4.1.0",
"shx": "0.3.4",
"sinon": "15.0.4",
"sinon-chai": "3.7.0",
Expand Down
47 changes: 25 additions & 22 deletions extension/scripts/coverIntegrationTests.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
const { resolve, join } = require('path')
const { writeFileSync } = require('fs-extra')
const execa = require('execa')

const getExeca = async () => {
const { execa } = await import('execa')
return execa
}

let activationEvents = []
let failed
Expand All @@ -18,26 +22,25 @@ activationEvents = packageJson.activationEvents
packageJson.activationEvents = ['onStartupFinished']
writeFileSync(packageJsonPath, JSON.stringify(packageJson))

const tests = execa('node', [join(cwd, 'dist', 'test', 'runTest.js')], {
cwd
})
getExeca().then(async execa => {
const tests = execa('node', [join(cwd, 'dist', 'test', 'runTest.js')], {
cwd
})

pipe(tests)
tests
.then(() => {})
.catch(() => {
pipe(tests)
try {
await tests
} catch {
failed = true
})
.finally(() => {
packageJson.activationEvents = activationEvents

writeFileSync(packageJsonPath, JSON.stringify(packageJson))

const prettier = execa('prettier', ['--write', 'package.json'], { cwd })
pipe(prettier)
prettier.then(() => {
if (failed) {
process.exit(1)
}
})
})
}
packageJson.activationEvents = activationEvents

writeFileSync(packageJsonPath, JSON.stringify(packageJson))

const prettier = execa('prettier', ['--write', 'package.json'], { cwd })
pipe(prettier)
await prettier
if (failed) {
process.exit(1)
}
})
13 changes: 13 additions & 0 deletions extension/scripts/virtualenv-install.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
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'))
})
4 changes: 3 additions & 1 deletion extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ 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 @@ -303,7 +304,8 @@ class Extension extends Disposable {

let extension: undefined | Extension

export function activate(context: ExtensionContext): void {
export async function activate(context: ExtensionContext): Promise<void> {
await esmPackagesImported
extension = new Extension(context)
context.subscriptions.push(extension)
}
Expand Down
32 changes: 0 additions & 32 deletions extension/src/process/execution.test.ts

This file was deleted.

5 changes: 2 additions & 3 deletions extension/src/process/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ 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 doesProcessExists 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 Expand Up @@ -86,7 +85,7 @@ export const executeProcess = async (
}

export const processExists = (pid: number): Promise<boolean> =>
doesProcessExists(pid)
doesProcessExist(pid)

export const stopProcesses = async (pids: number[]): Promise<boolean> => {
let allKilled = true
Expand Down
9 changes: 5 additions & 4 deletions extension/src/python/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { join } from 'path'
import { setupVenv } from '.'
import { setupTestVenv } from '.'
import { Process, createProcess } from '../process/execution'
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 All @@ -16,15 +17,15 @@ beforeEach(() => {
jest.resetAllMocks()
})

describe('setupVenv', () => {
describe('setupTestVenv', () => {
it('should create the correct python processes on sane operating systems', async () => {
mockedCreateProcess.mockResolvedValue(mockedProcess)
mockedGetProcessPlatform.mockReturnValue('freebsd')

const envDir = '.env'
const cwd = __dirname

await setupVenv(__dirname, envDir, 'dvc')
await setupTestVenv(__dirname, envDir, 'dvc')

expect(mockedCreateProcess).toHaveBeenCalledTimes(3)
expect(mockedCreateProcess).toHaveBeenCalledWith({
Expand Down Expand Up @@ -53,7 +54,7 @@ describe('setupVenv', () => {
const envDir = '.env'
const cwd = __dirname

await setupVenv(__dirname, envDir, '-r', 'requirements.txt')
await setupTestVenv(__dirname, envDir, '-r', 'requirements.txt')

expect(mockedCreateProcess).toHaveBeenCalledTimes(3)
expect(mockedCreateProcess).toHaveBeenCalledWith({
Expand Down
4 changes: 3 additions & 1 deletion extension/src/python/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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 All @@ -29,11 +30,12 @@ export const installPackages = (
export const getDefaultPython = (): string =>
getProcessPlatform() === 'win32' ? 'python' : 'python3'

export const setupVenv = async (
export const setupTestVenv = async (
cwd: string,
envDir: string,
...installArgs: string[]
) => {
await esmPackagesImported
if (!exists(join(cwd, envDir))) {
const initVenv = createProcess({
args: ['-m', 'venv', envDir],
Expand Down
8 changes: 4 additions & 4 deletions extension/src/test/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ require('../../vscode/mockModule')
const importModulesAfterMockingVsCode = () => {
const { removeDir } = require('../../fileSystem')
const { runMocha } = require('../util/mocha')
const { setupVenv } = require('../../python')
const { setupTestVenv } = require('../../python')

return { removeDir, runMocha, setupVenv }
return { removeDir, runMocha, setupTestVenv }
}

const { setupVenv, removeDir, runMocha } = importModulesAfterMockingVsCode()
const { setupTestVenv, removeDir, runMocha } = importModulesAfterMockingVsCode()

async function main() {
try {
Expand All @@ -19,7 +19,7 @@ async function main() {
'ts',
async () => {
await mkdirp(TEMP_DIR)
await setupVenv(
await setupTestVenv(
TEMP_DIR,
ENV_DIR,
'git+https://github.com/iterative/dvc#egg=dvc[s3]'
Expand Down
2 changes: 1 addition & 1 deletion extension/src/test/e2e/util.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Key } from 'webdriverio'
import { $$, browser } from '@wdio/globals'
import { ViewControl } from 'wdio-vscode-service'
import { PlotsWebview } from './pageObjects/plotsWebview'
import { PlotsWebview } from './pageObjects/plotsWebview.js'

const findProgressBars = () => $$('.monaco-progress-container')

Expand Down
6 changes: 4 additions & 2 deletions extension/src/test/suite/cli/child.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { delay } from '../../../util/time'

require('../../../vscode/mockModule')

const importModuleAfterMockingVsCode = () => {
const importModuleAfterMockingVsCode = async () => {
const { Cli } = require('../../../cli')
const { esmPackagesImported } = require('../../../util/esm')
await esmPackagesImported
return { Cli }
}

const main = async () => {
const { Cli } = importModuleAfterMockingVsCode()
const { Cli } = await importModuleAfterMockingVsCode()

const cli = new Cli()

Expand Down
36 changes: 36 additions & 0 deletions extension/src/test/suite/process/execution.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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
})
})
})
29 changes: 29 additions & 0 deletions extension/src/util/esm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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 }
1 change: 1 addition & 0 deletions extension/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"compilerOptions": {
"resolveJsonModule": true,
"moduleResolution": "node16",
"module": "commonjs",
"target": "es6",
"outDir": "dist",
Expand Down
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",
"storybook": "yarn workspace dvc-vscode-webview storybook",
"build-storybook": "yarn turbo run build-storybook --filter=dvc-vscode-webview",
"setup:venv": "ts-node ./scripts/virtualenv-install.ts",
"setup:venv": "yarn turbo run lint:build && yarn workspace dvc run setup-venv",
"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", "execa", "process-exists"],
"ignoreDeps": ["@types/node", "@types/vscode"],
"extends": ["config:base"],
"packageRules": [
{
Expand Down
14 changes: 0 additions & 14 deletions scripts/virtualenv-install.ts

This file was deleted.

Loading