mochajs / mocha

☕️ simple, flexible, fun javascript test framework for node.js & the browser
https://mochajs.org
MIT License
22.64k stars 3.02k forks source link

🚀 Feature: Ability to teardown globals on after hooks #2206

Closed jwalton closed 10 months ago

jwalton commented 8 years ago

When run with --check-leaks, this test:

describe('geolocate', () => {
    before(() => global.navigator = {});
    after(() => delete global.navigator);

    it('should get the current position from navigator', () => {
        // Do nothing.
    });

});

fails with "Error: global leak detected: navigator", even though the after cleans up the navigator object.

dasilvacontin commented 8 years ago

@jwalton globals are checked right after the it finishes:

If you don't check after the it, you can't know whether the global was introduced by the hook or the it.

ScottFreeCode commented 8 years ago

Given that the "leak" in this case is coming from a hook (the before hook, rather than the after hook), that's a bit ironic; could leaks be checked after the before/beforeEach hooks and any detected there be added to the ignored globals for the check against the tests?

dasilvacontin commented 8 years ago

@ScottFreeCode well pointed out, I had overlooked that detail. Maybe it would be nice to track the existing globals before the it and after the it so we can allow global variable teardown in hooks.

ScottFreeCode commented 8 years ago

Another nice thing about that approach: it would probably also reduce the cases for which globals need to be ignored in general, since if I'm not mistaken right now it's needed to ignore globally declared dependencies and such even if they didn't leak from the actual tests.

jwalton commented 8 years ago

It is nice to find globals that don't leak from the tests, though. Saved me once or twice when I was missing a "var" at top level scope. :)

danielstjules commented 8 years ago

could leaks be checked after the before/beforeEach hooks and any detected there be added to the ignored globals for the check against the tests?

I think that's a bit too much magic. I would much prefer having the check run after all afterEach/after hooks, which would allow @jwalton's original example to pass.

JoshuaKGoldberg commented 10 months ago

Duplicate of #4954. That one was filed much later than this one, but has a very clean reproduction case - so let's go with it.