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

feat: update test statuses as they are received #1075

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
8 changes: 8 additions & 0 deletions .hintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": [
"development"
],
"hints": {
"typescript-config/strict": "off"
}
}
7 changes: 3 additions & 4 deletions src/JestExt/core.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import * as vscode from 'vscode';
import { JestTotalResults } from 'jest-editor-support';

import { statusBar, StatusBar, Mode, StatusBarUpdate, SBTestStats } from '../StatusBar';
import {
TestResultProvider,
Expand Down Expand Up @@ -44,6 +42,7 @@ import { MessageAction } from '../messaging';
import { getExitErrorDef } from '../errors';
import { WorkspaceManager, isInFolder } from '../workspace-manager';
import { ansiEsc, JestOutputTerminal } from './output-terminal';
import type { AggregatedResult } from '@jest/reporters';

interface RunTestPickItem extends vscode.QuickPickItem {
id: DebugTestIdentifier;
Expand Down Expand Up @@ -864,10 +863,10 @@ export class JestExt {
this.coverageOverlay.updateVisibleEditors();
});
}
private updateWithData(data: JestTotalResults, process: JestProcessInfo): void {
private updateWithData(data: AggregatedResult, process: JestProcessInfo): void {
const noAnsiData = resultsWithoutAnsiEscapeSequence(data);
const normalizedData = resultsWithLowerCaseWindowsDriveLetters(noAnsiData);
this._updateCoverageMap(normalizedData.coverageMap);
this._updateCoverageMap(normalizedData.coverageMap?.data);

const statusList = this.testResultProvider.updateTestResults(normalizedData, process);

Expand Down
24 changes: 19 additions & 5 deletions src/JestExt/process-listeners.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import * as vscode from 'vscode';
import { JestTotalResults } from 'jest-editor-support';
import { cleanAnsi, toErrorString } from '../helpers';
import { JestProcess, JestProcessEvent } from '../JestProcessManagement';
import { ListenerSession, ListTestFilesCallback } from './process-session';
import { Logging } from '../logging';
import { JestRunEvent } from './types';
import { JestRunEvent, TestResultJestRunEventArguments } from './types';
import { MonitorLongRun } from '../Settings';
import { extensionName } from '../appGlobals';
import { RunShell } from './run-shell';
import type { AggregatedResult } from '@jest/reporters';

// command not found error for anything but "jest", as it most likely not be caused by env issue
const POSSIBLE_ENV_ERROR_REGEX =
Expand Down Expand Up @@ -45,7 +45,7 @@ export class AbstractProcessListener {
break;
}
case 'executableJSON': {
this.onExecutableJSON(jestProcess, args[0] as JestTotalResults);
this.onExecutableJSON(jestProcess, args[0] as AggregatedResult);
break;
}
case 'executableOutput': {
Expand Down Expand Up @@ -85,7 +85,7 @@ export class AbstractProcessListener {
this.CmdNotFoundEnv = true;
}
}
protected onExecutableJSON(_process: JestProcess, _data: JestTotalResults): void {
protected onExecutableJSON(_process: JestProcess, _data: AggregatedResult): void {
// no default behavior...
}
protected onExecutableOutput(_process: JestProcess, _data: string, _raw: string): void {
Expand Down Expand Up @@ -180,6 +180,7 @@ const IS_OUTSIDE_REPOSITORY_REGEXP =
const WATCH_IS_NOT_SUPPORTED_REGEXP =
/^s*--watch is not supported without git\/hg, please use --watchAlls*/im;
const RUN_EXEC_ERROR = /onRunComplete: execError: (.*)/im;
const ON_TEST_RESULT = /onTestResult: test: (.*)/im;
const RUN_START_TEST_SUITES_REGEX = /onRunStart: numTotalTestSuites: ((\d)+)/im;
const CONTROL_MESSAGES = /^(onRunStart|onRunComplete|Test results written to)[^\n]+\n/gim;

Expand Down Expand Up @@ -299,7 +300,7 @@ export class RunTestListener extends AbstractProcessListener {
}

//=== event handlers ===
protected onExecutableJSON(process: JestProcess, data: JestTotalResults): void {
protected onExecutableJSON(process: JestProcess, data: AggregatedResult): void {
this.session.context.updateWithData(data, process);
}

Expand All @@ -314,6 +315,8 @@ export class RunTestListener extends AbstractProcessListener {

this.onRunEvent.fire({ type: 'data', process, text: message, raw: cleaned });

this.handleTestResult(process, message);

this.handleRunComplete(process, message);

this.handleWatchNotSupportedError(process, message);
Expand All @@ -335,6 +338,17 @@ export class RunTestListener extends AbstractProcessListener {
this.onRunEvent.fire({ type: 'start', process });
}
}
protected handleTestResult(process: JestProcess, output: string) {
if (output.includes('onTestResult')) {
const data = output.match(ON_TEST_RESULT)?.[1];
if (!data) {
return;
}

const parsedData = JSON.parse(data) as TestResultJestRunEventArguments;
this.onExecutableJSON(process, parsedData.aggregatedResult);
}
}
protected handleRunComplete(process: JestProcess, output: string): void {
if (output.includes('onRunComplete')) {
this.runEnded();
Expand Down
9 changes: 7 additions & 2 deletions src/JestExt/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JestTotalResults, ProjectWorkspace } from 'jest-editor-support';
import { ProjectWorkspace } from 'jest-editor-support';

import * as vscode from 'vscode';
import { LoggingFactory } from '../logging';
Expand All @@ -8,6 +8,8 @@ import { JestProcessInfo } from '../JestProcessManagement';
import { JestOutputTerminal } from './output-terminal';
import { TestIdentifier } from '../TestResults';

import type { AggregatedResult } from '@jest/reporters';

export enum WatchMode {
None = 'none',
Watch = 'watch',
Expand All @@ -31,6 +33,9 @@ export interface JestExtSessionContext extends JestExtContext {
export interface RunEventBase {
process: JestProcessInfo;
}
export type TestResultJestRunEventArguments = {
aggregatedResult: AggregatedResult;
};
export type JestRunEvent = RunEventBase &
(
| { type: 'scheduled' }
Expand All @@ -47,7 +52,7 @@ export interface JestSessionEvents {
onTestSessionStopped: vscode.EventEmitter<void>;
}
export interface JestExtProcessContextRaw extends JestExtContext {
updateWithData: (data: JestTotalResults, process: JestProcessInfo) => void;
updateWithData: (data: AggregatedResult, process: JestProcessInfo) => void;
onRunEvent: vscode.EventEmitter<JestRunEvent>;
}
export type JestExtProcessContext = Readonly<JestExtProcessContextRaw>;
Expand Down
35 changes: 15 additions & 20 deletions src/TestResults/TestResult.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { TestReconciliationStateType } from './TestReconciliationState';
import { JestFileResults, JestTotalResults } from 'jest-editor-support';
import { FileCoverage } from 'istanbul-lib-coverage';
import { CoverageMap, FileCoverage } from 'istanbul-lib-coverage';
import * as path from 'path';
import { cleanAnsi, toLowerCaseDriveLetter } from '../helpers';
import { MatchEvent } from './match-node';
import type { AggregatedResult, TestResult as JestTestResult } from '@jest/reporters';

export interface Location {
/** Zero-based column number */
Expand Down Expand Up @@ -43,21 +43,21 @@ export interface TestResult extends LocationRange {
assertionHistory?: MatchEvent[];
}

function testResultWithLowerCaseWindowsDriveLetter(testResult: JestFileResults): JestFileResults {
const newFilePath = toLowerCaseDriveLetter(testResult.name);
function testResultWithLowerCaseWindowsDriveLetter(testResult: JestTestResult): JestTestResult {
const newFilePath = toLowerCaseDriveLetter(testResult.testFilePath);
if (newFilePath) {
return {
...testResult,
name: newFilePath,
testFilePath: newFilePath,
};
}

return testResult;
}

export const testResultsWithLowerCaseWindowsDriveLetters = (
testResults: JestFileResults[]
): JestFileResults[] => {
testResults: JestTestResult[]
): JestTestResult[] => {
if (!testResults) {
return testResults;
}
Expand All @@ -77,20 +77,15 @@ function fileCoverageWithLowerCaseWindowsDriveLetter(fileCoverage: FileCoverage)
return fileCoverage;
}

// TODO should fix jest-editor-support type declaration, the coverageMap should not be "any"
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const coverageMapWithLowerCaseWindowsDriveLetters = (data: JestTotalResults): any => {
export const coverageMapWithLowerCaseWindowsDriveLetters = (data: AggregatedResult): CoverageMap | undefined => {
if (!data.coverageMap) {
return;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const result: any = {};
const filePaths = Object.keys(data.coverageMap);
for (const filePath of filePaths) {
const newFileCoverage = fileCoverageWithLowerCaseWindowsDriveLetter(
data.coverageMap[filePath] as FileCoverage
);
for (const filePath of data.coverageMap.files()) {
const newFileCoverage = fileCoverageWithLowerCaseWindowsDriveLetter(data.coverageMap.fileCoverageFor(filePath));
result[newFileCoverage.path] = newFileCoverage;
}

Expand All @@ -105,8 +100,8 @@ export const coverageMapWithLowerCaseWindowsDriveLetters = (data: JestTotalResul
* @param data Parsed JSON results
*/
export const resultsWithLowerCaseWindowsDriveLetters = (
data: JestTotalResults
): JestTotalResults => {
data: AggregatedResult
): AggregatedResult => {
if (path.sep === '\\') {
return {
...data,
Expand All @@ -121,7 +116,7 @@ export const resultsWithLowerCaseWindowsDriveLetters = (
/**
* Removes ANSI escape sequence characters from test results in order to get clean messages
*/
export const resultsWithoutAnsiEscapeSequence = (data: JestTotalResults): JestTotalResults => {
export const resultsWithoutAnsiEscapeSequence = (data: AggregatedResult): AggregatedResult => {
if (!data || !data.testResults) {
return data;
}
Expand All @@ -130,8 +125,8 @@ export const resultsWithoutAnsiEscapeSequence = (data: JestTotalResults): JestTo
...data,
testResults: data.testResults.map((result) => ({
...result,
message: cleanAnsi(result.message),
assertionResults: result.assertionResults.map((assertion) => ({
failureMessage: cleanAnsi(result.failureMessage),
testResults: result.testResults.map((assertion) => ({
...assertion,
failureMessages: assertion.failureMessages.map((message) => cleanAnsi(message)),
})),
Expand Down
8 changes: 2 additions & 6 deletions src/TestResults/TestResultProvider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
TestReconciler,
JestTotalResults,
TestFileAssertionStatus,
IParseResults,
parse,
Expand All @@ -9,6 +8,7 @@ import {
ItBlock,
SnapshotParserOptions,
} from 'jest-editor-support';
import type { AggregatedResult } from '@jest/reporters';
import { TestReconciliationState, TestReconciliationStateType } from './TestReconciliationState';
import { TestResult, TestResultStatusInfo } from './TestResult';
import * as match from './match-by-context';
Expand Down Expand Up @@ -361,16 +361,12 @@ export class TestResultProvider {
* @param filePath
* @returns valid sorted test result or undefined if the file is not a test.
*/

getSortedResults(filePath: string): SortedTestResults | undefined {
if (this.isTestFile(filePath) === 'no') {
return;
}

const record = this.testSuites.get(filePath) ?? this.addTestSuiteRecord(filePath);
if (record.sorted) {
return record.sorted;
}

const sorted: SortedTestResults = {
fail: [],
Expand Down Expand Up @@ -398,7 +394,7 @@ export class TestResultProvider {
return sorted;
}

updateTestResults(data: JestTotalResults, process: JestProcessInfo): TestFileAssertionStatus[] {
updateTestResults(data: AggregatedResult, process: JestProcessInfo): TestFileAssertionStatus[] {
const results = this.reconciler.updateFileWithJestStatus(data);
results?.forEach((r) => {
const record = this.testSuites.get(r.file) ?? this.addTestSuiteRecord(r.file);
Expand Down
6 changes: 5 additions & 1 deletion src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,11 @@ export function escapeRegExp(str: string): string {
/**
* ANSI colors/characters cleaning based on http://stackoverflow.com/questions/25245716/remove-all-ansi-colors-styles-from-strings
*/
export function cleanAnsi(str: string): string {
export function cleanAnsi(str: string|undefined|null): string {
if(!str) {
return "";
}

return str.replace(
// eslint-disable-next-line no-control-regex
/[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g,
Expand Down
20 changes: 17 additions & 3 deletions src/reporter.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
// Custom Jest reporter used by jest-vscode extension

import type { AggregatedResult } from '@jest/test-result';
import { Reporter, TestContext } from '@jest/reporters';
import {
Reporter,
TestContext,
ReporterOnStartOptions,
Test,
TestResult,
AggregatedResult,
} from '@jest/reporters';
import type { TestResultJestRunEventArguments } from './JestExt';

class VSCodeJestReporter implements Reporter {
onRunStart(aggregatedResults: AggregatedResult): void {
onRunStart(aggregatedResults: AggregatedResult, _options: ReporterOnStartOptions): void {
process.stderr.write(
`onRunStart: numTotalTestSuites: ${aggregatedResults.numTotalTestSuites}\r\n`
);
Expand All @@ -18,9 +25,16 @@ class VSCodeJestReporter implements Reporter {
process.stderr.write('onRunComplete\r\n');
}
}

getLastError(): Error | undefined {
return;
}

onTestResult(_test: Test, _testResult: TestResult, aggregatedResult: AggregatedResult): void {
process.stderr.write(
`onTestResult: test: ${JSON.stringify({ aggregatedResult: aggregatedResult } as TestResultJestRunEventArguments)}\r\n`
);
}
}

module.exports = VSCodeJestReporter;
1 change: 0 additions & 1 deletion src/test-provider/test-item-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@ const isEmpty = (node?: ItemNodeType): boolean => {
return true;
};

// type AssertNode = NodeType<TestAssertionStatus>;
abstract class TestResultData extends TestItemDataBase {
constructor(readonly context: JestTestProviderContext, name: string) {
super(context, name);
Expand Down
7 changes: 2 additions & 5 deletions src/test-provider/test-provider-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,8 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol {
return this.options.end();
}

if (this.parentRun) {
this.parentRun.end();
Copy link
Author

Choose a reason for hiding this comment

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

This line seems to break a lot of the tests. Not sure if it is a real problem though, or it can be ignored. Also not sure how to solve it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change just a refactoring? I noticed the refactored logic is not the same as the original. Is there any other reason for changing the logic?

Copy link
Author

Choose a reason for hiding this comment

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

Is this change just a refactoring? I noticed the refactored logic is not the same as the original. Is there any other reason for changing the logic?

I actually added a comment on this line myself on the pull request.

It's what makes the tests fail around remembering to end the runs.

However, with this line on, when one of the tests switch status to green, the rest of them will also go out of the "running spinner" state, although they are still running.

Do you have any idea of why this happens?

Copy link
Collaborator

Choose a reason for hiding this comment

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

However, with this line on, when one of the tests switch status to green, the rest of them will also go out of the "running spinner" state, although they are still running.

My guess (without actually tracing the calls) is the code assumed batch mode, so once an "end" event is received, it will try to close the run. The TestExplorer will then mark all items "done" in that run accordingly. You might need to introduce a new stage/event when processing the partial result, or at least not trigger the "end" event before the whole run actually finishes...

if (isVscodeRun(this.parentRun)) {
this.parentRun = undefined;
}
if (this.parentRun && isVscodeRun(this.parentRun)) {
this.parentRun = undefined;
}

this.options?.onEnd?.();
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"noUnusedLocals": true,
"allowSyntheticDefaultImports": true,
"noUnusedParameters": true,
"forceConsistentCasingInFileNames": true,
"target": "es6",
"outDir": "out",
"lib": ["es6", "es7", "es2020", "es2021"],
Expand Down