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

Expose the globals as modules in mocha. #1077

Merged
merged 3 commits into from
Jan 7, 2014
Merged

Conversation

Raynos
Copy link
Contributor

@Raynos Raynos commented Dec 19, 2013

This change allows you to write mocha tests without referencing globals in your test files.

This is useful when you want to lint your tests and your linter complains about globals.

For example

var assert = require("assert")
var describe = require("mocha/describe")
var it = require("mocha/it")
describe('Array', function(){
  describe('#indexOf()', function(){
    it('should return -1 when the value is not present', function(){
      assert.equal(-1, [1,2,3].indexOf(5));
      assert.equal(-1, [1,2,3].indexOf(0));
    })
  })
})

This also has a benefit of being able to write mocha tests in either the BDD or the TDD style by just requiring either test or it and the command line setting of the interface is irrelevant.

@tj
Copy link
Contributor

tj commented Dec 24, 2013

hmm personally I'd rather export them in mocha than have tons of files

@tj
Copy link
Contributor

tj commented Dec 24, 2013

otherwise LGTM

@rauchg
Copy link
Contributor

rauchg commented Dec 24, 2013

+1 on exposing them as part of the mocha namespace, -1 on multiple files

@jonathanong
Copy link
Contributor

wow. such files.

@jasonpincin
Copy link

+1 on this happening either as files or in namespace, will take either

@travisjeffery
Copy link
Contributor

+1 under mocha's namespace

@Raynos wanna update your changes?

@Raynos
Copy link
Contributor Author

Raynos commented Dec 24, 2013

@guille @travisjeffery the beautiful, beautiful files :(

I exposed them in lib/mocha.js instead.

@Raynos
Copy link
Contributor Author

Raynos commented Dec 30, 2013

@visionmedia is this good to go?

@travisjeffery
Copy link
Contributor

@Raynos no, they're undefined there since the methods aren't put on global until pre-require is emitted on the suite here: https://github.com/visionmedia/mocha/blob/master/lib/mocha.js#L156.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 1, 2014

@travisjeffery that shouldn't matter. I require mocha at the top of the test file so since the globals exist by then I will get them.

@travisjeffery
Copy link
Contributor

run it. that pre-require event is named with respect to the test file, e.g. require('some_test_file'), not mocha.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 1, 2014

Ok, I'll look into it, add a test case and make sure it actually works.

The multiple files approach would have just worked for free :P

@Raynos
Copy link
Contributor Author

Raynos commented Jan 2, 2014

@travisjeffery you were right. I added a test!

It works now.

@visionmedia is there a place where i can PR the docs for mocha itself ? there not in this repo as far as I can see.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 2, 2014

@travisjeffery because we have have redefined describe and it as local variables it will use the exported ones not the global ones.

Without the fix (moving the exports in the event listener) it will fail.

I'll happily update it so that it bootstraps the mocha suite runner if it hasn't been bootstrapped by the time its required. then tests can be run with node

The point of this fix is to not have globals in your test files even if you run the tests with mocha. It also allows you to name describe and it whatever the hell you want.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 2, 2014

I've thought about how to get mocha working with node but it would get dirty since people require mocha to run the test runner programmatically and requiring mocha to get a test runner set up in the background for you would conflict with that.

I could probably get it to work with require('mocha/describe') and have that bootstrap a mocha test runner with the default settings if the program was not run with the mocha executable but run with node directly. That way the programmatic require('mocha') api to create and run your own test runners won't be effected.

@rauchg
Copy link
Contributor

rauchg commented Jan 7, 2014

+1 – very good PR imho

travisjeffery added a commit that referenced this pull request Jan 7, 2014
Expose the globals as modules in mocha.
@travisjeffery travisjeffery merged commit cc0341c into mochajs:master Jan 7, 2014
@travisjeffery
Copy link
Contributor

👍

@Raynos docs are in the gh-pages branch.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 7, 2014

@travisjeffery thanks! I've added a PR into the gh-pages to document this feature.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 7, 2014

@travisjeffery @guille if you could bump the minor / patch version and publish this then that would be great :)

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.

6 participants