Html testing utils - #79
Conversation
📝 WalkthroughWalkthroughRefactors HTML utilities: extracts hiccup inspection/text and option-matching into Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RoleModule as rapid-test.html.role
participant Utils as rapid-test.html.utils
participant HZip as rapid-test.hiccup-zipper
Caller->>RoleModule: get-by-role(role, hiccup, opts?)
RoleModule->>HZip: hiccup-zipper(hiccup)
HZip-->>RoleModule: zipper stream
loop each node
RoleModule->>Utils: get-base-tag / get-attribute / get-accessible-name
Utils-->>RoleModule: properties, text, name
RoleModule->>RoleModule: role-match?(role, node, props)
alt matches and opts pass
RoleModule-->>Caller: return node (or collect for get-all-by-role)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
f6046bf to
fe9a1be
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/rapid_test/html/role.clj`:
- Around line 135-142: The role-match? method for :combobox currently calls
(long size) which will ClassCastException on string attributes; change the size
coercion to use parse-long like get-heading-level does: retrieve size via
util/get-attribute, call parse-long (or a safe parse wrapper used elsewhere)
only when size is present, bind the parsed value (e.g. size-n) and then use (or
(nil? size) (<= size-n 1)) in the existing predicate; update the role-match?
implementation accordingly so strings like "2" are parsed safely.
In `@src/main/rapid_test/html/utils.clj`:
- Around line 21-44: The transient `props` mutations discard assoc! return
values, violating the transient contract; fix by removing transients and using
immutable assoc calls (replace (transient base-props) with base-props and
replace assoc! calls at the :class and :id sites with assoc so the final result
reflects those changes), or alternatively capture the assoc! return values into
the `props` binding (e.g. rebind props from assoc! results before calling
persistent!); update the code around props, assoc!/assoc, persistent!, and the
bindings for class-sb and id to ensure the mutated map is the one returned.
🧹 Nitpick comments (4)
src/main/rapid_test/html/utils.clj (1)
78-90:get-textonnilinput relies onhiccup-zipperacceptingnil— worth a guard.The test suite calls
(get-text nil)and expects"". This works only ifhiccup-zipperdoesn't throw onnil. A nil-check at the entry point would make the contract explicit and prevent a subtle breakage ifhiccup-zipperever tightens its preconditions.Proposed guard
(defn get-text "Get the (nested) text of a hiccup node." ([hiccup] - (.toString (get-text (StringBuilder.) (hiccup-zipper hiccup)))) + (if (nil? hiccup) + "" + (.toString (get-text (StringBuilder.) (hiccup-zipper hiccup)))))src/main/rapid_test/html/role.clj (2)
186-200: Remove REPL scratchcommentblock before merging.This contains mutable
defforms that shadow each other and serve no documentation purpose. Consider removing or converting to a proper docstring/test.
19-28::listitemdoesn't supportexplicit-role?fallback, unlike other roles.Every other role method falls back to checking
(explicit-role? hiccup :listitem), but this one only checks the tag + parent structure. If this is intentional per ARIA spec, a brief comment would help. Otherwise, add the fallback for consistency.test/main/rapid_test/html/utils_test.clj (1)
1-50: Good foundational tests. Consider adding coverage forget-accessible-name,get-heading-level,matches-options?, andinside-sectioning-content?.These utility functions have non-trivial logic (aria-label precedence, aria-level parsing, regex matching, zipper traversal) that would benefit from direct unit tests, even though they're exercised indirectly through
html_test.clj.
d8cd79a to
1b6f285
Compare
1b6f285 to
944f855
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/main/rapid_test/html_test.clj`:
- Around line 92-98: The test currently asserts that [:input {:type "email"}] is
not a :textbox, but per HTML-AAM and Testing Library it should be treated as a
textbox; update the test expectation and the role-detection logic so
sut/get-by-role (and whatever role mapping function in the sut namespace that
classifies inputs by type) treats input elements with type "email" (and no list
attribute) as matching :textbox; modify the code path that checks input types
(e.g., the function or map that handles "text", "search", "tel", "url", "email",
etc.) to include "email", and change the test line to expect some? instead of
nil? for [:input {:type "email"}].
| (testing "textbox" | ||
| (is (some? (sut/get-by-role :textbox [:div [:textarea]]))) | ||
| (is (some? (sut/get-by-role :textbox | ||
| [:div [:input {:type "text"}]]))) | ||
| (is (some? (sut/get-by-role :textbox [:div [:input]]))) | ||
| (is (nil? (sut/get-by-role :textbox | ||
| [:div [:input {:type "email"}]])))) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HTML-AAM specification input type email textbox role mapping
💡 Result:
Per the HTML Accessibility API Mappings (HTML-AAM) spec, an <input type="email"> is mapped to the ARIA textbox role (i.e., it is exposed to accessibility APIs as a text input). It may also carry an object attribute indicating its specific text-input type (e.g., text-input-type:email in IA2/MSAA mappings, and UIA localized control type “email”). [1], [2]
If the email input uses a suggestions source (i.e., it has a list attribute pointing to a <datalist>), then the computed ARIA role is combobox (with the relationship to the suggestion list exposed via aria-controls/list). [1]
Sources
[1] HTML-AAM Working Draft (2024-02-26) – mappings for email state and for inputs with suggestions source element. (w3.org)
[2] HTML-AAM Working Draft (2021-08-10) – email state mapping to textbox and text-input-type:email. (w3.org)
Citations:
- 1: https://www.w3.org/TR/2024/WD-html-aam-1.0-20240226/?utm_source=openai
- 2: https://www.w3.org/TR/2021/WD-html-aam-1.0-20210810/?utm_source=openai
🌐 Web query:
Testing Library getByRole textbox input type email
💡 Result:
<input type="email"> has the implicit ARIA role textbox (when it has no list attribute), so you query it like any other textbox and (ideally) filter by its accessible name (usually the <label> text). [1][2]
const email = screen.getByRole('textbox', { name: /email/i })If you don’t have a proper label, add one (or use aria-label / aria-labelledby) so the name option works as expected. [1][3]
input[type="email"] should match :textbox per HTML-AAM spec and Testing Library.
Lines 97-98 assert that [:input {:type "email"}] does not match :textbox, but per the HTML-AAM spec, <input type="email"> (without a list attribute) has the implicit ARIA role textbox. Testing Library's getByRole('textbox') also matches email inputs. Update this test and the underlying implementation to include email inputs in the :textbox role.
🤖 Prompt for AI Agents
In `@test/main/rapid_test/html_test.clj` around lines 92 - 98, The test currently
asserts that [:input {:type "email"}] is not a :textbox, but per HTML-AAM and
Testing Library it should be treated as a textbox; update the test expectation
and the role-detection logic so sut/get-by-role (and whatever role mapping
function in the sut namespace that classifies inputs by type) treats input
elements with type "email" (and no list attribute) as matching :textbox; modify
the code path that checks input types (e.g., the function or map that handles
"text", "search", "tel", "url", "email", etc.) to include "email", and change
the test line to expect some? instead of nil? for [:input {:type "email"}].
1 participant
Add the rest of the get-by-role testing utils
Summary by CodeRabbit
New Features
Refactor
Tests