Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to supply options programmatically #414

Open
newhoggy opened this issue Apr 8, 2024 · 20 comments
Open

Add ability to supply options programmatically #414

newhoggy opened this issue Apr 8, 2024 · 20 comments

Comments

@newhoggy
Copy link

newhoggy commented Apr 8, 2024

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
Copy link
Collaborator

Bodigrim commented Apr 8, 2024

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

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

@Bodigrim
Copy link
Collaborator

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

@newhoggy
Copy link
Author

newhoggy commented Apr 10, 2024

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 #411

@Bodigrim
Copy link
Collaborator

launchTestTree opts tree k0 = do
(testActions, fins) <- createTestActions opts tree
let NumThreads numTheads = lookupOption opts

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
Copy link
Author

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
Copy link
Collaborator

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

@newhoggy
Copy link
Author

newhoggy commented Apr 10, 2024

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
Copy link
Author

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

  • A way to time tests and record them in a file.
  • A way to put timeouts on a test
  • A way to retry a test if it fails
  • A way to retry a test if it times out

@Bodigrim
Copy link
Collaborator

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
Copy link
Author

newhoggy commented Apr 10, 2024

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
Copy link
Author

newhoggy commented Apr 10, 2024

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
Copy link
Collaborator

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
Copy link
Author

newhoggy commented Apr 10, 2024

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
Copy link
Author

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
Copy link
Collaborator

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

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

thoughtpolice pushed a commit to kadena-io/chainweb-node that referenced this issue Apr 30, 2024
This slows the tests by 20s on my machine but is probably worth it
for iteration speed.

See UnkindPartition/tasty#414 for background.

Change-Id: I4bd7f0ba24b29c4ce9bf18ea2842f57337c2dfa3
thoughtpolice pushed a commit to kadena-io/chainweb-node that referenced this issue Apr 30, 2024
This slows the tests by 20s on my machine but is probably worth it
for iteration speed, because it lets run individual tests (or groups)
at our whim.

See UnkindPartition/tasty#414 for background.

Change-Id: I4bd7f0ba24b29c4ce9bf18ea2842f57337c2dfa3
thoughtpolice pushed a commit to kadena-io/chainweb-node that referenced this issue Apr 30, 2024
This slows the tests by 20s on my machine but is probably worth it
for iteration speed, because it lets run individual tests (or groups)
at our whim.

See UnkindPartition/tasty#414 for background.

Change-Id: I4bd7f0ba24b29c4ce9bf18ea2842f57337c2dfa3
edmundnoble added a commit to kadena-io/chainweb-node that referenced this issue May 3, 2024
This slows the tests by 20s on my machine but is probably worth it
for iteration speed, because it lets run individual tests (or groups)
at our whim.

See UnkindPartition/tasty#414 for background.

Change-Id: I4bd7f0ba24b29c4ce9bf18ea2842f57337c2dfa3
@Bodigrim
Copy link
Collaborator

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

@newhoggy
Copy link
Author

Thanks!

@phadej
Copy link
Contributor

phadej commented Aug 13, 2024

adjustOptions doesn't work with e.g. --quickcheck-tests. If I do

main = defaultMain $ adjustOption (\_ -> QuickCheckTests 1000) $ testGroup ...

then --quickcheck-tests wont do anything on the CLI, as I override it. I could use \QuickCheckTests n -> QuickCheckTests (n * 10) but that feels like a hack, as I need to know the default. But I want to override the default.

I think I'm looking for the API like

main = defaultMainWithOptions [SomeOption (QuickCheckTests 100), ...] $ testGroup ...

which would prepend option values. This could even override the defaultValue in --help with a bit more plumbing.

@Bodigrim
Copy link
Collaborator

I normally use adjustOption (max (QuickCheckTests 1000)), so that I can override it with a bigger number from CLI if needed. Isn't it good enough?

@phadej
Copy link
Contributor

phadej commented Aug 14, 2024

@Bodigrim no, you cannot override it with smaller one. The #427 isn't that complicated addition for what I'd consider a principal solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants