Skip to content

Add clojure.string-test.split - #960

Merged
jeaye merged 5 commits into
jank-lang:mainfrom
djblue:split
Oct 26, 2026
Merged

jeaye merged 5 commits into
jank-lang:mainfrom
djblue:split

Conversation

@djblue

@djblue djblue commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

I'm looking for feedback one how we want to test split. Also, I have a feeling that edge cases will be dominated by implementation defined behavior, so do we want to capture that as well?

@jeaye
jeaye requested review from dgr and a balanced review from Copilot October 19, 2026 18:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Two ClojureScript expectations incorrectly include leading empty strings and would fail.

1 open issue
What changed in this PR

Adds clojure.string/split coverage to the cross-dialect test suite.

Changes:

  • Tests basic splitting, limit handling, and whitespace.
  • Records dialect-specific empty-pattern behavior.
  • Adds examples from ClojureDocs.
File Description
test/​clojure/​string_test/​split.cljc Adds guarded split tests with shared and dialect-specific expectations.

Review effort: Balanced

Comment thread test/clojure/string_test/split.cljc
@jeaye

jeaye commented Oct 19, 2026

Copy link
Copy Markdown
Member

This is already great coverage. The most important thing is to test what's covered by the doc string.

Splits string on a regular expression. Optional argument limit is
the maximum number of parts. Not lazy. Returns vector of the parts.
Trailing empty strings are not returned - pass limit of -1 to return all.

I don't think we should worry too much about all of the implementation details. We just want to provide a good coverage of the essential success cases and essential failure cases.

@djblue
djblue marked this pull request as ready for review October 26, 2026 00:31
@djblue

djblue commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

I think we have enough coverage to satisfy the docstring.

Comment thread test/clojure/string_test/split.cljc Outdated
@djblue
djblue requested a review from jeaye October 26, 2026 19:28

@jeaye jeaye left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work!

@jeaye
jeaye merged commit a920de9 into jank-lang:main Oct 26, 2026
4 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.

3 participants