Add tests for repeatedly#878
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new core test namespace to cover clojure.core/repeatedly behavior across supported dialects, addressing Issue #456.
Changes:
- Introduce
clojure.core-test.repeatedlywith tests for laziness, side effects, arity behavior, and error cases. - Gate the suite behind
when-var-existsfor portability across runtimes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (catch #?(:cljs :default | ||
| :default Exception) _ nil)) |
There was a problem hiding this comment.
This isn't going to work for some dialects, like jank. We have p/thrown? to handle exception details. Perhaps we can rework this test to use p/thrown?. If not, I think we should add a (p/exception) or similar, to get the base exception type for any given dialect.
There was a problem hiding this comment.
Yeah I think we should probably implement p/exception anyways, and I can make and take a cleanup issue for it
There was a problem hiding this comment.
Yea, exceptions are difficult to deal with in Clojure dialects. This is one area where dialects should maybe try to come together, otherwise portable code is pretty much impossible. You basically just have to avoid anything throwing.
| (catch #?(:cljs :default | ||
| :default Exception) _ nil)) |
There was a problem hiding this comment.
Yea, exceptions are difficult to deal with in Clojure dialects. This is one area where dialects should maybe try to come together, otherwise portable code is pretty much impossible. You basically just have to avoid anything throwing.
| (testing "Single argument" | ||
| (is (= 0 (first (repeatedly +))))) |
There was a problem hiding this comment.
You might want to test the remainder of the seq for laziness.
There was a problem hiding this comment.
How would you recommend doing this?
There was a problem hiding this comment.
I think you generate an infinite seq ((range)) and then realize the first element and test that. If it doesn't throw, you're good to go. If it realizes the whole thing, then you'll get an OOM exception.
When I ran it on the test suite locally I got an exception so I think the local CLJS we're using doesn't support it |
Okie dokie! Let's not worry about it for now then. |
Maybe shoot a quick Slack message out to #cljs-dev. |
Closes #456