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
keep hierarchy for skipped suites w/o a callback #3242
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.
Hi, thanks. I think this looks OK, but please see a couple comments I have.
I'll probably add more coverage around this after it's merged.
lib/interfaces/common.js
Outdated
@@ -113,6 +113,8 @@ module.exports = function (suites, context, mocha) { | |||
suites.shift(); | |||
} else if (typeof opts.fn === 'undefined' && !suite.pending) { | |||
throw new Error('Suite "' + suite.fullTitle() + '" was defined but no callback was supplied. Supply a callback or explicitly skip the suite.'); | |||
} else if (typeof opts.fn === 'undefined' && suite.pending) { |
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.
please simplify
} else if (!opts.fn && suite.pending) {
describe('a suite', function(){ | ||
describe.skip('skipped suite 1'); | ||
describe.skip('skipped suite 2'); | ||
describe('anothor suite', function(){ |
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.
spelling: "another"
aeb1b36
to
bf3b0a8
Compare
Thank you for the feedback. |
@outsideris Thanks! |
Requirements
Description of the Change
As my local test for #3184,
describe.skip
without a callback couldn't keep a hierarchy of suites.For Instance.
is show below output.
It is correct. But when omit a callback in
describe.skip
.A hierarchy of the suites is wrong like below.
So, this PR add
suites.shift();
for a skipped suite w/o a callback.Alternate Designs
I'm not sure.
Why should this be in core?
Some test can be skipped not intentionally.
Benefits
Skipped suites don't make side effect for other tests.
Possible Drawbacks
I'm not sure.
Applicable issues
#3184
I don't agree #2014 is related to this issue.