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

Install all components #164

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Install all components #164

merged 1 commit into from
Mar 4, 2022

Conversation

trptcolin
Copy link
Collaborator

@trptcolin trptcolin commented Jun 5, 2017

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:

(let [x 1]
  (it "works 1" (= x 1))
  (it "works 2" (= x 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

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:

(let [x 1]
  (it "works 1" (= x 1))
  (it "works 2" (= x 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
@teaforthecat
Copy link

This looks like it would be nice to have. Any reason not to merge this?

@mdwhatcott
Copy link
Collaborator

Thanks @trptcolin!

@mdwhatcott mdwhatcott deleted the install-all-components branch March 4, 2022 17:25
mdwhatcott added a commit to mdw-cc/speclj that referenced this pull request Mar 19, 2022
Unfortunately, this commit hampered running specs in ClojureScript
(`lein cljs`). The high-level descriptions were being found (and
reported) but none of their contained characteristics or nested contexts
were executed.

As a result of this revert, the new 'focus'-related features are
somewhat hampered. Specifically, a context that is nested within a
focused context is not being executed as expected. I hope to resolve
this in a subsequent commit.
@mdwhatcott
Copy link
Collaborator

@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!

@trptcolin
Copy link
Collaborator Author

trptcolin commented Mar 19, 2022

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!)

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

Successfully merging this pull request may close these issues.

3 participants