-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
#2819 - this.skip() in before fails to skip tests with nested describe calls #2836
Conversation
this.skip() in before fails to skip tests with nested describe calls
Changes Unknown when pulling 5267c53 on Elergy:issue/2819 into ** on mochajs:master**. |
Thank you, I didn't see #2571 before. |
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 have a question about one of the changes and I think the failure messages could be improved.
}); | ||
|
||
it('should skip this test', function () { | ||
expect(2).to.equal(1); |
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.
In all the failure cases, you should call fail directly: expect().to.fail('message here')
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!
lib/runnable.js
Outdated
* Mark the runnable as pending or not pending | ||
* @param pending | ||
*/ | ||
Runnable.prototype.setPending = function (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.
What is the value of switching from property to a method?
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.
In most cases, it's better to use setters instead of using properties directly.
It's a common problem with using properties (and I encountered this problem while fixing this bug) that it's not easy to understand where someone changes the property.
You know that the property was changed, but you don't know when - all you can do is to debug the whole application.
You can search for a string 'pending = ', but it can be a problem when several objects contain fields with the same name.
if you have a setter, you can easily set a breakpoint inside the setter and find who called this method very quickly.
4547268
to
7613521
Compare
this.skip() in before fails to skip tests with nested describe calls