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

Full parameterized tests support #649

Merged
merged 3 commits into from
Jan 27, 2021
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Please add your own contribution below inside the Master section
Bug-fixes within the same version aren't needed

## Master
* fully support parameterized tests in matching, diagnosis and debugging - @connectdotz
* optimization: remove stop/start the internal jest tests process during debug - @connectdotz
* add a new setting for "jest.jestCommandLine" that supersede "jest.pathToJest" and "jest.pathToConfig" - @connectdotz

-->
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@
"dependencies": {
"istanbul-lib-coverage": "^3.0.0",
"istanbul-lib-source-maps": "^4.0.0",
"jest-editor-support": "^28.0.0",
"jest-editor-support": "^28.1.0",
"jest-snapshot": "^25.5.0",
"vscode-codicons": "^0.0.4"
},
Expand Down
17 changes: 14 additions & 3 deletions src/DebugCodeLens/DebugCodeLens.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
import * as vscode from 'vscode';
import { TestIdentifier } from '../TestResults';

export type DebugTestIdentifier = string | TestIdentifier;
export class DebugCodeLens extends vscode.CodeLens {
readonly fileName: string;
readonly testName: string;
readonly testIds: DebugTestIdentifier[];
readonly document: vscode.TextDocument;

/**
*
* @param document
* @param range
* @param fileName
* @param testIds test name/pattern.
* Because a test block can have multiple test results, such as for paramertized tests (i.e. test.each/describe.each), there could be multiple debuggable candidates, thus it takes multiple test identifiers.
* Noite: If a test id is a string array, it should represent the hierarchiical relationship of a test structure, such as [describe-id, test-id].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos ("Noite", "hierarchiical")

*/
constructor(
document: vscode.TextDocument,
range: vscode.Range,
fileName: string,
testName: string
...testIds: DebugTestIdentifier[]
) {
super(range);
this.document = document;
this.fileName = fileName;
this.testName = testName;
this.testIds = testIds;
}
}
23 changes: 13 additions & 10 deletions src/DebugCodeLens/DebugCodeLensProvider.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as vscode from 'vscode';
import { extensionName } from '../appGlobals';
import { escapeRegExp } from '../helpers';
import { basename } from 'path';
import { DebugCodeLens } from './DebugCodeLens';
import { TestReconciliationState } from '../TestResults';
import { TestReconciliationStateType } from '../TestResults';
import { TestState, TestStateByTestReconciliationState } from './TestState';
import { GetJestExtByURI } from '../extensionManager';

Expand All @@ -18,7 +17,7 @@ export class DebugCodeLensProvider implements vscode.CodeLensProvider {
this.onDidChange = new vscode.EventEmitter();
}

