-
-
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
Resolves the test sequencer even when not explicitly set #8267
Conversation
Yeah... |
@@ -478,6 +478,10 @@ export default function normalize( | |||
options.testRunner = require.resolve('jest-jasmine2'); | |||
} | |||
|
|||
if (!options.testSequencer) { |
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.
good catch. Do we need the if
at all? Or can we do what we do on line 463?
options.testEnvironment = getTestEnvironment({
rootDir: options.rootDir,
testEnvironment: options.testEnvironment || DEFAULT_CONFIG.testEnvironment,
});
So (ish)
options.testSequencer = getTestSequencer({
rootDir: options.rootDir,
testSequencer: options.testSequencer || DEFAULT_CONFIG.testSequencer,
});
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.
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.
(might be a slippery slope, and should rather be part of a larger refactor of normalize
)
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.
If my comment doesn't make sense, feel free to ignore it 😀
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.
Thank you! Just a lint error to resolve and then will release it.
e2e test |
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
The default value of the test sequencer isn't resolved unless explicitly set (we only iterate on
options
, which contains the user-defined options but not the default ones). This diff ensures that we callrequire.resolve
to obtain the real default path of the sequencer (since@jest/core
is making the require call, it would fail otherwise under PnP).Test plan
I modified the code in my cache and checked that my tests were running. We probably should try to add an integration test for PnP at some point.