Add filterable sequential test groups by Raveline · Pull Request #448 · UnkindPartition/tasty

Raveline · 2025-07-03T13:19:05Z

This is an attempt to address the use case reported in #445.

Rationale

There are many scenarios where one would want sequentialTestGroups (as in: don't run these tests in parallel) and still want the ability to filter them, particularly when working on a single test at the bottom of a long sequential group.
Furthermore, as answered by @Bodigrim, the current behaviour is not obvious from documentation (it is implied by the README in "Dependencies", in a list of caveat about patterns. Adding a dedicated utility for sequential groups that can be filtered make the intent more obvious. I would hazard that the crux of the matter is that two very different concerns were mixed:

should the test be run in parallel or not;
are there dependencies between tests;

One could want to have sequential tests in the absence of actual dependencies but simply, for instance, to avoid race conditions. Currently, one has to either use "after" or renounce the ability to filter.

Constraint

Though ideally the filtering behaviour should be controlled by a specific option and not through the Sequential / Parallel nature of the tests, I couldn't find a way to do it without breaking the current API which I think should be avoided at all costs. Hence the addition of a new constructor for ExecutionMode.

I'm not 100% sold on my own proposal, because (a) "filterableSequentialTestGroup" is certainly descriptive but it's a bit too long a name (b) it doesn't clearly separate two very different concerns (c) it leads to some annoying repetition in code (see the changes in Tast/Run.hs.

However, it works, changes are little, intent is clear, and it fixes an issue that was raised several times (see also #411).

Considering this issue is rather blocking for me currently, I'm more than willing to apply any requested changes or explore any other direction to have sequential tests be filterable.

Bodigrim · 2025-07-03T18:43:02Z

@martijnbastiaan may I ask you to review this as the original author of sequential test groups?

Bodigrim · 2025-07-03T18:48:59Z

@Raveline I'd like to wait a little bit hoping for someone to come up with a more elegant solutions, but that's not too bad. Luckily ExecutionMode is not exported, so this is not a breaking change.

If I forget about this PR, please ping me in a week from now.

martijnbastiaan · 2025-07-03T20:08:35Z

You're right, those two concerns did get mixed.

One could want to have sequential tests in the absence of actual dependencies but simply, for instance, to avoid race conditions.

I think the name nonParallelTestGroup or anyOrderTestGroup would probably be better in that case. This leaves the door open for a test order randomization to detect implicit/accidental test dependencies. The name filterableSequentialTestGroup implies that they really should run in order. What do you think @Bodigrim @Raveline?

martijnbastiaan · 2025-07-03T20:12:52Z

I wonder if this models what we want and sets us up for sensible naming?

data ExecutionMode = Dependent | NonDependent ExecutionOrder
data ExecutionOrder = NoOrder | AnyOrder | InOrder

-- testGroup          => NonDependent NoOrder
-- anyOrderTestGroup  => NonDependent AnyOrder
-- inOrderTestGroup   => NonDependent InOrder  (renamed from `filterableSequentialTestGroup`)
-- dependentTestGroup => Dependent             (renamed from `sequentialTestGroup`)

Name	Filterable	Parallelizable	Sequential
NonDependent NoOrder	X	X
NonDependent AnyOrder	X
NonDependent InOrder	X		X
Dependent			X

I'm not asking for this to happen in this PR, but I'm curious to hear your thoughts.

Raveline · 2025-07-05T06:15:48Z

@martijnbastiaan : I didn't think about this notion of order, but you are right, it is connected to the notion of filterable. If @Bodigrim is ok with the model you proposed, I'll be more than happy to try my hand at implementing it (minus the renaming of sequentialTestGroup, which I think we should keep as is for backwards compatibility reasons).

martijnbastiaan · 2025-07-05T09:12:47Z

Yeah, just blindly renaming existing functions is a no-go, agreed. I'd be in favor of deprecating it to prevent misunderstandings like #443, but in the end that's up to @Bodigrim. I'll get to reviewing the PR properly once we decide what to do.

Bodigrim · 2025-07-05T16:41:33Z

I'm fine with deprecating sequentialTestGroup and introducing dependentTestGroup instead.

Given that there is no anyOrderTestGroup at the moment, I'd use only two options data ExecutionOrder = NoSpecificOrder | SequentialOrder. Although I'm really bad with naming, so keen to hear what others think about it. CC @andreasabel @VictorCMiraldo.

andreasabel · 2025-07-07T06:08:40Z

@martijnbastiaan wrote:

data ExecutionOrder = NoOrder | AnyOrder | InOrder

A priori, I would not understand the difference between NoOrder and AnyOrder. Linguistically, "no (particular) order" and "any order" seem to say the same thing.
@Bodigrim wrote:

data ExecutionOrder = NoSpecificOrder | SequentialOrder

This is linguistically clear.

martijnbastiaan · 2025-07-07T11:34:38Z

Agreed, "no order" and "any order" are linguistically the same. My 2ct would be that I'd much rather have:

data ExecutionMode = Dependent | NonDependent Parallel
data Parallel = Parallel | NonParallel

As mentioned before, this leaves the door open for test order randomization. I doubt anyone really needs sequential execution for non-dependent test cases.

Bodigrim · 2025-07-07T18:43:17Z

Could "NonDependent" poissbly be "Independent"?..

Anyway, we are not going to expose these data types, so naming can be adjusted later. I'm roughly on board with the plan.

Bodigrim · 2025-07-08T21:16:38Z

One thing we need to spell out in the documentation: all the notion of sequencing / depending is just a hint to the default TestManager. Other test managers (such as tasty-rerun) may and (unfortunately) will ignore the additional semantics.

Raveline · 2025-07-11T07:26:35Z

Where would you put the comment explaining this, @Bodigrim ? On the documentation for Ingredients, or in the functions declaring test groups ?

Bodigrim · 2025-07-11T18:19:39Z

On the functions please.

Bodigrim · 2025-07-11T22:04:10Z

core/Test/Tasty/Core.hs

 -- order, all dependencies will be run (overriding filter).
 -- For parallel execution, see 'testGroup'.
 --
+-- Note that this is will only work when used with a `TestManager`. If you use


"with the default TestManager"
(everything is a TestManager, launchTestTree is the default one)

Bodigrim

@martijnbastiaan could you please review?

Bodigrim · 2025-07-12T17:55:19Z

core/Test/Tasty/Run.hs

-            sequence trees
-          Sequential depType ->
-            snd <$> mapAccumM (goSeqGroup depType) mempty trees
+          (Independent Parallel) -> sequence trees


Redundant brackets, I think.

Bodigrim · 2025-07-12T17:56:08Z

core/Test/Tasty/Core.hs

+--
+-- Note that this is will only work when used with the default `TestManager`.
+-- If you use another manager, like `tasty-rerun` for instance, sequentiality
+-- will be ignored.


"might be possibly ignored", please, here and below.

Bodigrim · 2025-07-12T17:57:01Z

core/Test/Tasty/Core.hs

 --
 -- @since 1.5
 sequentialTestGroup :: TestName -> DependencyType -> [TestTree] -> TestTree
 sequentialTestGroup nm depType = setSequential . TestGroup nm . map setParallel


So, are we going to deprecate this one and introduce a better name?

Well, I didn't want to do it in advance, not knowing if there was a deprecation strategy in place. Just in case, I've added deprecation (and proper renaming in tests) in my last commit.

martijnbastiaan

Thanks @Raveline!

Let's not forget to close ~~#443 and~~ #445 after.

martijnbastiaan · 2025-07-24T16:26:10Z

core/Test/Tasty/Core.hs

+-- | Create a named group of test cases or other groups. Tests are executed in
+-- order, all dependencies will be run (overriding filter).
+-- For parallel execution, see 'testGroup'.


Suggested change

-- | Create a named group of test cases or other groups. Tests are executed in

-- order, all dependencies will be run (overriding filter).

-- For parallel execution, see 'testGroup'.

-- | Create a named group of test cases or other groups. Tests are executed in

-- order and each test is considered a dependency of the next one. If a filter

-- is applied, any dependencies are run too, even if they would otherwise not

-- match the filter's criteria.

--

-- For parallel execution, see 'testGroup'. For ordered test execution, but

-- without dependencies, see 'inOrderTestGroup'.

This does not quite resolves #443, I think. What happens if one of sequential tests fails? Does it stop execution of subsequent ones?

I thought that was covered by DependencyType, but after reading it I'm still not really sure what's going on. Do you think this PR should be blocked on that? I'm happy to open a new PR tackling the problem after merging this one -- I feel it's a problem I introduced to begin with.

My understanding is that in a AllSucceed block, you will have the behaviour described by @Bodigrim (but not in an AllFinish one).

I'm not sure there is actually a use-case for this (but I might be lacking imagination !). I think users are more likely to need a "stop at the first failed test" (disregarding any dependency and / or order) flag for the whole runner, which I don't think Tasty offer and would be a neat addition.

Do you think this PR should be blocked on that? I'm happy to open a new PR tackling the problem after merging this one -- I feel it's a problem I introduced to begin with.

Sure, no problem.

I'm happy to open a new PR tackling the problem after merging this one -- I feel it's a problem I introduced to begin with.

@martijnbastiaan just a kind reminder.

It is on my stack, but the stack is big at the moment..

Improve documentation Co-authored-by: Martijn Bastiaan <[email protected]>

core/Test/Tasty/Core.hs

Fix comment Co-authored-by: ˌbodʲɪˈɡrʲim <[email protected]>

andreasabel

(I just had a brief look at the API and found nothing alienating.)

Bodigrim · 2025-08-03T23:15:38Z

Thanks @Raveline!

Raveline added 2 commits July 3, 2025 15:00

Add FilterableSequential behaviour

52e7ef7

Add test for filterableSequentialTestGroup

0787b80

Bodigrim reviewed Jul 11, 2025

View reviewed changes

Raveline force-pushed the add-filterable-sequential-test-groups branch from ac2a884 to a40a635 Compare July 12, 2025 05:52

Bodigrim reviewed Jul 12, 2025

View reviewed changes

Raveline added 3 commits July 15, 2025 09:47

Refactor execution mode to separate test order and parallelization

dab0ef7

Add comment on TestManager

8068be2

Deprecate sequentialTestGroup in favour of dependentTestGroup

e9c0d44

Raveline force-pushed the add-filterable-sequential-test-groups branch from a40a635 to e9c0d44 Compare July 15, 2025 07:47

martijnbastiaan approved these changes Jul 24, 2025

View reviewed changes

Update core/Test/Tasty/Core.hs

080f405

Improve documentation Co-authored-by: Martijn Bastiaan <[email protected]>

Bodigrim reviewed Jul 25, 2025

View reviewed changes

core/Test/Tasty/Core.hs Outdated Show resolved Hide resolved

Bodigrim requested review from VictorCMiraldo and andreasabel July 25, 2025 18:09

Update core/Test/Tasty/Core.hs

cadb9e1

Fix comment Co-authored-by: ˌbodʲɪˈɡrʲim <[email protected]>

andreasabel approved these changes Jul 26, 2025

View reviewed changes

Bodigrim merged commit 6cbcdcc into UnkindPartition:master Aug 3, 2025
14 checks passed

Raveline mentioned this pull request Aug 11, 2025

Using sequential test groups seems to break the ability to filter out specific tests #445

Closed

zlondrej mentioned this pull request Dec 22, 2025

Mutually exclusive test cases/groups #463

Open

CryptoPascal31 mentioned this pull request Apr 7, 2026

Fix build by updating Crypton deps + Tasty warning kda-community/chainweb-node#25

Merged

--- | Create a named group of test cases or other groups. Tests are executed in
--- order, all dependencies will be run (overriding filter).
--- For parallel execution, see 'testGroup'.
+-- | Create a named group of test cases or other groups. Tests are executed in
+-- order and each test is considered a dependency of the next one. If a filter
+-- is applied, any dependencies are run too, even if they would otherwise not
+-- match the filter's criteria.
+--
+-- For parallel execution, see 'testGroup'. For ordered test execution, but
+-- without dependencies, see 'inOrderTestGroup'.

Conversation

Raveline commented Jul 3, 2025

Rationale

Constraint

Uh oh!

Bodigrim commented Jul 3, 2025

Uh oh!

Bodigrim commented Jul 3, 2025

Uh oh!

martijnbastiaan commented Jul 3, 2025

Uh oh!

martijnbastiaan commented Jul 3, 2025

Uh oh!

Raveline commented Jul 5, 2025

Uh oh!

martijnbastiaan commented Jul 5, 2025

Uh oh!

Bodigrim commented Jul 5, 2025

Uh oh!

andreasabel commented Jul 7, 2025

Uh oh!

martijnbastiaan commented Jul 7, 2025

Uh oh!

Bodigrim commented Jul 7, 2025

Uh oh!

Bodigrim commented Jul 8, 2025

Uh oh!

Raveline commented Jul 11, 2025

Uh oh!

Bodigrim commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bodigrim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnbastiaan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreasabel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bodigrim commented Aug 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

martijnbastiaan left a comment •

edited

Loading