-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(testrunner): pass error into test fixtures #3605
Conversation
16fecbb
to
dd0a98e
Compare
}); | ||
|
||
async function runTest(filePath: string) { | ||
const outputDir = path.join(__dirname, 'test-results') |
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.
this will break if tests are run in parallel, which they are in my patch in review. #3595
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.
Would it make sense to convert runTest into a fixture that depends on the parallelIndex then to isolate output folders?
@@ -81,7 +81,7 @@ export async function runTests(config: RunnerConfig, suite: Suite, reporter: Rep | |||
// Trial run does not need many workers, use one. | |||
const jobs = (config.trialRun || config.debug) ? 1 : config.jobs; | |||
const runner = new Runner(suite, { ...config, jobs }, reporter); | |||
|
|||
fs.mkdirSync(config.outputDir, { recursive: true }); |
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.
Let's also clean it before running?
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 can't because the index file is required by every test worker. This should be in cli right?
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 decided to clean it on the call site, otherwise can be unexpected...
No description provided.