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.

Sign up for GitHub

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

Jump to bottom

feat: Added skipped and focused status to FormattedTestResult #13700

Merged
merged 9 commits into from Dec 31, 2022
Merged

feat: Added skipped and focused status to FormattedTestResult #13700

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ab2ae6d
feature: Added skipped and focused status to FormattedTestResult
lpizzinidev Dec 30, 2022
86c7a47
fixed test result status evaluation
lpizzinidev Dec 30, 2022
fd93b8c
Added changelog entry and brought back erased test
lpizzinidev Dec 31, 2022
c6ccacd
Added type casting to tests and missing  type
lpizzinidev Dec 31, 2022
0b55335
Removed unecessary typecast to AssertionResult
lpizzinidev Dec 31, 2022
bf6d28f
Apply suggestions from code review
SimenB Dec 31, 2022
9e1e23a
Revert "Apply suggestions from code review"
SimenB Dec 31, 2022
6bf4cfc
Revert "Removed unecessary typecast to AssertionResult"
SimenB Dec 31, 2022
ba61051
move variables inside test scope
SimenB Dec 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -2,6 +2,8 @@

### Features

- `[jest-test-result]` Added `skipped` and `focused` status to `FormattedTestResult` ([#13700](https://github.com/facebook/jest/pull/13700))

### Fixes

- `[jest-resolve]` add global paths to `require.resolve.paths` ([#13633](https://github.com/facebook/jest/pull/13633))
Expand Down
56 changes: 55 additions & 1 deletion packages/jest-test-result/src/__tests__/formatTestResults.test.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
*/

import formatTestResults from '../formatTestResults';
import {AggregatedResult} from '../types';
import type {AggregatedResult} from '../types';
SimenB marked this conversation as resolved.
Show resolved Hide resolved

describe('formatTestResults', () => {
const assertion = {
Expand All @@ -32,4 +32,58 @@ describe('formatTestResults', () => {
assertion.fullName,
);
});

const skippedAssertion = {
fullName: 'Pending test',
status: 'pending',
title: 'is still pending',
};
SimenB marked this conversation as resolved.
Show resolved Hide resolved

const skippedResults: AggregatedResult = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const skippedResults: AggregatedResult = {
const skippedResults = {

testResults: [
{
numFailingTests: 0,
numPassingTests: 0,
numPendingTests: 2,
numTodoTests: 2,
perfStats: {end: 2, runtime: 1, slow: false, start: 1},
// @ts-expect-error
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of @ts-expect-error it would be better to use casting (see other suggestions) in all tests of this file.

I see that @ts-expect-error was already present here and you just followed the pattern. It works, but casting feels somewhat cleaner.

Suggested change
// @ts-expect-error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Applied changes (thank you for the feedback).
The pipeline keeps failing but I don't think it is related to my changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect. Thanks! Apologies about misleading suggestion before, I just noticed that all AssertionResult casts are actually unnecessary. Could you remove them, please?

There is an odd issue with type checks on CI. Hard to debug, because all works smoothly locally. I was trying to find the root of this problem, but no luck so far. Can be ignored for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pipelines are a nightmare to debug 😄
When you'll have the time, I have 2 open PRs which should be ready for review (#13639 and #13607).

testResults: [skippedAssertion],
},
],
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
};
} as AggregatedResult;


it('should mark result status to skipped', () => {
const result = formatTestResults(skippedResults, undefined, null);
expect(result.testResults[0].assertionResults[0].status).toEqual(
skippedAssertion.status,
);
});

const focusedAssertion = {
fullName: 'Focused test',
status: 'focused',
title: 'focused test',
};

const focusedResults: AggregatedResult = {
testResults: [
{
numFailingTests: 0,
numPassingTests: 1,
numPendingTests: 1,
numTodoTests: 2,
perfStats: {end: 2, runtime: 1, slow: false, start: 1},
// @ts-expect-error
testResults: [focusedAssertion],
},
],
};

it('should mark result status to focused', () => {
const result = formatTestResults(focusedResults, undefined, null);
expect(result.testResults[0].assertionResults[0].status).toEqual(
focusedAssertion.status,
);
});
});
21 changes: 20 additions & 1 deletion packages/jest-test-result/src/formatTestResults.ts
View file
Open in desktop
Expand Up @@ -33,6 +33,21 @@ const formatTestResult = (
};
}

if (testResult.skipped) {
const now = Date.now();
return {
assertionResults: testResult.testResults,
coverage: {},
endTime: now,
message: testResult.failureMessage ?? '',
name: testResult.testFilePath,
startTime: now,
status: 'skipped',
summary: '',
};
}

const allTestsExecuted = testResult.numPendingTests === 0;
const allTestsPassed = testResult.numFailingTests === 0;
return {
assertionResults: testResult.testResults,
Expand All @@ -44,7 +59,11 @@ const formatTestResult = (
message: testResult.failureMessage ?? '',
name: testResult.testFilePath,
startTime: testResult.perfStats.start,
status: allTestsPassed ? 'passed' : 'failed',
status: allTestsPassed
? allTestsExecuted
? 'passed'
: 'focused'
: 'failed',
summary: '',
};
};
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-test-result/src/types.ts
View file
Open in desktop
Expand Up @@ -125,7 +125,7 @@ export type FormattedTestResult = {
message: string;
name: string;
summary: string;
status: 'failed' | 'passed';
status: 'failed' | 'passed' | 'skipped' | 'focused';
startTime: number;
endTime: number;
coverage: unknown;
Expand Down