Allow Multiple Glob/Path Arguments from CLI

[cpruitt]

Addresses:

• Allow multiple patterns (globs & exclude patterns) #32
• CLI should take multiple paths and not a glob pattern #44.

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 CLI should take multiple paths and not a glob pattern #44 (comment))
• 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]

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

[searls]

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]

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]

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]

@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]

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]

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]

The JS needs refactoring and docs need updating.

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

[cpruitt]

@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.

[searls]

Looking good, didn't read all the criteria changes very closely. I'm cool merging if we can rename the --example flag ❤️

[searls]

I had to re-read the PR a couple times to understand what was meant by --example, assuming this is a BDD/rspecism that leaked from the code. Would --name be more predictable and consistent with other tools like minitest?

If you agree, I'd ask you to change the name of this option to --name or similar before we merge

[cpruitt]

@searls This has been changed to --name

[searls]

Would you be open to refactoring this so we're not re-assigning the same var over and over in the same function? Seems like the sort of thing that'd best(be(several(calls())))) to named functions, hopefully obviating the comments

[cpruitt]

@searls I've refactored https://github.com/testdouble/teenytest/blob/92ce2b86d66845e4c0dbd5df59127b548cff2809/lib/configure/criteria.js. If anything still seems like it should be changed (or just rubs you the wrong way) let me know.

[cpruitt]

@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]

@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]

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]

Landed in 5.3.0 👏