Mocha: grep behaviour is badly documented

[ghost]

I think that the following line of libmocha.js:

should change to

[dasilvacontin]

You should justify/provide reasons. Anyways, your code wouldn't work.

var str = 'test+sort';
var re = new RegExp(str);
re.test(str); //false

[dasilvacontin]

'The value of grep option not exactly match the suite name'. If with option you mean function argument, then indeed, it doesn't. Because it's not the intended behaviour of the function. Otherwise, we would need another function that given a string it generates a regex without 'exact match' limitations.

If with option you mean the --grep option, it doesn't either. With mocha --grep apple I should be able to run all tests containing apple on its title. If you want an exact match, provide a regex to the --grep option.

[ghost]

@dasilvacontin,thank you for the reply. I mean the --grep option. Please provide an example of --grep option that can do an exact match.

[dasilvacontin]

It should be mocha --grep ^apple$. However, it doesn't work.

I've investigated the issue. Runner tests the regex against test.fullTitle(). fullTitle() returns the name of the test preceded by the descriptions in the describes. That way we can easily match all the tests nested in the describe. However, to allow an exact-match regex, we should also test against the test title only. I propose adding an OR:

var match = self._grep.test(test.fullTitle()) || self._grep.test(test.title);

Any thoughts @boneskull @travisjeffery?

[dasilvacontin]

Same thing for matching against suite title.

[a8m]

I'm fan of the currently grep behavior, it looks sense and fits great with tdd/bdd workflow
this change could cause an inconsistent behavior, so I'm -1 on this change, you should make you regex more robust.

[dasilvacontin]

@a8m, are you commenting on @fzx's change, or my suggestion?

[jbnicolai]

@dasilvacontin I'm on the fence in regards to your suggestion. I see the use case, and feel like it should be supported, but am also hesitant as it makes the behaviour less intuitive.

[ghost]

Why 'It should be mocha --grep ^apple$. However, it doesn't work'? I think the cause is in the lib/mocha.js line number of 226(see my pic at first post). The ^ and $ char are escaped by function escapeRe.

[a8m]

on your suggestion @dasilvacontin.

[dasilvacontin]

@fzx yes, they are escaped. But in any case, the regex is being checked against parent describes + suite/test title.

[dasilvacontin]

@jbnicolai I would personally use .only to achieve this behaviour, tbh.

[ghost]

I dislike the .only method, because you will sometimes forget to remove it.

[ghost]

@dasilvacontin, why to escape the --grep option value?

[a8m]

You should use the .only option @fzx (just don't forget about it haha).

[a8m]

mixing matching titles would cause an unexpected behaviour @dasilvacontin

[dasilvacontin]

@fzx I don't know. Mocha is not checking if value for --grep is a string or a regex. Not sure if it's a bug, or regex was never meant to be used with --grep and I misunderstood the use of the word pattern in mocha's command line documentation.

[dasilvacontin]

@a8m, what would you currently expect if you tried using an exact-match regex via --grep?

And when not using a match-begin/end regex, behaviour would stay the same, wouldn't it? (not meaning I'm in favor of the change, just curious since you mention unexpected behaviour)

[a8m]

@dasilvacontin you can use --grep with exact-match style today
like so: mocha dir/ --grep='^Your.*(?=Reg).+Exp$'.
I׳m just saying that test-matching in two different titles is not a good idea imo

[jbnicolai]

test-matching in two different title is not a good idea imo

I agree

[a8m]

@dasilvacontin for example, running this tests with: --grep=^Array.+toString, run both.
I'm expecting to run only the first one.

describe('Array', function() {
  describe('.toString()', function() {
    it('....', function() {
      // ...
    });
  });
});

describe('My super smart function', function() {
  it('Array with .toString() logic', function() {
    // ...
  });
});

[dasilvacontin]

test-matching in two different title is not a good idea imo

I agree too, it's horrible.

[dasilvacontin]

@fzx, actually, since mocha is creating a regex with the given value of --grep at /bin/_mocha, the command line input is not being escaped.

@a8m, do you think that's clear to users? Because without the underlying-understanding of how grep matches work, I wonder where/how people figure out it's going to work that way, if it's not by trying. To be honest, I found out today. And that makes me worry.

[a8m]

@dasilvacontin I'm a user(a happy one, haha).
I think it's easy to get it after you play it a bit, but we can make the user's life more easier by expending the docs.
I can assign it to myself.

[dasilvacontin]

Yep, improving the documentation is what I was about to propose. I took a look at it and it only mentions super easy cases that don't even use regex. Sure, go ahead. :)

[iongion]

a8m how did you do it, this does not work mocha tests/ --grep="^root$", but this does mocha tests/ --grep="root"

[ralf57]

Any news about this?

[dasilvacontin]

@ralf57 behaviour is intended, just badly documented. Regex is applied on full title.

[dasilvacontin]

I dislike the .only method, because you will sometimes forget to remove it.

Also, if like @ghost you ran into this because you forget to remove .only, you could use https://github.com/dasilvacontin/dot-only-hunter.

[boneskull]

@ScottFreeCode Do you think you can update this with your recent additions for grep?

[ScottFreeCode]

I don't think any of us have done any recent additions for grep, that I recall, and I know I haven't -- in fact as far as I remember the closest I've come to touching grep was submitting some old PRs to fix the HTML reporter's usage of it (but not the underlying behavior).

[dschuyler]

If I understand this thread correctly: I'd prefer to see an additional function added (aside from grep() and fgrep()) to match the whole string. That third function would essentially do an fgrep with the ^ $ to match the whole string.