Skip to content

Add tests for hash-map, hash-set, and set #87

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

Merged
merged 2 commits into from
Jun 19, 2025

Conversation

rafonsecad
Copy link
Contributor

No description provided.

@NoahTheDuke
Copy link
Contributor

Maybe add nested calls? So (= {:a {:b {:c 1} :d 2}} (hash-map :a (hash-map :b (hash-map :c 1) :d 2))), etc.

I don't know how exactly to test it, but for example, you can't write #{(rand) (rand)} but you can write (hash-set (rand) (rand)), so maybe a test for that behavior?

[clojure.core-test.portability #?(:cljs :refer-macros :default :refer) [when-var-exists]]))

(when-var-exists clojure.core/set
(deftest test-set
Copy link
Member

@jeaye jeaye Jun 18, 2025

Choose a reason for hiding this comment

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

I think we should have a test which asserts that set removes duplicates.

Something like (set [1 2 3 1 2 3]).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeaye, I just added (is (= #{1 2 3} (set [1 1 2 2 3 3 3])))

@jeaye
Copy link
Member

jeaye commented Jun 18, 2025

Maybe add nested calls? So (= {:a {:b {:c 1} :d 2}} (hash-map :a (hash-map :b (hash-map :c 1) :d 2))), etc.

Sounds like a good thing to test.

I don't know how exactly to test it, but for example, you can't write #{(rand) (rand)} but you can write (hash-set (rand) (rand)), so maybe a test for that behavior?

We don't want to be using rand at all in these tests. Also, the lack of support for #{(foo) (foo)} is a reader feature, not a clojure.core feature. We don't need to test for it here.

@NoahTheDuke
Copy link
Contributor

(rand) wasn't the best example, I just meant that the reader can't handle two of the same calls in a set or map literal, whereas the functions can handle it. That's fine tho.

@rafonsecad
Copy link
Contributor Author

@NoahTheDuke, thanks for the comment, I just added (is (= {:a {:b {:c 1} :d 2}} (hash-map :a (hash-map :b (hash-map :c 1) :d 2))))

Copy link
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.

Great job! Thanks so much for helping with these.

@jeaye jeaye merged commit 434fc49 into jank-lang:main Jun 19, 2025
2 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