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

Remove Expectations defexpect implicit expect support? #15

Open
seancorfield opened this issue Dec 12, 2024 · 4 comments
Open

Remove Expectations defexpect implicit expect support? #15

seancorfield opened this issue Dec 12, 2024 · 4 comments

Comments

@seancorfield
Copy link

I think this was a mistake in Expectations:

(defexpect name expected actual) is a special case shorthand for (defexpect name (expect expected actual)) provided as an easy way to migrate legacy Expectation tests to the 'lazytest.core' compatibility version.

I think you should not support it in LazyTest.

You only recently added defexpect so I doubt anyone is relying on this behavior (I'm actively removing it from the "legacy" tests at work, and none of the nses I've converted to LazyTest rely on this).

@NoahTheDuke
Copy link
Owner

Do you want defexpect or expecting at all?

@seancorfield
Copy link
Author

Yes, I think they're valuable as a migration tool -- from the clojure.test Expectations -- but in the next Expectations release, the implicit expect will be deprecated and generate a warning, and in the release after that it will throw an exception.

You could make it fail to force folks to correct it, but I suspect the universe of Expectations users that a) use the implicit form and b) are migrating to LazyTest is extremely close to empty (i.e., it's just me).

Do you have a diagnostic if a test suite has no tests? That would catch it, I think?

@NoahTheDuke
Copy link
Owner

Throwing a warning or a test failure if a suite has no children would be a good solution, good idea.

@seancorfield
Copy link
Author

And to be clear, the only reason (defexpect test-name expected actual) exists at all is that Classic Expectations used unnamed top-level forms like (expect expected actual) and auto-generated a name behind the scene. I thought it would somehow be less work for folks to replace expect with defexpect some-test-name rather than wrapping the original expect with (defexpect some-test-name ... ) (or wrapping a whole block of expect forms in a single defexpect with a name -- which was actually the approach we took with most of our namespaces at work, TBH).

Anyone migrating from clojure.test-compatible Expectations to LazyTest is probably going to want to add expecting (or it) or switch defexpect to defdescribe and add a suite docstring, in order to benefit from LazyTest's improved output/reporting.

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

No branches or pull requests

2 participants