Skip to content

Add tests for repeatedly#878

Merged
jeaye merged 8 commits into
jank-lang:mainfrom
E-A-Griffin:egriffin/repeatedly
May 20, 2026
Merged

Add tests for repeatedly#878
jeaye merged 8 commits into
jank-lang:mainfrom
E-A-Griffin:egriffin/repeatedly

Conversation

@E-A-Griffin
Copy link
Copy Markdown
Collaborator

@E-A-Griffin E-A-Griffin commented May 13, 2026

Closes #456

@E-A-Griffin E-A-Griffin changed the title Repeatedly Add tests for repeatedly May 13, 2026
@jeaye jeaye requested review from Copilot and dgr May 13, 2026 00:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.repeatedly with tests for laziness, side effects, arity behavior, and error cases.
  • Gate the suite behind when-var-exists for portability across runtimes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Copy link
Copy Markdown
Member

@jeaye jeaye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help, Emma!

Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Copy link
Copy Markdown
Collaborator

@dgr dgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good.

Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Comment thread test/clojure/core_test/repeatedly.cljc Outdated
Comment thread test/clojure/core_test/repeatedly.cljc Outdated
@E-A-Griffin
Copy link
Copy Markdown
Collaborator Author

Hm, I just went to https://cljs.io/ and tried 1/3 and it gave 0.333333. I then cross-referenced that with the differences from Clojure doc which says Ratio isn't supported.

So I used this to see that CLJS supports reading ratios, but doesn't have a ratio object and instead just reads them into floats.

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

@jeaye
Copy link
Copy Markdown
Member

jeaye commented May 15, 2026

Hm, I just went to https://cljs.io/ and tried 1/3 and it gave 0.333333. I then cross-referenced that with the differences from Clojure doc which says Ratio isn't supported.
So I used this to see that CLJS supports reading ratios, but doesn't have a ratio object and instead just reads them into floats.

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.

@dgr
Copy link
Copy Markdown
Collaborator

dgr commented May 15, 2026

Hm, I just went to https://cljs.io/ and tried 1/3 and it gave 0.333333. I then cross-referenced that with the differences from Clojure doc which says Ratio isn't supported.
So I used this to see that CLJS supports reading ratios, but doesn't have a ratio object and instead just reads them into floats.

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.

Comment thread test/clojure/core_test/repeatedly.cljc Outdated
@E-A-Griffin E-A-Griffin requested review from dgr and jeaye May 20, 2026 04:15
Copy link
Copy Markdown
Member

@jeaye jeaye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, Emma!

@jeaye jeaye merged commit db61a31 into jank-lang:main May 20, 2026
5 checks passed
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.

clojure.core/repeatedly

4 participants