-
-
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
getConsoleOutput receives global noStackTrace #10081
Conversation
@SimenB can you have a look when you are available 😃 |
@@ -14,15 +14,16 @@ import { | |||
import type {ConsoleBuffer} from './types'; | |||
|
|||
export default ( | |||
// TODO: remove in 26 | |||
// TODO: remove in 27 |
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.
Missed 26, I can follow closer and create PR when 27 is near.
packages/jest-runner/src/runTest.ts
Outdated
@@ -121,6 +121,7 @@ async function runTestInternal( | |||
// 4 = the console call is buried 4 stack frames deep | |||
BufferedConsole.write([], type, message, 4), | |||
config, | |||
globalConfig.noStackTrace, |
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.
can we pass the whole globalConfig
instead?
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 an integration test as well? That way we can be 100% sure it works 🙂
96c4714
to
ed13e4d
Compare
config: StackTraceConfig = { | ||
rootDir: root, | ||
testMatch: [], | ||
}, | ||
globalConfig: Config.GlobalConfig, |
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.
Needs to be optional until next major
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.
I created this one and can create a PR for it, please let me know if anything is missing.
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 👍
@@ -40,12 +43,12 @@ export default ( | |||
if (type === 'warn') { | |||
message = chalk.yellow(message); | |||
typeMessage = chalk.yellow(typeMessage); | |||
noStackTrace = false; | |||
noStackTrace = false || globalConfig.noStackTrace; |
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.
false ||
will always fallback
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 for catching! updated in a following commit.
as CLI option or config
@@ -40,12 +44,12 @@ export default ( | |||
if (type === 'warn') { | |||
message = chalk.yellow(message); | |||
typeMessage = chalk.yellow(typeMessage); | |||
noStackTrace = false; | |||
noStackTrace = globalConfig ? globalConfig.noStackTrace : false; |
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.
noStackTrace = globalConfig ? globalConfig.noStackTrace : false; | |
noStackTrace = globalConfig?.noStackTrace ?? false; |
import {formatStackTrace} from 'jest-message-util'; | ||
import getConsoleOutput from '../getConsoleOutput'; | ||
import BufferedConsole from '../BufferedConsole'; | ||
import {LogType} 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.
import {LogType} from '../types'; | |
import type {LogType} from '../types'; |
'warn', | ||
]; | ||
|
||
cases.forEach(logType => { |
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.
it.each
?
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!
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. |
Summary
Although
formatStackTrace
acts according toStackTraceOptions
,getConsoleOutput
needs to takenoStackTrace
value in global config and use it to override the options passed toformatStackTrace
. This will resolve the bug mentioned hereTest plan
Pass existing test cases. Adding new test case to ensure
getConsoleOutput
does override corresponding configuration.