-
Notifications
You must be signed in to change notification settings - Fork 13k
Adds tracking for failed tests, re-running failed tests. #16701
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
Gotta take a look but this is great :) 🍰 |
@@ -57,9 +58,12 @@ const cmdLineOptions = minimist(process.argv.slice(2), { | |||
inspect: process.env.inspect || process.env["inspect-brk"] || process.env.i, | |||
host: process.env.TYPESCRIPT_HOST || process.env.host || "node", | |||
browser: process.env.browser || process.env.b || "IE", | |||
bail: process.env.bail || 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.
What is bail
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.
Exit on the first failed test. This already exists in jakefile.js
scripts/mocha-file-reporter.js
Outdated
} | ||
} | ||
|
||
/** |
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.
nit: either we remove this or we add it everywhere with comments
scripts/mocha-file-reporter.js
Outdated
* @param keepFailed {Boolean} | ||
* @param fn {Function} | ||
*/ | ||
FileReporter.writeFailures = function (file, failures, keepFailed, fn) { |
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.
Could you rename file
to fileName
or filePath
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 only added it here for better intellisense.
c341c4b
to
ede814b
Compare
scripts/mocha-file-reporter.js
Outdated
var reporterOptions = this.reporterOptions = options.reporterOptions || {}; | ||
reporterOptions.file = reporterOptions.file || ".failed-tests"; | ||
reporterOptions.keepFailed = reporterOptions.keepFailed || false; | ||
reporterOptions.reporter = reporterOptions.reporter; |
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 looks like a rigid idiom, but it still bugs me. Can you remove 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.
Just one comment. Otherwise looks good.
@rbuckton can you refresh this PR? |
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.
Leaving a review to get this out of my pending PR reviews list
Superseded by #25004. |
This change adds the following capabilities:
runtests
orruntests-parallel
), any failing tests will be written to a .failed-tests file.--opts
option.--grep
option.jake runtests failed=true
orgulp runtests --failed
will run only the failing tests from the last test run.failed
option will be superseded by thetests
option, if both are provided.failed
option is not currently supported on theruntests-parallel
task.keepFailed=true
to jake or--keepFailed
to gulp.Examples (jake)
Examples (gulp)