Skip to content
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 for duplicate entries in mocha.options.globals #3934

Closed
wants to merge 1 commit into from

Conversation

pascalpp
Copy link
Contributor

@pascalpp pascalpp commented May 28, 2019

Fixes #3933

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

Adds a deduping reducer to Mocha.prototype.globals to ensure that no duplicate items are added.

Alternate Designs

Decided against using Array.prototype.includes, opted to use IndexOf instead, to ensure the solution has a broad base of support since mocha runs in a lot of places. The method is already using Array.prototype.filter, so I think it's safe to use Array.prototype.reduce, which I believe has equivalent support.

Why should this be in core?

Slight change to a core method of the Mocha constructor API. Not a candidate for its own package.

Benefits

Mostly just house-cleaning.

Possible Drawbacks

Possible speed impact, but probably negligible.

Applicable issues

#3933
Originated from a discussion with @juergba here: #3914

@pascalpp pascalpp changed the title added test and fix for duplicate globals Fixes https://github.com/mochajs/mocha/issues/3933 Fix for duplicate entries in mocha.options.globals May 28, 2019
@pascalpp
Copy link
Contributor Author

@juergba Here's that PR I promised you to fix the duplicate globals issue. Let me know if you think this is the right approach.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 92.429% when pulling 74a9896 on pascalpp:issue/3933 into 29b7615 on mochajs:master.

@pascalpp
Copy link
Contributor Author

Will attempt to resolve this in #3914

@pascalpp pascalpp closed this May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mocha constructor creates duplicate entries in mocha.globals
2 participants