-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: Added skipped
and focused
status to FormattedTestResult
#13700
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>
@@ -6,7 +6,7 @@ | |||
*/ | |||
|
|||
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 FormattedTestResult
skipped
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
skipped
andfocused
status toFormattedTestResult
as suggested in this comment.In my implementation, I consider the test
skipped
if thetestResult.skipped
flag is true.I consider the test
focused
if 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.