Closed trptcolin closed 2 years ago
This looks like it would be nice to have. Any reason not to merge this?
Thanks @trptcolin!
@trptcolin - I hate to be the bearer of bad news, but unfortunately, merging this PR hampered our ability to run specs in ClojureScript (lein cljs
in this project). The high-level descriptions were being found (and reported) but none of their contained characteristics or nested contexts were being executed. (We really should have all the specs running in a test environment via github actions to catch these sorts of problems before ever merging...)
So, we've manually reverted the changes submitted in this PR. You can see the referenced commit (above) for more of the gory details. We're not opposed to this functionality, but it needs to work in a ClojureScript environment as well as the Java runtime environment.
You are welcome to submit a new PR if you so desire!
Oh no, sorry to hear! I actually have no memory of this to be honest, glad to hear you caught the issue quickly.
I don’t do a lot of clj/cljs these days so happy for someone else to take a look. I re-opened the original issue (#163) in the meantime (and of course feel free to re-close it if I’m misunderstanding!)
Before this commit, inside a
describe
speclj would only install the component returned by each inner expression. This moves installation to the place where components are created, so that test code like this works as expected and runs 2 tests instead of just 1:This has always been a bit of a stumbling block, and while we'd normally use an
around
or similar to contextualize code, there's no reason I can see that this shouldn't just work.refs #163