Add tests for with-meta - #953
Conversation
|
@jeaye for your consideration (can't mark you as reviewer, sorry for the noise.) |
jeaye
left a comment
jeaye
left a comment
There was a problem hiding this comment.
Hi! Thanks for the PR. In our other tests, we've been more exhaustive about supported types and I think that would be useful here, too.
| {:a 0} {} {:a 0} | ||
| {:a 0} ^:b {} {:a 0} ;; with-meta overrides existing metadata |
There was a problem hiding this comment.
There are a lot of types which support meta aside from just array maps. We should enumerate these, since the ones which support it should be consistent across dialects. For example, namespaces, vars, functions, symbols, vectors, lists, sets, hash maps, and various sequences.
@dgr What do you think about being exhaustive here?
There was a problem hiding this comment.
Forgive me for a drive-by comment. But I think issue #921 is also of interest
There was a problem hiding this comment.
Pull request overview
Adds a new core test file covering clojure.core/with-meta behavior across supported dialects (guarded by when-var-exists), including basic metadata application and an edge-case around non-map metadata values.
Changes:
- Introduces
test/clojure/core_test/with_meta.cljcwithwith-metatest cases for setting/overriding metadata. - Adds a dialect-conditional expectation for the “non-map metadata” edge case (CLJS vs other runtimes).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| [clojure.core-test.portability #?(:cljs :refer-macros :default :refer) [when-var-exists] :as p])) | ||
|
|
||
| (when-var-exists with-meta | ||
| (deftest test-with_meta |
Closes #580
I locally ran tests for:
I failed to install the other runtimes :/