UnkindPartition / tasty

Modern and extensible testing framework for Haskell
637 stars 108 forks source link

Add ability to supply options programmatically #414

Open newhoggy opened 3 months ago

newhoggy commented 3 months ago

Sometimes it is useful for a test suite to use a default that is different than the one that tasty uses.

For example a test suite might have some concurrency issues and needs NumThreads 1 to be able to run correctly.

Whilst this can be done manually with --num-threads 1, ideally test-suites should be able to run correctly out of the box without additional flags.

Bodigrim commented 3 months ago

Are existing mechanisms (such as adjustOption) insufficient? Why?

There is also sequentialTestGroup which can potentially mitigate multithreading issues as discussed in #406.

Bodigrim commented 3 months ago

If this is about NumThreads specifically, defaultMain* could unwrap top levels of PlusTestOptions and incorporate them into opts. This would avoid extending API.

newhoggy commented 3 months ago

sequentialTestGroup doesn't work for us because it creates dependencies between tests.

This breaks --pattern for us because if we select a test with --pattern it will run not just the selected tests but its dependencies as well.

See https://github.com/UnkindPartition/tasty/issues/411

Bodigrim commented 3 months ago

https://github.com/UnkindPartition/tasty/blob/83b3a53087b391bf831c19e0a04dd2dd93dbb884/core/Test/Tasty/Run.hs#L605-L607

Instead of directly going for lookupOption, one can unwrap top-level PlusTestOptions from tree and apply them to opts first, then lookup NumThreads in the result.

newhoggy commented 3 months ago

Hmm, should it not be other way around? ie. if --num-threads is supplied on the command line, it should overwrite what's specified by the tests?

Bodigrim commented 3 months ago

No, adjustOption overrides global environment and in an ideal world NumThreads should be no exception.

newhoggy commented 3 months ago

Is tasty expressive enough to model SingleThreaded as an option?

That would be ideal for us because it would allow a sub-tree to be run single threaded allowing the rest to be concurrent.

newhoggy commented 3 months ago

There are some other things I'm also interest in:

Bodigrim commented 3 months ago

Is tasty expressive enough to model SingleThreaded as an option?

That would be ideal for us because it would allow a sub-tree to be run single threaded allowing the rest to be concurrent.

It should be possible to extend data ExecutionMode with one more constructor, similar to Sequential, but not affecting --pattern. This is a rather delicate matter though and a breaking change, and we would rather abstain from a new major release for a while. That said, I imagine you could be fine depending on Git repo...

What's your use case? Usually when people want to execute tests sequentially, it's because they depend on the output of each other and that's why Sequential affects --pattern. But you made it clear that this is undesirable.

A way to time tests and record them in a file.

There are TestReporters to do this, e. g., tasty-ant-xml.

A way to put timeouts on a test

localOption (mkTimeout 10000).

A way to retry a test if it fails A way to retry a test if it times out

Could be implemented as newtypes with custom IsTest instances, I imagine.

newhoggy commented 3 months ago

What's your use case? Usually when people want to execute tests sequentially, it's because they depend on the output of each other and that's why Sequential affects --pattern. But you made it clear that this is undesirable.

The use case is we're running integration tests in CI. Theoretically the tests are independent, however the code being tested is timing sensitive and in CI, with limited compute, the tests interfere with each other so they must not run at the same time.

We desire to have no dependencies between tests because we want --pattern to select as if there are no dependencies.

In summary, we don't need sequential, we need non-concurrency.

newhoggy commented 3 months ago

Could be implemented as newtypes with custom IsTest instances, I imagine.

It seems inappropriate for a test itself to retry because retrying means all the retries would get lumped together in one timing, which makes the timeouts not work properly, and the test reporter would have no visibility over the timing of each individual retry or that the test retried n times.

Bodigrim commented 3 months ago

The use case is we're running integration tests in CI. Theoretically the tests are independent, however the code being tested is timing sensitive and in CI, with limited compute, the tests interfere with each other so they must not run at the same time.

Hmm. Can you wrap each test in withResource, taking and putting an MVar, so that they block on each other?

It seems inappropriate for a test itself to retry because retrying means all the retries would get lumped together in one timing, which makes the timeouts not work properly, and the test reporter would have no visibility over the timing of each individual retry or that the test retried n times.

instance IsTest has access to timeouts, so you can pass 1/N of top-level timeout to each of retries. And you can put arbitrary data in resultDescription.


You can swap the default TestManager with a custom one, doing whatever you need. This is rather heavyhanded approach for casual projects, but might be a reasonable choice for you.

newhoggy commented 3 months ago

Can you wrap each test in withResource, taking and putting an MVar, so that they block on each other?

That was the first thing I tried except I used semaphores, but for some reason it didn't work. The test still failed.

newhoggy commented 3 months ago

You can swap the default TestManager with a custom one, doing whatever you need

Ah yes, I see that it skips launchTestTree altogether. Do you know of an example implementation of TestManager that runs tests?

Bodigrim commented 3 months ago

Do you know of an example implementation of TestManager that runs tests?

https://hackage-search.serokell.io/?q=TestManager

Bodigrim commented 1 month ago

@newhoggy could you possibly check the latest master? You should be able to specify adjustOptions (NumThreads 1) on the top level now.

newhoggy commented 1 month ago

Thanks!