UnkindPartition / tasty

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

Refactor `createTestActions` #364

Closed martijnbastiaan closed 1 year ago

martijnbastiaan commented 1 year ago

The refactor breaks the fold up in to two stages:

This removes an accidental quadratic complexity in favor of a linear one caused by continuously mapping over tests while building that list of tests. This removes the need for one layer in Tr's monad stack: WriterT. At the same time, it uses dedicated data structures instead of tuples for ease of reading.

This commit aims to make future extensions easier.


Note that we need:

Expose children in foldGroup

In order to build TGroup. The refactor alone doesn't warrant this API change of course, it is mainly used in https://github.com/UnkindPartition/tasty/pull/343.

Bodigrim commented 1 year ago

@andreasabel could you possibly review this please? I'm overcommitted at the moment. The breaking change to foldGroup is indeed justified by further development in #343.

martijnbastiaan commented 1 year ago

I could rework #343 to not rely on this refactor I suppose, but the stack of Traversal/Writer/Reader/IO () is making my head spin a little bit so it would take some time. Of course I'd prefer it if either you or @andreasabel would find the time to review it. But, no real hurry.

martijnbastiaan commented 1 year ago

Nicely done, I've been always too scary to look into Test.Tasty.Run.

Thanks! It took me a bit to figure out what made it tick. Hopefully the refactor helps future endeavors related to it.

Bodigrim commented 1 year ago

@andreasabel would you like to take a look?

Bodigrim commented 1 year ago

@andreasabel this is a tough and time-consuming thing to review, but I'm afraid to burn out the contributor with a long wait. If there are no suggestions from your side by May 1, I'll take a risk to go ahead with a merge.

Bodigrim commented 1 year ago

Thanks @martijnbastiaan!

andreasabel commented 1 year ago

@Bodigrim : I am afraid I don't really have an opinion on this PR, since I am not a power user of tasty. I am fine to admit a refactoring which is well motivated.