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

Fix issue #325 (add better grep support to js api) #376

Closed

Conversation

davemckenna01
Copy link
Contributor

Fix issue #325 - When testing in the browser, the mocha object now has a grep() method which you can chain like so: mocha.grep('foo').run() - with @shwaydogg

@tj
Copy link
Contributor

tj commented Apr 10, 2012

we should use the Mocha prototype in ./lib/mocha.js, right now you can only pass a grep: option but it would be nice to have this there, allowing a regexp or a string

@davemckenna01
Copy link
Contributor Author

Ah I see - I had it in my head that this was an enhancement to browser test running only. But ya we should totally make this api available in Node, etc.

One question: it appears that in the browser tests you have that you're not actually instantiating a Mocha object ... you're adding methods (specifically .run() and .setup()) to the Mocha constructor, and not using the Mocha.prototype methods. What's the reason for that?

Related to that, should we keep the code we have in support/tail.js?

@tj
Copy link
Contributor

tj commented Apr 10, 2012

it is available to node, in ./lib/mocha.js, we need to do the opposite, bring the node js api to the client portion. A reasonable amount of tail.js would be replaced by ./lib/mocha.js

@shwaydogg
Copy link

Is the following the best way or an appropriate way to run mocha: programmatically / from the node REPL / test the API?

var mocha = require('./bin/_mocha');

mocha.run();

@tj
Copy link
Contributor

tj commented Apr 13, 2012

@shwaydogg there's a js api now, but it's built to be.. an api haha not a super simple REPL thing, if you wanted that why not just use the CLI?

@davemckenna01
Copy link
Contributor Author

Ok we've got mocha.grep() and new Mocha({grep:..}) working nicely - how would you like us to test this? Do you want tests a la Mocha itself:

describe('grep()', function(){
  it('should add a string to grep for', function(){
    var mocha = new Mocha({grep:/bar/});
    console.log(mocha);
    assert.equal(/bar/, mocha.grep);
  });
});

or a la what you have in test/jsapi/index.js:

var mocha = new Mocha({
  ui: 'bdd',
  globals: ['okGlobalA', 'okGlobalB'],
  // ignoreLeaks: true,
  growl: true
});

...

mocha.run(function(){
  console.log('done');
}).on('pass', function(test){
  // console.log('... %s', test.title);
});

@tj
Copy link
Contributor

tj commented Apr 13, 2012

awesome :D

Ideally we have some unit tests, my test/jsapi/index.js is just because I dont have time, so probably something more like the first

@davemckenna01
Copy link
Contributor Author

Tests added - should be good to go :)

@tj
Copy link
Contributor

tj commented Apr 15, 2012

weird git wont let me curl the .patch, I was going to squash these into one commit

Mocha.prototype.grep = function(re){
if ('string' === typeof re) re = new RegExp(re);
if (re && 'RegExp' === re.constructor.name) this.options.grep = re;
return this;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just me but I stay away from overly defensive programming , I'd do:

this.options.grep = 'string' == typeof re
  ? re = new RegExp(re)
  : re;

same with above

Copy link
Contributor

Choose a reason for hiding this comment

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

my bad, no clue why i re-assigned re:

this.options.grep = 'string' == typeof re
  ? new RegExp(re)
  : re;

@davemckenna01
Copy link
Contributor Author

Cool - I cleaned up those lines - ya the commits are a mess sorry. Curling the patch works for me.

@davemckenna01
Copy link
Contributor Author

I can just create a new branch with the one commit, and pull request that, if that's easier for you.

@davemckenna01
Copy link
Contributor Author

Slipped by me too - fixed.

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.

3 participants