Skip to content

Commit

Permalink
Renamed pathToJest to jestCommandLine in ProjectWorkspace (#45)
Browse files Browse the repository at this point in the history
  • Loading branch information
rossknudsen authored Jun 6, 2020
1 parent 5e122c5 commit a1f4f36
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 82 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 15 additions & 3 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,29 @@ export interface JestSettings {

export function getSettings(workspace: ProjectWorkspace, options?: Options): Promise<JestSettings>;

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;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/Process.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import ProjectWorkspace from './project_workspace';
*/
// eslint-disable-next-line import/prefer-default-export
export const createProcess = (workspace: ProjectWorkspace, args: Array<string>): 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) {
Expand Down
23 changes: 12 additions & 11 deletions src/__tests__/process.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,32 @@ 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);

expect(spawn.mock.calls[0][0]).toBe('jest');
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);

expect(spawn.mock.calls[0][0]).toBe('npm test --');
});

it('appends args', () => {
const workspace: any = {pathToJest: 'npm test --'};
const workspace: any = {jestCommandLine: 'npm test --'};
const args = ['--option', 'value', '--another'];
createProcess(workspace, args);

Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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 = [];
Expand All @@ -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;
Expand Down
84 changes: 80 additions & 4 deletions src/__tests__/project_workspace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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);
});
});
6 changes: 3 additions & 3 deletions src/__tests__/settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand All @@ -141,7 +141,7 @@ describe('getSettings', () => {
{
localJestMajorVersion,
pathToConfig,
pathToJest,
jestCommandLine,
rootPath,
},
['--showConfig']
Expand Down
58 changes: 51 additions & 7 deletions src/project_workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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.
Expand Down Expand Up @@ -69,19 +95,37 @@ export default class ProjectWorkspace {

constructor(
rootPath: string,
pathToJest: string,
jestCommandLine: string,
pathToConfig: string,
localJestMajorVersion: number,
outputFileSuffix?: string,
collectCoverage?: boolean,
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;
this.collectCoverage = collectCoverage;
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
);
};
Loading

0 comments on commit a1f4f36

Please sign in to comment.