From a1f4f36d2c6c1c229365bdca5878fd29b6181507 Mon Sep 17 00:00:00 2001 From: Ross Knudsen Date: Sun, 7 Jun 2020 03:33:52 +1200 Subject: [PATCH] Renamed pathToJest to jestCommandLine in ProjectWorkspace (#45) --- CHANGELOG.md | 1 + index.d.ts | 18 +++++- package.json | 2 +- src/Process.js | 2 +- src/__tests__/process.test.js | 23 +++---- src/__tests__/project_workspace.test.js | 84 +++++++++++++++++++++++-- src/__tests__/settings.test.js | 6 +- src/project_workspace.ts | 58 ++++++++++++++--- tsconfig.json | 53 +--------------- 9 files changed, 165 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 089b246..c203889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Bug-fixes within the same version aren't needed ## Master * Replace babylon and typescript parsers with @babel/parser 7.x - @firsttris +* Renamed the property `pathToJest` to `jestCommandLine` in the ProjectWorkspace configuration object to better convey how the property is used internally. Left the original `pathToJest` with a deprecated flag. --> ### 27.2.0 diff --git a/index.d.ts b/index.d.ts index 997b556..00bb9fa 100644 --- a/index.d.ts +++ b/index.d.ts @@ -36,17 +36,29 @@ export interface JestSettings { export function getSettings(workspace: ProjectWorkspace, options?: Options): Promise; +export function createProjectWorkspace(config: ProjectWorkspaceConfig): ProjectWorkspace; + +export interface ProjectWorkspaceConfig { + jestCommandLine: string; + pathToConfig?: string; + rootPath: string; + localJestMajorVersion: number; + outputFileSuffix?: string; + collectCoverage?: boolean; + debug?: boolean; +} + export class ProjectWorkspace { constructor( rootPath: string, - pathToJest: string, + jestCommandLine: string, pathToConfig: string, - localJestMajorVersin: number, + localJestMajorVersion: number, outputFileSuffix?: string, collectCoverage?: boolean, debug?: boolean, ); - pathToJest: string; + jestCommandLine: string; pathToConfig: string; rootPath: string; localJestMajorVersion: number; diff --git a/package.json b/package.json index dd07217..8010255 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "jest-editor-support", - "version": "27.2.0", + "version": "27.3.0", "repository": { "type": "git", "url": "https://github.com/jest-community/jest-editor-support" diff --git a/src/Process.js b/src/Process.js index 61062e0..41fab52 100644 --- a/src/Process.js +++ b/src/Process.js @@ -18,7 +18,7 @@ import ProjectWorkspace from './project_workspace'; */ // eslint-disable-next-line import/prefer-default-export export const createProcess = (workspace: ProjectWorkspace, args: Array): ChildProcess => { - const runtimeExecutable = [workspace.pathToJest, ...args]; + const runtimeExecutable = [workspace.jestCommandLine, ...args]; // If a path to configuration file was defined, push it to runtimeArgs if (workspace.pathToConfig) { diff --git a/src/__tests__/process.test.js b/src/__tests__/process.test.js index b8b097b..d39736f 100644 --- a/src/__tests__/process.test.js +++ b/src/__tests__/process.test.js @@ -18,15 +18,15 @@ describe('createProcess', () => { }); it('spawns the process', () => { - const workspace: any = {pathToJest: ''}; + const workspace: any = {jestCommandLine: ''}; const args = []; createProcess(workspace, args); expect(spawn).toBeCalled(); }); - it('spawns the command from workspace.pathToJest', () => { - const workspace: any = {pathToJest: 'jest'}; + it('spawns the command from workspace.jestCommandLine', () => { + const workspace: any = {jestCommandLine: 'jest'}; const args = []; createProcess(workspace, args); @@ -34,8 +34,8 @@ describe('createProcess', () => { expect(spawn.mock.calls[0][1]).toEqual([]); }); - it('spawns a command with spaces from workspace.pathToJest', () => { - const workspace: any = {pathToJest: 'npm test --'}; + it('spawns a command with spaces from workspace.jestCommandLine', () => { + const workspace: any = {jestCommandLine: 'npm test --'}; const args = []; createProcess(workspace, args); @@ -43,7 +43,7 @@ describe('createProcess', () => { }); it('appends args', () => { - const workspace: any = {pathToJest: 'npm test --'}; + const workspace: any = {jestCommandLine: 'npm test --'}; const args = ['--option', 'value', '--another']; createProcess(workspace, args); @@ -53,7 +53,7 @@ describe('createProcess', () => { it('sets the --config arg to workspace.pathToConfig', () => { const workspace: any = { pathToConfig: 'non-standard.jest.js', - pathToJest: 'npm test --', + jestCommandLine: 'npm test --', }; const args = ['--option', 'value']; createProcess(workspace, args); @@ -64,7 +64,7 @@ describe('createProcess', () => { it('defines the "CI" environment variable', () => { const expected = Object.assign({}, process.env, {CI: 'true'}); - const workspace: any = {pathToJest: ''}; + const workspace: any = {jestCommandLine: ''}; const args = []; createProcess(workspace, args); @@ -73,7 +73,7 @@ describe('createProcess', () => { it('sets the current working directory of the child process', () => { const workspace: any = { - pathToJest: '', + jestCommandLine: '', rootPath: 'root directory', }; const args = []; @@ -84,14 +84,15 @@ describe('createProcess', () => { it('should set the "shell" property', () => { const expected = true; - const workspace: any = {pathToJest: ''}; + const workspace: any = {jestCommandLine: ''}; const args = []; createProcess(workspace, args); expect(spawn.mock.calls[0][2].shell).toBe(expected); }); + it('should set "detached" to true for non-windows system', () => { - const workspace: any = {pathToJest: ''}; + const workspace: any = {jestCommandLine: ''}; const args = []; const savedProcess = process; diff --git a/src/__tests__/project_workspace.test.js b/src/__tests__/project_workspace.test.js index 1106a9a..f0cd10d 100644 --- a/src/__tests__/project_workspace.test.js +++ b/src/__tests__/project_workspace.test.js @@ -7,13 +7,28 @@ * @flow */ -import ProjectWorkspace from '../project_workspace'; +import ProjectWorkspace, {createProjectWorkspace} from '../project_workspace'; describe('setup', () => { + beforeAll(() => { + // we mock console.warn because we emit a warning when the deprecated property is used. We will + // throw away the actual message to save cluttering the test output. + jest.spyOn(global.console, 'warn').mockImplementation(() => {}); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + afterEach(() => { + // make sure the call details are reset between each test. + jest.clearAllMocks(); + }); + it('sets itself up fom the constructor', () => { - const workspace = new ProjectWorkspace('root_path', 'path_to_jest', 'path_to_config', 1000); + const workspace = new ProjectWorkspace('root_path', 'jest_command_line', 'path_to_config', 1000); expect(workspace.rootPath).toEqual('root_path'); - expect(workspace.pathToJest).toEqual('path_to_jest'); + expect(workspace.jestCommandLine).toEqual('jest_command_line'); }); it('can safe guard outputFileSuffix', () => { const suffixStrings = [ @@ -25,8 +40,69 @@ describe('setup', () => { ['WITH?special ? character\n', 'with_special___character_'], ]; suffixStrings.forEach(pair => { - const workspace = new ProjectWorkspace('root_path', 'path_to_jest', 'path_to_config', 1000, pair[0]); + const workspace = new ProjectWorkspace('root_path', 'jest_command_line', 'path_to_config', 1000, pair[0]); expect(workspace.outputFileSuffix).toEqual(pair[1]); }); }); + + it('ensure that pathToJest is a shadow accessor for jestCommandLine', () => { + const localJestMajorVersion = 1000; + const pathToConfig = 'test'; + const jestCommandLine = 'path_to_jest'; + const rootPath = 'root_path'; + const workspace = new ProjectWorkspace(rootPath, jestCommandLine, pathToConfig, localJestMajorVersion); + + // first check that both pathToConfig and jestCommandLine are the same value. + expect(workspace.jestCommandLine).toBe(jestCommandLine); + expect(workspace.pathToJest).toBe(jestCommandLine); + + // then update the pathToJest property. + const updatedPathToJest = 'updated pathToJest'; + workspace.pathToJest = updatedPathToJest; + + // check that both pathToConfig and jestCommandLine yield the same value. + expect(workspace.jestCommandLine).toBe(updatedPathToJest); + expect(workspace.pathToJest).toBe(updatedPathToJest); + }); + + it('ProjectWorkspace factory can create a valid instance without pathToConfig', () => { + const config = { + jestCommandLine: 'jestCommandLine', + localJestMajorVersion: 1000, + rootPath: 'rootPath', + collectCoverage: false, + debug: true, + outputFileSuffix: 'suffix', + }; + + const instance = createProjectWorkspace(config); + + expect(instance.collectCoverage).toBe(config.collectCoverage); + expect(instance.debug).toBe(config.debug); + expect(instance.jestCommandLine).toBe(config.jestCommandLine); + expect(instance.localJestMajorVersion).toBe(config.localJestMajorVersion); + expect(instance.outputFileSuffix).toBe(config.outputFileSuffix); + expect(instance.rootPath).toBe(config.rootPath); + expect(instance.pathToConfig).toBe(undefined); + }); + + it('accessing pathToJest invokes console warning.', () => { + const config = { + jestCommandLine: 'jestCommandLine', + localJestMajorVersion: 1000, + rootPath: 'rootPath', + collectCoverage: false, + debug: true, + outputFileSuffix: 'suffix', + }; + + const instance = createProjectWorkspace(config); + + instance.pathToJest = 'new value'; + expect(global.console.warn).toBeCalledTimes(1); + + // eslint-disable-next-line no-unused-vars + const {pathToJest} = instance; + expect(global.console.warn).toBeCalledTimes(2); + }); }); diff --git a/src/__tests__/settings.test.js b/src/__tests__/settings.test.js index 9bf0493..70558bf 100644 --- a/src/__tests__/settings.test.js +++ b/src/__tests__/settings.test.js @@ -128,9 +128,9 @@ describe('getSettings', () => { it('passes command and args to createProcess', () => { const localJestMajorVersion = 1000; const pathToConfig = 'test'; - const pathToJest = 'path_to_jest'; + const jestCommandLine = 'path_to_jest'; const rootPath = 'root_path'; - const workspace = new ProjectWorkspace(rootPath, pathToJest, pathToConfig, localJestMajorVersion); + const workspace = new ProjectWorkspace(rootPath, jestCommandLine, pathToConfig, localJestMajorVersion); const {createProcess} = prepareProcess(); getSettings(workspace, { @@ -141,7 +141,7 @@ describe('getSettings', () => { { localJestMajorVersion, pathToConfig, - pathToJest, + jestCommandLine, rootPath, }, ['--showConfig'] diff --git a/src/project_workspace.ts b/src/project_workspace.ts index dd56c9b..038091e 100644 --- a/src/project_workspace.ts +++ b/src/project_workspace.ts @@ -6,6 +6,16 @@ * */ +export interface ProjectWorkspaceConfig { + jestCommandLine: string; + pathToConfig?: string; + rootPath: string; + localJestMajorVersion: number; + outputFileSuffix?: string; + collectCoverage?: boolean; + debug?: boolean; +} + /** * Represents the project that the extension is running on and it's state */ @@ -18,17 +28,33 @@ export default class ProjectWorkspace { rootPath: string; /** - * The path to Jest, this is normally a file path like - * `node_modules/.bin/jest` but you should not make the assumption that - * it is always a direct file path, as in a create-react app it would look - * like `npm test --`. + * The command to execute Jest on the command line, this is normally a file path like + * `node_modules/.bin/jest` but you should not make the assumption that it is always a direct + * file path, as in a create-react app it would look like `npm test --`. * * This means when launching a process, you will need to split on the first * space, and then move any other args into the args of the process. * * @type {string} */ - pathToJest: string; + jestCommandLine: string; + + /** + * @deprecated please use `jestCommandLine` instead. + * + * @type {string?} + */ + get pathToJest() { + // eslint-disable-next-line no-console + console.warn('Use of ProjectWorkspace.pathToJest is deprecated. Please use jestCommandLine instead.'); + return this.jestCommandLine; + } + + set pathToJest(commandLine: string) { + // eslint-disable-next-line no-console + console.warn('Use of ProjectWorkspace.pathToJest is deprecated. Please use jestCommandLine instead.'); + this.jestCommandLine = commandLine; + } /** * Path to a local Jest config file. @@ -69,7 +95,7 @@ export default class ProjectWorkspace { constructor( rootPath: string, - pathToJest: string, + jestCommandLine: string, pathToConfig: string, localJestMajorVersion: number, outputFileSuffix?: string, @@ -77,7 +103,7 @@ export default class ProjectWorkspace { debug?: boolean ) { this.rootPath = rootPath; - this.pathToJest = pathToJest; + this.jestCommandLine = jestCommandLine; this.pathToConfig = pathToConfig; this.localJestMajorVersion = localJestMajorVersion; this.outputFileSuffix = outputFileSuffix ? outputFileSuffix.replace(/[^a-z0-9]/gi, '_').toLowerCase() : undefined; @@ -85,3 +111,21 @@ export default class ProjectWorkspace { this.debug = debug; } } + +/** + * A factory to create a ProjectWorkspace instance from a ProjectWorkspaceConfig object. + */ +export const createProjectWorkspace = (config: ProjectWorkspaceConfig): ProjectWorkspace => { + // Note for pathToConfig we are forcing the TS compiler to accept undefined for ProjectWorkspace.pathToConfig. + // This property should be allowed to be optional, since Jest will work fine if no config file is provided. It + // will just use defaults. + return new ProjectWorkspace( + config.rootPath, + config.jestCommandLine, + (config.pathToConfig as unknown) as string, + config.localJestMajorVersion, + config.outputFileSuffix, + config.collectCoverage, + config.debug + ); +}; diff --git a/tsconfig.json b/tsconfig.json index 554c74e..85a6676 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,64 +1,13 @@ { "compilerOptions": { - /* Basic Options */ "target": "es6", /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019' or 'ESNEXT'. */ "module": "commonjs", /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */ - "lib": ["ES2015"], /* Specify library files to be included in the compilation. */ "allowJs": false, /* Allow javascript files to be compiled. */ - // "checkJs": true, /* Report errors in .js files. */ - // "jsx": "preserve", /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */ "declaration": true, /* Generates corresponding '.d.ts' file. */ - // "declarationMap": true, /* Generates a sourcemap for each corresponding '.d.ts' file. */ - // "sourceMap": true, /* Generates corresponding '.map' file. */ - // "outFile": "./", /* Concatenate and emit output to single file. */ "outDir": "./build", /* Redirect output structure to the directory. */ - // "rootDir": "./", /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */ - // "composite": true, /* Enable project compilation */ - // "incremental": true, /* Enable incremental compilation */ - // "tsBuildInfoFile": "./", /* Specify file to store incremental compilation information */ - // "removeComments": true, /* Do not emit comments to output. */ - // "noEmit": true, /* Do not emit outputs. */ - // "importHelpers": true, /* Import emit helpers from 'tslib'. */ - // "downlevelIteration": true, /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */ - // "isolatedModules": true, /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */ - - /* Strict Type-Checking Options */ "strict": false, /* Enable all strict type-checking options. */ - "noImplicitAny": false, /* Raise error on expressions and declarations with an implied 'any' type. */ - // "strictNullChecks": true, /* Enable strict null checks. */ - // "strictFunctionTypes": true, /* Enable strict checking of function types. */ - // "strictBindCallApply": true, /* Enable strict 'bind', 'call', and 'apply' methods on functions. */ - // "strictPropertyInitialization": true, /* Enable strict checking of property initialization in classes. */ - // "noImplicitThis": true, /* Raise error on 'this' expressions with an implied 'any' type. */ - // "alwaysStrict": true, /* Parse in strict mode and emit "use strict" for each source file. */ - - /* Additional Checks */ - // "noUnusedLocals": true, /* Report errors on unused locals. */ - // "noUnusedParameters": true, /* Report errors on unused parameters. */ - // "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */ - // "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */ - - /* Module Resolution Options */ - // "moduleResolution": "node", /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */ - // "baseUrl": "./", /* Base directory to resolve non-absolute module names. */ - // "paths": {}, /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */ - // "rootDirs": [], /* List of root folders whose combined content represents the structure of the project at runtime. */ - // "typeRoots": [], /* List of folders to include type definitions from. */ - // "types": [], /* Type declaration files to be included in compilation. */ - // "allowSyntheticDefaultImports": true, /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */ + "noImplicitAny": false, /* Raise error on expressions and declarations with an implied 'any' type. */ "esModuleInterop": true /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */ - // "preserveSymlinks": true, /* Do not resolve the real path of symlinks. */ - - /* Source Map Options */ - // "sourceRoot": "", /* Specify the location where debugger should locate TypeScript files instead of source locations. */ - // "mapRoot": "", /* Specify the location where debugger should locate map files instead of generated locations. */ - // "inlineSourceMap": true, /* Emit a single file with source maps instead of having a separate file. */ - // "inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */ - - /* Experimental Options */ - // "experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */ - // "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */ }, "include": ["./src/**/*.ts"], - //"exclude": ["**/*.test.ts"] } \ No newline at end of file