get showWhenTestStateIn() {
get showWhenTestStateIn(): TestState[] {
return this._showWhenTestStateIn;
}

Expand All @@ -31,7 +30,7 @@ export class DebugCodeLensProvider implements vscode.CodeLensProvider {
return this.onDidChange.event;
}

provideCodeLenses(document: vscode.TextDocument, _: vscode.CancellationToken): vscode.CodeLens[] {
provideCodeLenses(document: vscode.TextDocument, _: vscode.CancellationToken): DebugCodeLens[] {
const result = [];
const ext = this.getJestExt(document.uri);
if (!ext || this._showWhenTestStateIn.length === 0 || document.isUntitled) {
Expand All @@ -43,20 +42,24 @@ export class DebugCodeLensProvider implements vscode.CodeLensProvider {
const fileName = basename(document.fileName);

for (const test of testResults) {
if (!this.showCodeLensAboveTest(test)) {
const results = test.multiResults ? [test, ...test.multiResults] : [test];
const allIds = results.filter((r) => this.showCodeLensAboveTest(r)).map((r) => r.identifier);

if (!allIds.length) {
continue;
}

const start = new vscode.Position(test.start.line, test.start.column);
const end = new vscode.Position(test.end.line, test.start.column + 5);
const range = new vscode.Range(start, end);
result.push(new DebugCodeLens(document, range, fileName, test.name));

result.push(new DebugCodeLens(document, range, fileName, ...allIds));
}

return result;
}

showCodeLensAboveTest(test: { status: TestReconciliationState }) {
showCodeLensAboveTest(test: { status: TestReconciliationStateType }): boolean {
const state = TestStateByTestReconciliationState[test.status];
return this._showWhenTestStateIn.includes(state);
}
Expand All @@ -67,16 +70,16 @@ export class DebugCodeLensProvider implements vscode.CodeLensProvider {
): vscode.ProviderResult<vscode.CodeLens> {
if (codeLens instanceof DebugCodeLens) {
codeLens.command = {
arguments: [codeLens.document, codeLens.fileName, escapeRegExp(codeLens.testName)],
arguments: [codeLens.document, codeLens.fileName, ...codeLens.testIds],
command: `${extensionName}.run-test`,
title: 'Debug',
title: codeLens.testIds.length > 1 ? `Debug(${codeLens.testIds.length})` : 'Debug',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a space after "Debug"?

Copy link
Member

@stephtr stephtr Jan 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "Debug n failing tests"/"Debug failing test" too long? -- Edit: In some cases that doesn't make sense. It's better to stick to the "Debug (n)" approach.

};
}

return codeLens;
}

didChange() {
didChange(): void {
this.onDidChange.fire();
}
}
1 change: 1 addition & 0 deletions src/DebugCodeLens/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { DebugCodeLensProvider } from './DebugCodeLensProvider';
export { TestState } from './TestState';
export { DebugTestIdentifier } from './DebugCodeLens';
81 changes: 53 additions & 28 deletions src/JestExt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,24 @@ import {
TestResult,
resultsWithLowerCaseWindowsDriveLetters,
SortedTestResults,
TestResultStatusInfo,
TestReconciliationStateType,
} from './TestResults';
import { cleanAnsi, getJestCommandSettings } from './helpers';
import {
cleanAnsi,
testIdString,
IdStringType,
getJestCommandSettings,
escapeRegExp,
} from './helpers';
import { CoverageMapProvider, CoverageCodeLensProvider } from './Coverage';
import {
updateDiagnostics,
updateCurrentDiagnostics,
resetDiagnostics,
failedSuiteCount,
} from './diagnostics';
import { DebugCodeLensProvider } from './DebugCodeLens';
import { DebugCodeLensProvider, DebugTestIdentifier } from './DebugCodeLens';
import { DebugConfigurationProvider } from './DebugConfigurationProvider';
import { DecorationOptions } from './types';
import { isOpenInMultipleEditors } from './editor';
Expand All @@ -35,6 +43,9 @@ interface InstanceSettings {
multirootEnv: boolean;
}

interface RunTestPickItem extends vscode.QuickPickItem {
id: DebugTestIdentifier;
}
export class JestExt {
coverageMapProvider: CoverageMapProvider;
coverageOverlay: CoverageOverlay;
Expand Down Expand Up @@ -297,19 +308,43 @@ export class JestExt {
public runTest = async (
workspaceFolder: vscode.WorkspaceFolder,
fileName: string,
identifier: string
...ids: DebugTestIdentifier[]
): Promise<void> => {
const restart = this.jestProcessManager.numberOfProcesses > 0;
this.jestProcessManager.stopAll();
const idString = (type: IdStringType, id: DebugTestIdentifier): string =>
typeof id === 'string' ? id : testIdString(type, id);
const selectTest = async (
testIdentifiers: DebugTestIdentifier[]
): Promise<DebugTestIdentifier | undefined> => {
const items: RunTestPickItem[] = testIdentifiers.map((id) => ({
label: idString('display-reverse', id),
id,
}));
const selected = await vscode.window.showQuickPick<RunTestPickItem>(items, {
placeHolder: 'select a test to debug',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about an uppercase "Select"?

});

this.debugConfigurationProvider.prepareTestRun(fileName, identifier);
return selected?.id;
};
let testId: DebugTestIdentifier | undefined;
switch (ids.length) {
case 0:
return;
case 1:
testId = ids[0];
break;
default:
testId = await selectTest(ids);
break;
}

const handle = vscode.debug.onDidTerminateDebugSession(() => {
handle.dispose();
if (restart) {
this.startProcess();
}
});
if (!testId) {
return;
}

this.debugConfigurationProvider.prepareTestRun(
fileName,
escapeRegExp(idString('full-name', testId))
);

try {
// try to run the debug configuration from launch.json
Expand Down Expand Up @@ -551,22 +586,12 @@ export class JestExt {

private generateDotsForItBlocks(
blocks: TestResult[],
state: TestReconciliationState
state: TestReconciliationStateType
): DecorationOptions[] {
const nameForState = {
[TestReconciliationState.KnownSuccess]: 'Passed',
[TestReconciliationState.KnownFail]: 'Failed',
[TestReconciliationState.KnownSkip]: 'Skipped',
[TestReconciliationState.Unknown]:
'Test has not run yet, due to Jest only running tests related to changes.',
};

return blocks.map((it) => {
return {
range: new vscode.Range(it.start.line, it.start.column, it.start.line, it.start.column + 1),
hoverMessage: nameForState[state],
identifier: it.name,
};
});
return blocks.map((it) => ({
range: new vscode.Range(it.start.line, it.start.column, it.start.line, it.start.column + 1),
hoverMessage: TestResultStatusInfo[state].desc,
identifier: it.name,
}));
}
}
14 changes: 8 additions & 6 deletions src/TestResults/TestReconciliationState.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
export type TestReconciliationState = 'Unknown' | 'KnownSuccess' | 'KnownFail' | 'KnownSkip';
export type TestReconciliationStateType = 'Unknown' | 'KnownSuccess' | 'KnownFail' | 'KnownSkip';

// tslint:disable-next-line variable-name
export const TestReconciliationState = {
Unknown: 'Unknown' as TestReconciliationState,
KnownSuccess: 'KnownSuccess' as TestReconciliationState,
KnownFail: 'KnownFail' as TestReconciliationState,
KnownSkip: 'KnownSkip' as TestReconciliationState,
export const TestReconciliationState: {
[key in TestReconciliationStateType]: TestReconciliationStateType;
} = {
Unknown: 'Unknown',
KnownSuccess: 'KnownSuccess',
KnownFail: 'KnownFail',
KnownSkip: 'KnownSkip',
};
33 changes: 26 additions & 7 deletions src/TestResults/TestResult.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TestReconciliationState } from './TestReconciliationState';
import { TestReconciliationStateType } from './TestReconciliationState';
import { JestFileResults, JestTotalResults } from 'jest-editor-support';
import { FileCoverage } from 'istanbul-lib-coverage';
import * as path from 'path';
Expand All @@ -17,21 +17,24 @@ export interface LocationRange {
end: Location;
}

export interface TestIdentifier {
title: string;
ancestorTitles: string[];
}
export interface TestResult extends LocationRange {
name: string;

names: {
src: string;
assertionTitle?: string;
assertionFullName?: string;
};
identifier: TestIdentifier;

status: TestReconciliationState;
status: TestReconciliationStateType;
shortMessage?: string;
terseMessage?: string;

/** Zero-based line number */
lineNumberOfError?: number;

// multiple results for the given range, common for parameterized (.each) tests
multiResults?: TestResult[];
}

export const withLowerCaseWindowsDriveLetter = (filePath: string): string | undefined => {
Expand Down Expand Up @@ -131,3 +134,19 @@ export const resultsWithoutAnsiEscapeSequence = (data: JestTotalResults): JestTo
})),
};
};

// export type StatusInfo<T> = {[key in TestReconciliationState]: T};
export interface StatusInfo {
precedence: number;
desc: string;
}

export const TestResultStatusInfo: { [key in TestReconciliationStateType]: StatusInfo } = {
KnownFail: { precedence: 1, desc: 'Failed' },
Unknown: {
precedence: 2,
desc: 'Test has not run yet, due to Jest only running tests related to changes.',
},
KnownSkip: { precedence: 3, desc: 'Skipped' },
KnownSuccess: { precedence: 4, desc: 'Passed' },
};
43 changes: 40 additions & 3 deletions src/TestResults/TestResultProvider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TestReconciler, JestTotalResults, TestFileAssertionStatus } from 'jest-editor-support';
import { TestReconciliationState } from './TestReconciliationState';
import { TestResult } from './TestResult';
import { TestResult, TestResultStatusInfo } from './TestResult';
import { parseTest } from '../TestParser';
import * as match from './match-by-context';

Expand All @@ -19,6 +19,12 @@ interface SortedTestResultsMap {
[filePath: string]: SortedTestResults;
}

const sortByStatus = (a: TestResult, b: TestResult): number => {
if (a.status === b.status) {
return 0;
}
return TestResultStatusInfo[a.status].precedence - TestResultStatusInfo[b.status].precedence;
};
export class TestResultProvider {
verbose: boolean;
private reconciler: TestReconciler;
Expand All @@ -36,6 +42,36 @@ export class TestResultProvider {
this.sortedResultsByFilePath = {};
}

private groupByRange(results: TestResult[]): TestResult[] {
if (!results.length) {
return results;
}
// build a range based map
const byRange: Map<string, TestResult[]> = new Map();
results.forEach((r) => {
// Q: is there a better/efficient way to index the range?
const key = `${r.start.line}-${r.start.column}-${r.end.line}-${r.end.column}`;
const list = byRange.get(key);
if (!list) {
byRange.set(key, [r]);
} else {
list.push(r);
}
});
// sort the test by status precedence
byRange.forEach((list) => list.sort(sortByStatus));

//merge multiResults under the primary (highest precedence)
const consolidated: TestResult[] = [];
byRange.forEach((list) => {
if (list.length > 1) {
list[0].multiResults = list.slice(1);
}
consolidated.push(list[0]);
});
return consolidated;
}

getResults(filePath: string): TestResult[] {
if (this.resultsByFilePath[filePath]) {
return this.resultsByFilePath[filePath];
Expand All @@ -60,8 +96,9 @@ export class TestResultProvider {
} catch (e) {
console.warn(`failed to get test result for ${filePath}:`, e);
}
this.resultsByFilePath[filePath] = matchResult;
return matchResult;
const testResults = this.groupByRange(matchResult);
this.resultsByFilePath[filePath] = testResults;
return testResults;
}

getSortedResults(filePath: string): SortedTestResults {
Expand Down
Loading