Repository navigation
TestClassContains - #2694
TestClassContains#2694
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2694 +/- ##
=======================================
Coverage 94.12% 94.12%
=======================================
Files 32 32
Lines 10053 10053
=======================================
Hits 9462 9462
Misses 421 421
Partials 170 170 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Good test addition — table-driven structure is clean and covers the core contract well (exact token matching, substring rejection, whitespace handling around spaces). One notable gap: |
| classAttr: "foo bar baz", | ||
| classVal: "qux", | ||
| want: false, | ||
| }, |
There was a problem hiding this comment.
Consider adding a few more edge cases to pin down the contract:
-
Tab/newline as separator —
classAttr: "foo\tbar"withclassVal: "bar". The currentClassContainsimpl only splits on space, so this returnsfalse. A test case would document whether that's intentional or a bug (HTML spec treats\t,\n,\r,\fas valid whitespace separators). -
classVal with embedded space —
classAttr: "foo bar baz",classVal: "foo bar"→ should befalse(class names can't contain spaces). -
All-whitespace classAttr —
classAttr: " ",classVal: "foo"→ should befalse.
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive test suite for the ClassContains function, covering basic matching, substring rejections, and whitespace handling. Review feedback recommends improving the specificity of suffix substring tests and expanding coverage to include additional HTML-compliant whitespace characters, case sensitivity, and invalid class values.
| { | ||
| name: "rejects suffix substring", | ||
| classAttr: "param-required required", | ||
| classVal: "required-extra", | ||
| want: false, | ||
| }, |
There was a problem hiding this comment.
The test case named "rejects suffix substring" currently checks a value that is longer than the class in the attribute (e.g., "required-extra" vs "required"). It would be more robust to also include a test for a true suffix substring (e.g., "class" in "my-class") and rename the existing one to something more descriptive like "rejects longer value".
| { | |
| name: "rejects suffix substring", | |
| classAttr: "param-required required", | |
| classVal: "required-extra", | |
| want: false, | |
| }, | |
| { | |
| name: "rejects suffix substring", | |
| classAttr: "my-class", | |
| classVal: "class", | |
| want: false, | |
| }, | |
| { | |
| name: "rejects longer value", | |
| classAttr: "param-required required", | |
| classVal: "required-extra", | |
| want: false, | |
| }, |
| classVal: "qux", | ||
| want: false, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Consider adding test cases for other HTML-compliant whitespace characters (tabs, newlines, form feeds, and carriage returns), case sensitivity, and classVal containing spaces. These are important edge cases for robust HTML class matching according to the specification.
{
name: "matches with other whitespace",
classAttr: "foo\tbar\nbaz\rfuz\fqux",
classVal: "baz",
want: true,
},
{
name: "case sensitive match",
classAttr: "Foo",
classVal: "foo",
want: false,
},
{
name: "rejects classVal with spaces",
classAttr: "foo bar baz",
classVal: "foo bar",
want: false,
},
}
No description provided.