Skip to content

Add tests for apply#870

Merged
jeaye merged 7 commits intojank-lang:mainfrom
dgr:dgr-apply
Apr 22, 2026
Merged

Add tests for apply#870
jeaye merged 7 commits intojank-lang:mainfrom
dgr:dgr-apply

Conversation

@dgr
Copy link
Copy Markdown
Collaborator

@dgr dgr commented Apr 21, 2026

No description provided.

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 cross-dialect test namespace to exercise clojure.core/apply behavior within the existing clojure.core-test.* suite.

Changes:

  • Introduces clojure.core-test.apply with assertions covering apply on empty seqables, mixed fixed args + seq args, and IFn invocations (map/keyword/vector/set).
  • Uses the portability layer (when-var-exists, p/thrown?) to keep behavior consistent across supported dialects.

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

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

apply with an inequality function (e.g. <) could be helpful given the wonkiness we've caught with these prior and how different these functions operate than their operator equivalents in most PLs

@dgr
Copy link
Copy Markdown
Collaborator Author

dgr commented Apr 21, 2026

apply with an inequality function (e.g. <) could be helpful given the wonkiness we've caught with these prior and how different these functions operate than their operator equivalents in most PLs

What specifically are you trying to test? apply is pretty simple. It basically just validates that the first argument is an IFn and calls the correct arity with the supplied parameters. Given that, what do you think could go wrong with < that isn't verified by the test assertions in the current PR?

@jeaye
Copy link
Copy Markdown
Member

jeaye commented Apr 21, 2026

apply with an inequality function (e.g. <) could be helpful given the wonkiness we've caught with these prior and how different these functions operate than their operator equivalents in most PLs

What specifically are you trying to test? apply is pretty simple. It basically just validates that the first argument is an IFn and calls the correct arity with the supplied parameters. Given that, what do you think could go wrong with < that isn't verified by the test assertions in the current PR?

Thanks for the idea, Emma! I agree with Dave here that this one shouldn't be needed. Keep 'em coming.

@dgr
Copy link
Copy Markdown
Collaborator Author

dgr commented Apr 21, 2026

@jeaye , BTW, good idea to invoke the CoPilot review. I should do that more often. It caught a couple formatting things (trailing white space, comment on the wrong line, and a misspelling. I guess a little AI isn't a bad thing. :-)

@jeaye
Copy link
Copy Markdown
Member

jeaye commented Apr 21, 2026

@jeaye , BTW, good idea to invoke the CoPilot review. I should do that more often. It caught a couple formatting things (trailing white space, comment on the wrong line, and a misspelling. I guess a little AI isn't a bad thing. :-)

I'm glad you're not annoyed by it! I've been using it for my jank PRs and it actually catches a lot. So far, it's definitely been worth my while. It's free for the jank-lang org, so, as you said, we might as well.

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.

LGTM. Awaiting approval from Emma, as she may have more comments.

@dgr
Copy link
Copy Markdown
Collaborator Author

dgr commented Apr 21, 2026

OK, this should be ready now.

@dgr
Copy link
Copy Markdown
Collaborator Author

dgr commented Apr 21, 2026

BTW, @E-A-Griffin , there are some tests of apply with comparison functions in gt_eq.cljc and lt_eq.cljc, so it's not completely untested.

Copy link
Copy Markdown
Collaborator

@E-A-Griffin E-A-Griffin left a comment

Choose a reason for hiding this comment

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

Let's get this merged!

@jeaye jeaye merged commit eb8daa1 into jank-lang:main Apr 22, 2026
6 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.

5 participants