Skip to content

Conversation

@boneskull
Copy link
Member

IMO Suite#timeout should eventually do nothing if its parameter is undefined. it should not be up to Mocha's constructor to guard against this.

@boneskull boneskull self-assigned this Mar 7, 2019
@boneskull boneskull added the type: bug a defect, confirmed by a maintainer label Mar 7, 2019
@boneskull
Copy link
Member Author

cleaned up the constructor unit tests a little while I was in there

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 91.629% when pulling f44ee90 on boneskull/issue/3813 into e654253 on master.

@plroebuck
Copy link
Contributor

As undefined is not a valid value for any Mocha option, should the user "options" object have them stripped before merged with default values?

Otherwise, won't this just turn into a game of whack-a-mole?

@boneskull
Copy link
Member Author

boneskull commented Mar 7, 2019

As undefined is not a valid value for any Mocha option, should the user "options" object have them stripped before merged with default values?

Otherwise, won't this just turn into a game of whack-a-mole?

I'm thinking no, because prior to v6, anybody trying to pass undefined values via options in a way which was not previously guarded against would find a thrown exception.

The problem here, in particular, is that I accidentally removed such a guard.

If someone tries this with some other option that doesn't handled undefined nicely, they will get an exception--not new behavior--and I suppose we can live with that for now.

@boneskull
Copy link
Member Author

A 0.06% decrease in coverage should be a warning. 0.08 means your IDE gets impounded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch implementation requires increase of "patch" version number; "bug fixes" type: bug a defect, confirmed by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants