Skip Cabal package tests that cannot run in the current environment

[grayjay]

There are three commits:

Revert "Add flag to disable Cabal package tests that use shared libraries"

#3146 wasn't flexible enough to allow different tests to be skipped for different reasons.

Use a writer monad to define Cabal package tests

This commit addresses a TODO comment and allows tests to be filtered and grouped more easily.

Skip Cabal package tests that cannot run in the current environment

The functions testWhen and testUnless filter tests in the TestTreeM monad. The option --enable-all-tests ignores filtering. This commit also applies filtering to the existing package tests that cannot run on Windows.

The test suite chooses whether to disable tests that use shared libraries by checking D.System.buildOS and the version of GHC used for --with-ghc. Is that correct?

/cc @ezyang

[23Skidoo]

Looks good to me. @ezyang, what do you think?

[ezyang]

I am not sure if this is better but let me throw this comment out there and see what you think.

At the moment, the writer monad is set up so that you immediately run some code to determine if a test is runnable or not. This means you need a reader monad if you want to override some of the test hiding behavior.

An alternative would be to actually reify the condition tree, so that you can separate into two steps: (1) generating the tree (which would generate everything, and tag every subtree with the condition, either an AST or just a plain old function predicate, saying what needs to be true to run it), and then (2) a prune step, which actually goes ahead and removes tests. This saves you the reader monad, and if you AST-ize the conditions gives you the ability to manually override a condition or another. This also encourages centralization of predicates.

[ezyang]

Otherwise LGTM. I don't think it's worth bikeshedding this change too much, as long as the tests are still easy to write (looks like it to me) and the interface supports the use cases you need (looks like it to you.) We can refactor more if we need more functionality later.

[23Skidoo]

OK, I think I'm going to merge this PR now. Further improvements can come as separate PRs.

[23Skidoo]

Merged. Thanks, @grayjay!

[grayjay]

Thanks!

I am not sure if this is better but let me throw this comment out there and see what you think.

At the moment, the writer monad is set up so that you immediately run some code to determine if a test is runnable or not. This means you need a reader monad if you want to override some of the test hiding behavior.

An alternative would be to actually reify the condition tree, so that you can separate into two steps: (1) generating the tree (which would generate everything, and tag every subtree with the condition, either an AST or just a plain old function predicate, saying what needs to be true to run it), and then (2) a prune step, which actually goes ahead and removes tests. This saves you the reader monad, and if you AST-ize the conditions gives you the ability to manually override a condition or another. This also encourages centralization of predicates.

I was hoping to avoid the reader monad by using Tasty's support for options. It only provides functions like askOption :: IsOption v => (v -> TestTree) -> TestTree, though, which don't allow filtering.

I went with the reader/writer monad because it was easy to apply to the existing tests. I think that a tree with conditions that can be inspected would become more useful as the logic for choosing tests becomes more complex.