[WIP] Implement --focus and --skip CLI options

[ezzatron]

@brianium Here's a working prototype of the --focus and --skip CLI options.

In its current form, it works like Ginkgo, in that if you specify either option on the command line, it completely overrides any tests that are focused "programatically" with an f prefix. Seems like the approach that will cause the least confusion.

Definitely not perfect, and no tests yet, but hopefully it's a starting point for discussion :)

[codecov-io]

Current coverage is 96.97% (diff: 100%)

Merging #197 into feature/focused-specs will increase coverage by 0.16%

@@           feature/focused-specs       #197   diff @@
=======================================================
  Files                         20         20          
  Lines                        658        695    +37   
  Methods                      165        173     +8   
  Messages                       0          0          
  Branches                       0          0          
=======================================================
+ Hits                         637        674    +37   
  Misses                        21         21          
  Partials                       0          0          

Powered by Codecov. Last update 2973fa2...ff8bd78

[brianium]

@ezzatron Looks like a pretty simple implementation that offers a lot of functionality. I just had one question to confirm my understanding, but I think this looks good so far

[brianium]

Just to double check, we are saying $skipPattern and $focusPattern are not mutually exclusive right?

You could run a test that has a combination of a focus pattern and a skip pattern? Like "run integration tests, but skip the selenium ones?"

[ezzatron]

Yeah, that's the intent, at least. Ginkgo has a similar feature:

When using the command line flags you can specify one or both of --focus and --skip. If both are specified the constraints will be ANDed together.

It's not really clear what they mean by "ANDed together", but I assume they meant "matched by --focus AND NOT matched by --skip". It seems like the most useful and intuitive option.

[ezzatron]

@brianium I've pushed tests for this PR, so I think it's probably ready to pull, unless you have more questions or feedback.

[brianium]

Tests look good, just added one more question about validating patterns.

[brianium]

Should we validate patterns somehow? If a non delimited regex is given, PHP will throw warnings:

$ php -a
php > preg_match('lol', 'rofl');
PHP Warning:  preg_match(): Delimiter must not be alphanumeric or backslash in php shell code on line 1

Warning: preg_match(): Delimiter must not be alphanumeric or backslash in php shell code on line 1

[brianium]

We could argue that an invalid regex is the tester's problem, but a more helpful error message might be nice

[ezzatron]

Agreed, we could be more helpful / handle the warnings better. Based off of this StackOverflow answer, we can validate the patterns by running them with @ suppression against a null value.

I had given some thought to supporting different behaviour when you don't provide a full regex. For example, if someone types --focus foo.*, we could interpret that in a number of useful ways, rather than just rejecting it.

My personal preference would probably be to surround the pattern in /\b and \b/, turning the above example into /\bfoo.*\b. The \b matches word boundaries, which means that foo.* would match foo and foobar, but not barfoo. It would also still work with things like ^foo$, because \b also matches the start and end of the line. See here for an example.

[ezzatron]

I've pushed a proof of concept for the above. It "normalizes" the focus patterns in three steps:

• If the pattern is valid, it is used as-is.
• Otherwise, a new pattern is created by surrounding the original in ~\b and \b~, with instances of ~ replaced with \~.
• If this new pattern is also invalid, then a 'fall-back" pattern is created using preg_quote().

So basically, if the pattern is valid regex, but missing delimiters, then it will "just work" like a normal regex. If the pattern has really messed up stuff like unbalanced parentheses, then it will use plaintext matching instead.

Also, I figured ~ is a better delimiter here because / is more likely to be used in test descriptions.

The use of \b is probably debatable here, but I know that when I use PHPUnit's --filter option, I am constantly using them to avoid matching unwanted substrings.

[brianium]

I think this looks good man. \b seems like a sane default if an explicit pattern is not given. Anything else in the pipeline for this? If not, I'll pull it down and give the CLI a go. If that’s all good I am good to merge this.

[ezzatron]

@brianium I don't think there's any more code to write, it's only really the docs. I've already document fit(), fcontext(), and fdescribe(), but not these new CLI options. Shouldn't take too long though.

I opened up another PR on the docs with the current progress: peridot-php/peridot-php.github.io#3

[ezzatron]

@brianium Shall I squash this down, and then we can organize a time to make a new release together?

[brianium]

@ezzatron Sounds good man!