Create first html testing utils - #78
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds two new Clojure modules: a hiccup zipper for traversing and rebuilding hiccup element trees, and HTML testing utilities for props/attribute extraction, role-based queries, and text extraction. Also adds unit tests covering both modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant SUT as rapid-test.html
participant Zipper as rapid-test.hiccup-zipper
participant Role as role-match? (multimethod)
participant Tree as Hiccup Tree
Test->>SUT: call get-by-role(Tree, role)
SUT->>Zipper: hiccup-zipper(Tree)
SUT->>Zipper: traverse nodes (depth-first)
loop per node
Zipper->>SUT: current node
SUT->>Role: role-match?(role, node)
alt match
Role-->>SUT: true
SUT-->>Test: return node
else no match
Role-->>SUT: false
SUT->>Zipper: continue traversal
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/main/rapid_test/html.clj`:
- Around line 78-86: The role-match? method for :listitem can NPE when hzip is
the root because (zip/up hzip) may be nil; update role-match? to safely handle a
missing parent by checking the result of (zip/up hzip) (e.g., via when-let or
let binding) before calling (zip/node ...) and return false if there is no
parent; adjust references inside role-match? (function name role-match?, local
tag and parent-el) so parent-el is only computed when (zip/up hzip) is non-nil.
- Around line 88-93: The :default implementation of the multimethod role-match?
currently returns an ex-info object (in m/defmethod role-match? :default), which
is truthy so unsupported roles silently match; change it to throw the exception
instead of returning it by wrapping the ex-info call with throw (i.e., use
(throw (ex-info ...))) so unsupported roles raise an error when encountered
(this affects role-match? and callers like get-by-role).
- Around line 24-35: The case in the loop parsing tag-props-seq can throw
IllegalArgumentException when marker-char is neither "#" nor "." (e.g., stray
tokens from partitioning); update the loop handling in the function that builds
id and class-sb (the loop binding tag-props-seq and destructuring [marker-char
value & rst]) to include a default branch in the case that simply skips the
unexpected token and recurs (or otherwise advance the seq), ensuring marker-char
mismatches don’t crash the parser.
In `@test/main/rapid_test/hiccup_zipper_test.clj`:
- Line 13: Update the test description string in the testing call that currently
reads "List encosed children from `for` loop" to correct the typo to "List
enclosed children from `for` loop"; locate the testing form in
hiccup_zipper_test.clj (the (testing ...) invocation) and change the word
"encosed" to "enclosed" so the test description is spelled correctly.
- Around line 19-21: The "works fine with strings" testing block is empty due to
a misplaced closing parenthesis; move the assertion into that block by removing
the stray closing paren so the (testing "getting children" ...) assertion (which
uses zip/node, zip/next and sut/hiccup-zipper) is nested under the "works fine
with strings" label (or merge the two testing descriptions into one) so the
string-child assertion executes in the intended test context.
In `@test/main/rapid_test/html_test.clj`:
- Around line 6-8: The tests use non-standard hiccup selectors like
[:div.#my-id.class] which should be [:div#my-id.class]; update the test vectors
in test/main/rapid_test/html_test.clj to use standard selector syntax (e.g.,
replace [:div.#my-id.class] and [:div.#my-id] with [:div#my-id.class] and
[:div#my-id]) so sut/get-base-tag and sut/get-props are exercised with valid
input, and if you intentionally want to cover malformed selectors add a separate
test with a clear comment and/or add a safe default branch to get-props to
handle unexpected tokens.
🧹 Nitpick comments (3)
src/main/rapid_test/hiccup_zipper.clj (1)
14-37:childrenhandlesmorewrapping in a way that could be fragile.The initial work queue
(list maybe-props more)works becausemorefrom& moredestructuring is always a seq (or nil), so it gets flattened in theseq?branch. This is correct but subtle — a brief comment explaining whymoreis passed as-is (relying on it being a seq that gets spliced) would help future readers.src/main/rapid_test/html.clj (2)
13-46: Mixingatom,transient, andStringBuilderadds unnecessary mutability and complexity.
get-propsuses three mutable constructs (anatomfor id, aStringBuilderfor class, and atransientmap) where a simplereduce/loopaccumulating a persistent map would be clearer and more idiomatic Clojure. This also makes theassoc!/persistent!calls on lines 38–46 happen outside the loop but still mutate the transient, which is valid but easy to get wrong in future edits.
119-133: Consider removing thecommentblock or moving it to a scratch/REPL file.Rich comment blocks are handy during development but these
defforms rebindhiccupmultiple times, which could confuse readers. A brief note that this is a REPL scratch area would help, or it could be removed before merge.
| (loop [tag-props-seq (rest (map #(apply str %) | ||
| (partition-by #{\. \#} tag-str)))] | ||
| (when-not (empty? tag-props-seq) | ||
| (let [[marker-char value & rst] tag-props-seq] | ||
| (case marker-char | ||
| "#" (do (reset! id | ||
| (keyword value)) | ||
| (recur rst)) | ||
| "." (do (.append class-sb | ||
| (format " %s" | ||
| value)) | ||
| (recur rst)))))) |
There was a problem hiding this comment.
case without a default will throw IllegalArgumentException on unexpected marker values.
partition-by #{\. \#} splits . and # into separate groups, so a tag like :div.#my-id produces the sequence ("." "#" "my-id"). The destructuring [marker-char value & rst] binds marker-char=".", value="#", and on the next recurrence marker-char="my-id" — which matches neither "#" nor ".", causing an unhandled case crash.
Add a default clause (e.g., skip and recur) or switch to a parsing approach that doesn't rely on paired marker/value assumptions.
Minimal fix: add a default clause to skip unknown tokens
(case marker-char
"#" (do (reset! id
(keyword value))
(recur rst))
"." (do (.append class-sb
(format " %s"
value))
- (recur rst))))))
+ (recur rst))
+ (recur (cons value rst)))))))🤖 Prompt for AI Agents
In `@src/main/rapid_test/html.clj` around lines 24 - 35, The case in the loop
parsing tag-props-seq can throw IllegalArgumentException when marker-char is
neither "#" nor "." (e.g., stray tokens from partitioning); update the loop
handling in the function that builds id and class-sb (the loop binding
tag-props-seq and destructuring [marker-char value & rst]) to include a default
branch in the case that simply skips the unexpected token and recurs (or
otherwise advance the seq), ensuring marker-char mismatches don’t crash the
parser.
| (testing "works fine with strings") | ||
| (testing "getting children" | ||
| (is (= "error" (zip/node (zip/next (sut/hiccup-zipper [:span "error"]))))))) |
There was a problem hiding this comment.
Empty testing block — "works fine with strings" has no body.
The closing paren on line 19 ends the testing form immediately, making it a no-op. The string-child assertion on lines 20–21 lives under a separate "getting children" label instead. This looks like a misplaced parenthesis.
Proposed fix: merge the two blocks
- (testing "works fine with strings")
- (testing "getting children"
- (is (= "error" (zip/node (zip/next (sut/hiccup-zipper [:span "error"])))))))
+ (testing "works fine with strings"
+ (is (= "error" (zip/node (zip/next (sut/hiccup-zipper [:span "error"])))))))🤖 Prompt for AI Agents
In `@test/main/rapid_test/hiccup_zipper_test.clj` around lines 19 - 21, The "works
fine with strings" testing block is empty due to a misplaced closing
parenthesis; move the assertion into that block by removing the stray closing
paren so the (testing "getting children" ...) assertion (which uses zip/node,
zip/next and sut/hiccup-zipper) is nested under the "works fine with strings"
label (or merge the two testing descriptions into one) so the string-child
assertion executes in the intended test context.
| (is (= :div (sut/get-base-tag [:div.class]))) | ||
| (is (= :div (sut/get-base-tag [:div.#my-id.class]))) | ||
| (is (= :div (sut/get-base-tag [:div.#my-id])))) |
There was a problem hiding this comment.
The .# compound-tag syntax in these test cases is non-standard hiccup.
:div.#my-id.class places # after ., which isn't conventional hiccup (:div#my-id.class is). These tests pass for get-base-tag because it only reads up to the first ./#, but calling get-props on [:div.#my-id.class] would likely fail since the case in get-props has no default branch (see html.clj line 28). If the intent is to test robustness against malformed selectors, consider adding a comment. If not, fix to standard syntax.
🤖 Prompt for AI Agents
In `@test/main/rapid_test/html_test.clj` around lines 6 - 8, The tests use
non-standard hiccup selectors like [:div.#my-id.class] which should be
[:div#my-id.class]; update the test vectors in
test/main/rapid_test/html_test.clj to use standard selector syntax (e.g.,
replace [:div.#my-id.class] and [:div.#my-id] with [:div#my-id.class] and
[:div#my-id]) so sut/get-base-tag and sut/get-props are exercised with valid
input, and if you intentionally want to cover malformed selectors add a separate
test with a clear comment and/or add a safe default branch to get-props to
handle unexpected tokens.
61782ef to
afad0c7
Compare
afad0c7 to
1843194
Compare
1 participant
Adds some html testing utils a la Testing Library.
These are not at all comprehensive and need a lot more work.
The structure is about what I want.
Summary by CodeRabbit
New Features
Tests