Allow Multiple Glob/Path Arguments from CLI

cpruitt commented 5 years ago

Addresses:

I this is a PR to allow Multiple glob pattern arguments from the CLI. This will allow either multiple globs to be passed or will allow the shell to perform the expansion and pass multiple file path arguments. Each glob argument can have it's own # or : filter which is applied to any files matched by the glob. The same glob can be supplied multiple times with different filters. There is also an --example option which can be provided multiple times to specify global name filters which apply to all matched files.

Internally, the glob expansion step has been moved into the process of configuration criteria which happens very early on in the execution of the test runner (basically it's the first thing that happens). Instead of a single glob pattern, the configuration now holds an array of objects. Each object represents one unique file path (after glob expansion) along with any filters that have been provided for that path.

Criteria is now processed so that:

A single string or an array can be provided
Array elements are always expanded as globs by JavaScript, even if the shell already expanded them (See https://github.com/testdouble/teenytest/issues/44#issuecomment-524578711)
A Line number or function name filter suffix can be supplied as part of any glob in the array and it will only apply to files matching that glob pattern.
A global example (name) filter can also be provided as a string or array which will apply to all files (I'm open to changing the name of this property/flag)
After glob expansion duplicate files are removed with filters consolidated. Duplicate filters of the same type (line/name) are removed.
If a test name and a line number filter both resolve to the same test, that duplicate is not currently handled

As an example, given the arguments:

'./test/path/file-*.js#testOne' \
'./test/path/file-one.js#testOne' \
'./test/path/file-one.js#testTwo' \
'./test/path/file-two.js#testThree' \
'./test/path/file-two.js:56' \
'./test/path/file-two.js:78'

config.criteria.testFiles would be built as:

[
    {
        file: './test/path/file-one.js',
        name: ['testOne', 'testTwo']
    },
    {
        file: './test/path/file-two.js',
        name: ['testOne', 'testThree'],
        lineNumber: ['56', '78']
    },
]

Unit tests have been added for configure but they only cover the test criteria, not other aspects of the config.

Changes outside of configure have been made so that the existing SAFE tests are passing with these changes. (i.e. the criteria change doesn't break the world).

Bats tests have been added for the new CLI behavior. Bats tests are located in /safe along with other tests.

Readme has been updated with some additional information on multiple globs, name/line filters, and the global --example option.

CC: @searls @jasonkarns

cpruitt commented 5 years ago

Oops, looks like CI is saying let declarations are a problem. I'll clean that up shortly.

searls commented 5 years ago

Made two small commits after cursory review. Basically tests pass and the tests seem to say what you want teenytest to do! I didn't have time to scrutinize the additions to criteria.js etc

/me ducks out and lets you look at the CLI

cpruitt commented 5 years ago

Thanks for the commits. I worked on some CLI tests a bit today. I'm going to finish those up and then plan to go back and refactor the config a bit. Would like (🤞🏼) to have it at least working and pretty solid by the end of the week.

jasonkarns commented 5 years ago

We should probably combine the two bats "suites". Before this PR, we have a single bats test living under safe/example.bats (which is just a bats variant of the old example tests).

Strictly speaking, we could keep the bats tests under safe/ since it only runs .bats files but eh... @searls opinions on where the full e2e/cli tests should go in this project?

I'm somewhat tempted to just drop all the bats tests under safe/ since that's what they are. And we could rewrite the existing safe JS tests into bats, since they're already asserting on the CLI and TAP output.

searls commented 5 years ago

@jasonkarns you're closer to both bats and this project than I've been for a while so I am glad to defer to you. Consolidation sounds like a good word to me

cpruitt commented 5 years ago

My initial thought (like @jasonkarns) was that the existing safe tests could be switched over to bats tests exercising the CLI and everything could be consolidated. While working on this PR, however, it seemed practical to just keep them separated as a mental "here's a bucket for the stuff I'm working with" and then consolidate afterward when I have a better grasp of what's being consolidated. While working on this I wanted to leave the existing JS SAFE tests in place as a check to be sure noting changed (or at least, if it did that it was appropriately handled).

Also, just as a minor status update: As I'm writing the CLI tests, the behavior of dealing with multiple per-file filters isn't turning out to be correct out of the box. The config is being set up the way we want but the tests look like they are being run with the named filters combined as "matches all of" instead of "matches any of" so the end result is that something like teenytest './test-file.js#test_aaa' './test-file.js#test_bbb' will result in zero tests being run.

cpruitt commented 5 years ago

I've got teenytest working now with multiple glob/path args and have combined the ./bats tests into ./safe. The JS needs refactoring and docs need updating.

searls commented 5 years ago

The JS needs refactoring and docs need updating.

Are you tackling that or are you suggesting someone else do?

cpruitt commented 5 years ago

@searls Sorry, that wasn’t very clear. Yes, I intend to do both of those. I wanted to leave an update on the PR that the multi-glob feature was working but that I still had some cleanup work to do.

cpruitt commented 4 years ago

@searls If I recall, the --example naming came from a conversation with @jasonkarns about taking cues from rspec. I don't recall if that was explicit or if I was just taking the discussion too literally but, in any case, I don't disagree with changing it to --name.

I also don't disagree with the refactor to use nested functions. The variable reassignment was a decision of personal preference that I thought might not mesh well with the rest of the codebase. I personally still find nested(function_calls(to_be(harder_to(parse_quickly())))) and the way I had it made it easier for me to follow what I was doing, but I expect that I'm the exception, not the rule in that case. I'm happy to make the change. 🙂

cpruitt commented 4 years ago

@searls Sorry for the delay on the changes. Time has been a scarce resource lately. I added a few commits to address your change requests. If there’s anything else you’d like to see let me know.

searls commented 4 years ago

Nice, looks fantastic! Good README, left the code better-factored and better-tested than you found it. I know it took a lot of calendar time to see to completion but I think this is a really great feature that sets teenytest apart from a lot of other runners, big and small. Thanks, Cliff!

searls commented 4 years ago

Landed in 5.3.0 👏

testdouble / teenytest

Allow Multiple Glob/Path Arguments from CLI #47