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

Option to disable globals #2765

Closed
stevenvachon opened this issue Apr 11, 2017 · 14 comments
Closed

Option to disable globals #2765

stevenvachon opened this issue Apr 11, 2017 · 14 comments

Comments

@stevenvachon
Copy link

I don't want anything mocha-related defined on global. I understand that not everyone cares to explicitly have everything required/imported, but I do, so it can be optional.

@ORESoftware
Copy link

ORESoftware commented Apr 12, 2017

Not a core contributor here, but I don't think this is possible the way Mocha is written. If it's really important to you, you may need to use a different test runner.

Just out of curiosity, what is your primary reason for not wanting a couple of global variables defined? (I can imagine a couple reasons).

@stevenvachon
Copy link
Author

  1. Nothing should be global, except what is defined as such in the language.
  2. Code quality. I like explicitly knowing where things are coming from just by looking at a file.

@ORESoftware
Copy link

ORESoftware commented Apr 12, 2017

I am 100% with you - if that's very important to you, then you should try AVA or TapJS/Tape, instead. Of course, a core member might step in and say that you can avoid globals with Mocha, but I am 99% certain that's not possible.

@ScottFreeCode
Copy link
Contributor

I believe that if bug #2207 were fixed, it would be possible/trivial to create a no-op interface that doesn't set up any globals and therefore would effectively leave you with the require calls as the only way to access Mocha's functions. Possibly something as simple as adding and using a file containing module.exports = function() {}

See also #2210

@ScottFreeCode
Copy link
Contributor

(Apologies for the double-post --) This would also fix #956, if I understand correctly. Seems like the complaint is very old and adding the ability to require Mocha's interface components was a partial solution that never was followed up with an option to disable the globals.

@ORESoftware
Copy link

Yeah, it would be nice to have:

const {describe, it, before, beforeEach, after, afterEach} = require('mocha');

or whatever

@ScottFreeCode
Copy link
Contributor

Right -- you can do that now. What you can't do is make it an error to use those functions without having required them.

But on the other hand, I think that this should work to achieve that:

// requires.js
module.exports = function() {}
mocha --ui requires

...except that Mocha's BDD interface is being set up no matter what other interface is selected, which is a bug anyway (e.g. it gets set up if you use mocha --ui tdd or mocha --ui exports too).

So in other words, I believe this request would be really simple to fulfill -- if we first fix an unrelated bug that merely happens to have an effect that renders the solution here moot.

@ScottFreeCode
Copy link
Contributor

Urgh -- you might be right about this enhancement not being so possible; I was just digging into #2207 and discovered along the way that the "require interface" is implemented by way of a giant hack: hardcoding the exports in question for BDD and TDD to reference whichever one is set up globally.

...I... don't even know what to say to that. The only reason I can imagine it might have been done is that Mocha's interfaces technically get set up once per file rather than once and for all globally when run in Node, using a convoluted system of event-emitting from the root suite as files are loaded. But on the other hand, I can't imagine a good reason to set up the interface once per file like that -- and it certainly won't be doing that in the browser, so I would be surprised if it's even having any effect now as it is... This looks an awful lot like a backwards-compatibility hack for legacy code that was designed around environment assumptions that aren't even valid anymore -- though of course, being absolutely certain it's that and not breaking anything if we changed it... Hrmm. Not easy.

Well, now I'm just annoyed enough I might try to get a PR working just to spite the whole thing, but I can't make any promises.

@ScottFreeCode
Copy link
Contributor

Yup, you were right all along -- this isn't feasible, short of some pretty drastic changes to the semi-internal structure.

It's pretty weird -- I tried to fake the system out by imitating the global setup but using a dummy suite and a dummy object in place of the global object, to see if I could get the interface to attach to the dummy object instead -- no such luck, because while I was able to call the interface functions exported that way, doing so didn't cause the tests to be used. Apparently there's some hidden coupling buried in there somewhere between the suite or some such context and what the interface functions interact with under the hood.

...Which, horrifyingly, makes a certain amount of sense now that I think about the fact that you can theoretically create multiple instances using the Mocha constructor programmatically and you'd need to tie any given test to that instance, but the interface itself is global. Of course, by "a certain amount of sense" I mean "why is it tied to so many hard-to-substitute things instead of only being tied to the instance of Mocha??" and "why is this whole setup brokenly convoluted in order to allow multiple instances of Mocha when multiple instances of Mocha is itself broken for other reasons??" To which there are probably further answers, but at this point I don't know that I want to find out.

Even having worked with Mocha for some time now, I'm still learning new things. Unfortunately, at this point I'm mostly learning just how deep the rabbit-hole goes. Talk about legacy code.

@drazisil drazisil added the status: waiting for author waiting on response from OP - more information needed label Apr 12, 2017
@stale stale bot added the stale this has been inactive for a while... label Aug 12, 2017
@stale
Copy link

stale bot commented Aug 12, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stevenvachon
Copy link
Author

So, the mocha team would rather quit and go the way of grunt?

@stale stale bot removed the stale this has been inactive for a while... label Aug 12, 2017
@ScottFreeCode ScottFreeCode removed the status: waiting for author waiting on response from OP - more information needed label Aug 12, 2017
@boneskull
Copy link
Contributor

@stevenvachon please keep your comments constructive.

duplicate of #956

@stevenvachon
Copy link
Author

It was constructive, albeit a bit negative.

@boneskull
Copy link
Contributor

what’s constructive is finding a solution, offering to help, or sending a PR

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

No branches or pull requests

5 participants