-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
feat: Added skipped and focused status to FormattedTestResult
#13700
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Could you add a changelog entry?
packages/jest-test-result/src/__tests__/formatTestResults.test.ts
Outdated
Show resolved
Hide resolved
packages/jest-test-result/src/__tests__/formatTestResults.test.ts
Outdated
Show resolved
Hide resolved
| testResults: [skippedAssertion], | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }; | |
| } as AggregatedResult; |
| title: 'is still pending', | ||
| }; | ||
|
|
||
| const skippedResults: AggregatedResult = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const skippedResults: AggregatedResult = { | |
| const skippedResults = { |
| numPendingTests: 2, | ||
| numTodoTests: 2, | ||
| perfStats: {end: 2, runtime: 1, slow: false, start: 1}, | ||
| // @ts-expect-error |
There was a problem hiding this comment.
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.
| // @ts-expect-error |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>
|
|
||
| import formatTestResults from '../formatTestResults'; | ||
| import {AggregatedResult} from '../types'; | ||
| import type {AggregatedResult, AssertionResult} from '../types'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimenB It was unnecessary to add AssertionResult. My mistake, see: #13700 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't hurt
skipped and focused status to FormattedTestResultskipped and focused status to FormattedTestResult
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
fix #5711
Summary
Added the
skippedandfocusedstatus toFormattedTestResultas suggested in this comment.In my implementation, I consider the test
skippedif thetestResult.skippedflag is true.I consider the test
focusedif there are still pending tests and no failures occur.Let me know if this is the correct approach or if changes need to be made.