NorfairKing / sydtest

A modern testing framework for Haskell with good defaults and advanced testing features.
113 stars 25 forks source link

Make test discovery work for nested Spec files #68

Closed L7R7 closed 1 year ago

L7R7 commented 1 year ago

This resolves the assumption that was mentioned in the code. The Spec.hs file with the pragma doesn't need to be a the top level. Instead, it will try to figure out the test base dir by traversing up the tree until it finds a directory that doesn't start with an uppercase letter. So the new assumption is: The base directory for the tests is the first parent that starts with a lower case name. I'm not sure if this assumption holds all the time and I don't know if there is a potential problem with infinite recursion.

I tested it locally with the following project: https://github.com/L7R7/haskell-incubator/tree/main/sydtest-playground/test. It shows that one can use several nested Spec files (even with a deeper nesting) and thus allows a combination of specs with and without outer resources

L7R7 commented 1 year ago

@NorfairKing ready for review, let me know what you think!

NorfairKing commented 1 year ago

Hi @L7R7 Thank you for your patience, I've been traveling. I have one small concern, and I'm actually not sure if this will break anything, but given that CI passes, I'm inclined to just press merge once my concern is addressed.

L7R7 commented 1 year ago

@NorfairKing no worries! I found a way to keep traversing on relative paths instead of absolute ones. It's not the most elegant solution, but it's the more efficient solution to traverse the correct directory and then append the prefix to the paths instead of traversing all files and then filter for the correct files.

Do you want me to add some documentation to the readme describing the use case with several discoveries in one test suite?

NorfairKing commented 1 year ago

Do you want me to add some documentation to the readme describing the use case with several discoveries in one test suite?

That would be great. Tests would be good too but that's probably expecting too much given that there were none ?

L7R7 commented 1 year ago

I'll add a section to the readme :+1:

regarding tests: I thought about that, but I couldn't come up with a good way to implement it. I'd like to test that it discovers the tests, but how do I make sure the test suite fails when discovery isn't working? The only part I definitely could test is that it doesn't include Specs that are valid, but shouldn't be included because they live in a different module than my Spec file (I would just put a test there that always fails, so if it is discovered, tests turn red)

NorfairKing commented 1 year ago

@L7R7 On the topic of tests: I think it would be good enough to have both usages (subdir or not) of sydtest-discover being used in some sydtest test suite.

L7R7 commented 1 year ago

Alright, sounds reasonable. I'll put something together!

L7R7 commented 1 year ago

@NorfairKing I added some docs and a test suite. I tried to come up with the most simple example that demonstrates the use case.

I don't know why the build is failing, but I think it's unrelated to my changes

NorfairKing commented 1 year ago

@L7R7 I just fixed CI, merged this manually, and released sydtest-discover. I also added you to the contributors file. Thank you!

L7R7 commented 1 year ago

Awesome!