Repository: mochajs/mocha

Problem statement:
Window properties impeded using mocha
I recently tried to use mocha in a project whose runtime environment had a `window.ui` property and we had to patch our copy of mocha.js to make it work. The window property overshadows the mocha prototype property and mocha wouldn't even initialize properly. I suspect there are other critical properties that, if they appear as members of window, will cause similar problems.

I'm not sure if this is something the project wants to address directly, or if it's a "Doctor, it hurts when I do that - Don't do that" but I figured I'd report it anyway.

### Prerequisites

- [x] Checked that your issue hasn't already been filed by cross-referencing [issues with the `faq` label](https://github.com/mochajs/mocha/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Afaq%20)
- [x] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
- [x] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
- [x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: `node node_modules/.bin/mocha --version`(Local) and `mocha --version`(Global). We recommend that you _not_ install Mocha globally.

Actually, the last two are kind of N/A for this issue.

### Description

In this piece of code at the end of mocha.js, windows properties overwrite mocha properties, even those that are necessary for mocha to function.
```javascript
	var browserEntry = Object.assign(mocha, commonjsGlobal);
	return browserEntry;
```
In our case, the ui function was getting overwritten by a ui property introduced by the Foundry execution framework in which we were running our tests.

Once we adjusted this code to do the following, we were able to run mocha tests in our environment in the usual way:

```javascript
	// Wicked hack for Quench - 
	// get ui property out of copy of global merged with mocha before merging so it doesn't clobber mocha's ui function

	var hackedCommonJsGlobal = Object.assign({}, commonjsGlobal);
	delete hackedCommonJsGlobal.ui;
	var browserEntry = Object.assign(mocha, hackedCommonJsGlobal);
	return browserEntry;
```

It occurs to me that there may be other mocha properties that shouldn't be overshadowed by properties from window as well, and that mocha could protect itself from.

### Steps to Reproduce

Not sure how applicable this is.  Set up a globalThis.ui property before you import mocha and you should see the effect.

**Expected behavior:** Property doesn't affect mocha's ability to initialize before running tests.

**Actual behavior:** Property overshadows mocha's ui function and mocha is unable to initialize from options, even `setup('bdd')`, before running any tests. 


**Reproduces how often:** Quite consistently - if your globalThis has properties that happen to overshadow mocha properties.

### Versions

We were working with mocha 8.x and 9.x. The version of node doesn't specifically matter. 

### Additional Information

We aren't running mocha standalone from a command line in NOde, and we aren't spawning a process to run it and piping back the results, but we're running it as a library embedded in another app running in the browser. The tests need to use the APIs of the browser app in which the runner is executing.

This issue was found in the context of a module that lets us run tests within the FoundryVTT environment. This module adds the UI to the environment to kick off the tests and display the results, but it does it along with loading the core UI, the world, the system, etc.  so quite a lot of stuff for the environment has loaded, including the window.ui property, well before the imports of this specific module are invoked (including the mocha and chai imports). It is intended to be used in support of building other modules.

With this one tweak, the tests seem to run fine.




Hints:

Patch:
diff --git a/browser-entry.js b/browser-entry.js
--- a/browser-entry.js
+++ b/browser-entry.js
@@ -213,7 +213,4 @@ Mocha.process = process;
 global.Mocha = Mocha;
 global.mocha = mocha;
 
-// this allows test/acceptance/required-tokens.js to pass; thus,
-// you can now do `const describe = require('mocha').describe` in a
-// browser context (assuming browserification).  should fix #880
-module.exports = Object.assign(mocha, global);
+module.exports = mocha;
diff --git a/lib/cli/watch-run.js b/lib/cli/watch-run.js
--- a/lib/cli/watch-run.js
+++ b/lib/cli/watch-run.js
@@ -46,7 +46,7 @@ exports.watchParallelRun = (
 
       // this `require` is needed because the require cache has been cleared.  the dynamic
       // exports set via the below call to `mocha.ui()` won't work properly if a
-      // test depends on this module (see `required-tokens.spec.js`).
+      // test depends on this module.
       const Mocha = require('../mocha');
 
       // ... and now that we've gotten a new module, we need to use it again due
@@ -108,7 +108,7 @@ exports.watchRun = (mocha, {watchFiles, watchIgnore}, fileCollectParams) => {
 
       // this `require` is needed because the require cache has been cleared.  the dynamic
       // exports set via the below call to `mocha.ui()` won't work properly if a
-      // test depends on this module (see `required-tokens.spec.js`).
+      // test depends on this module.
       const Mocha = require('../mocha');
 
       // ... and now that we've gotten a new module, we need to use it again due


Test patch:
diff --git a/test/browser-specific/setup.js b/test/browser-specific/setup.js
--- a/test/browser-specific/setup.js
+++ b/test/browser-specific/setup.js
@@ -8,3 +8,5 @@ global.expect = require('unexpected')
   .use(require('unexpected-map'))
   .use(require('unexpected-sinon'))
   .use(require('unexpected-eventemitter'));
+
+require('../../browser-entry');
diff --git a/test/unit/required-tokens.spec.js b/test/unit/required-tokens.spec.js
deleted file mode 100644
--- a/test/unit/required-tokens.spec.js
+++ /dev/null
@@ -1,12 +0,0 @@
-'use strict';
-
-var assert = require('assert');
-var describe = require('../..').describe;
-var it = require('../..').it;
-
-describe('using imported describe', function() {
-  it('using imported it', function(done) {
-    assert.ok(true);
-    done();
-  });
-});