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.

Sign up for GitHub

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

Jump to bottom

Add ability to unload files from require cache #3534

Closed
wants to merge 12 commits into from
Closed

Add ability to unload files from require cache #3534

wants to merge 12 commits into from

Conversation

plroebuck
Copy link
Contributor

@plroebuck plroebuck commented Oct 26, 2018 •
edited
Loading

Description of the Change

Adds a feature which allows unloading test files from require cache programmatically.
This feature simplifies rerunning tests since there is no need to recreate the Mocha instance.

Currently handled by one of these options:

  1. The user has to implement same logic in his own code.
  2. The user has to create a new Mocha instance before each run.

Alternative Designs

A potentially more complete design could take a snapshot of the cache before testing begins and remove everything added afterwards. Prefer to see how well this implementation does before going further.

Why should this be in core?

  1. Because it allows reuse of object instances.
  2. Provides missing half of Mocha's public API referenced here.
  3. (Hopeful) reduction of user questions about how to do this.

Benefits

Better reusability, fewer headaches for programmers.

Possible Drawbacks

Still possible that even though registered files were unloaded from the require cache,
they may have in turn required something else which will remain in the cache.

Applicable issues

semver-minor

@plroebuck plroebuck added type: feature enhancement proposal area: usability concerning user experience or interface semver-minor implementation requires increase of "minor" version number; "features" labels Oct 26, 2018
@plroebuck plroebuck self-assigned this Oct 26, 2018
@plroebuck
Copy link
Contributor Author

Previous discussion can be found in original PR #3388.

@plroebuck
Copy link
Contributor Author

plroebuck commented Oct 26, 2018 •
edited
Loading

Intended to use Mocha#unloadFile method as purge implementation here, but kept getting weird cascade of prettier errors.

#3544 caused by JSDoc creating an out directory...

@coveralls
Copy link

coveralls commented Oct 26, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 90.374% when pulling 5b42d9d on WithoutCaps-master into adb1f61 on master.

@plroebuck plroebuck added the status: needs review a maintainer should (re-)review this pull request label Oct 26, 2018
@plroebuck plroebuck mentioned this pull request Oct 26, 2018
Closed
@plroebuck plroebuck changed the title Adds ability to unload files from require cache Add ability to unload files from require cache Oct 27, 2018
@plroebuck plroebuck removed the request for review from outsideris October 31, 2018 00:54
boneskull
boneskull suggested changes Nov 2, 2018
View reviewed changes
Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document this on the site.

Other than that, just some suggestions/comments; no other major issues.

lib/mocha.js Show resolved Hide resolved
lib/mocha.js Show resolved Hide resolved
test/node-unit/mocha.spec.js Show resolved Hide resolved
test/node-unit/mocha.spec.js Outdated Show resolved Hide resolved
lib/mocha.js Show resolved Hide resolved
@plroebuck
Copy link
Contributor Author

We should document this on the site.

What is this in reference to? More than the API docs?

@plroebuck
test(node-unit/mocha.spec.js): Update assertions
5b42d9d 
Converted from 'to have property' to 'to have keys', eliminating `forEach` loops. Saved results of
`require.resolve` to variable to prevent rework.
@plroebuck plroebuck added this to the v6.0.0 milestone Nov 4, 2018
@plroebuck plroebuck removed the status: needs review a maintainer should (re-)review this pull request label Nov 4, 2018
@plroebuck
Copy link
Contributor Author

@boneskull anything else?

@boneskull
Copy link
Contributor

@plroebuck I mean in docs/index.md; document the feature and show a use case. not sure where, exactly, but a new section before "Usage" seems reasonable.

@boneskull
Copy link
Contributor

the other changes look good

@boneskull boneskull removed this from the v6.0.0 milestone Dec 10, 2018
@boneskull
Copy link
Contributor

this still needs documentation if we want it in v6.0.0

@plroebuck
Copy link
Contributor Author

Done long ago https://github.com/mochajs/mocha/wiki/Using-Mocha-programmatically

@boneskull
Copy link
Contributor

hmm, no, I guess it doesn't need documenting on the main site.
conflicts will need to be resolved

@plroebuck plroebuck mentioned this pull request Feb 13, 2019
Merged
@juergba juergba deleted the WithoutCaps-master branch January 8, 2020 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bamieh Bamieh Bamieh approved these changes

@boneskull boneskull boneskull approved these changes

Assignees

@plroebuck plroebuck

Labels
area: usability concerning user experience or interface semver-minor implementation requires increase of "minor" version number; "features" type: feature enhancement proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@plroebuck @coveralls @boneskull @Bamieh