UnkindPartition / tasty

Modern and extensible testing framework for Haskell
640 stars 110 forks source link

Allow --pattern|-p option to repeat #380

Closed rhendric closed 1 year ago

rhendric commented 1 year ago

The effect of multiple -p options is the same as &&-ing the expressions together, but allowing them to be specified in separate options makes scripting test commands simpler.


Motivation: I want to run tests with a small shell script such as the following:

#!/usr/bin/env bash
default_test_options=(...)
test-exe "${default_test_options[@]}" "$@"

If default_test_options contains a pattern filter like -p ! /very-slow-test/, then I'm currently unable to provide any options to the script to further focus on a set of interest. I would have to write some extra logic to the script to detect pattern filters and inject them into the default pattern filter, making the script much more complicated than I would like it to be.

Allowing multiple -p options to be provided to Tasty programs resolves the issue.

Bodigrim commented 1 year ago

CC @martijnbastiaan for review

rhendric commented 1 year ago

Since there are two monoids (conjunction and disjunction), and people have different ideas which one should be the default, I think we should not arbitrarily decide on either.

But it isn't arbitrary. Of the two monoids, only conjunction has a mempty consistent with what Tasty does when no -p options are provided: run all the tests.

But you cannot have monoidal env vars.

I don't understand this objection. Sure, the interface of environment variables doesn't let you specify a variable multiple times. But it's still the case that anything you can specify with command line options can also be specified with env vars. It's also still the case that environment variables are overridden if any matching options are provided on the command line. What problems would this cause?

andreasabel commented 1 year ago

Of the two monoids, only conjunction has a mempty consistent with what Tasty does when no -p options are provided: run all the tests.

Ok, this is an argument for the bias you are implementing here. I am convinced.

rhendric commented 1 year ago

How and where should tests be written for this? core-tests looks a little idiosyncratic; am I supposed to test this by creating a new executable in core-tests.cabal with accompanying .hs and .sh files? And then how are those things invoked?

Bodigrim commented 1 year ago

am I supposed to test this by creating a new executable in core-tests.cabal with accompanying .hs and .sh files?

That would be fine from my perspective.

And then how are those things invoked?

Please check .github/workflows/ci.yml.

rhendric commented 1 year ago

I've written tests, added a changelog entry, and taken another approach with optionCLParser that will probably make everyone angry.

Bodigrim commented 1 year ago

...and taken another approach with optionCLParser that will probably make everyone angry.

I'm actually quite happy with it :) @andreasabel @martijnbastiaan opinions?

Bodigrim commented 1 year ago

@andreasabel just a gentle reminder to review, I'd like to wrap up things for a release soon.

Bodigrim commented 1 year ago

Thanks @rhendric!