Skip to content

fix(test): the JUnit report drops every character XML forbids - #874

Open
thegoodengineer wants to merge 1 commit into
reticlehq:mainfrom
thegoodengineer:fix/junit-xml-char-range
Open

fix(test): the JUnit report drops every character XML forbids#874
thegoodengineer wants to merge 1 commit into
reticlehq:mainfrom
thegoodengineer:fix/junit-xml-char-range

Conversation

@thegoodengineer

Copy link
Copy Markdown
Contributor

What & why

stripXmlIllegal cites XML 1.0 section 2.2 and then enforces only the lower bound of its Char production:

#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]

That leaves the two upper holes open: the surrogate block, and the noncharacters U+FFFE and U+FFFF.

U+FFFF is the one that actually breaks a reader. A <testcase name="..."> carrying it is rejected outright by a real parser:

2:43: disallowed character.

which is precisely the outcome this function exists to prevent. Its own comment says an unparseable document means "CI shows nothing instead of showing the failure", and that is what happens: the run looks empty rather than red.

A lone surrogate is the milder half. Writing the file as UTF-8 turns it into U+FFFD, so it corrupts a character rather than killing the parse. I checked that rather than assuming, and I am not claiming it as a parse failure. It is illegal by the same clause though, and dropping it keeps the rule one thing rather than two.

No issue filed; found while reading packages/test.

How it was verified

packages/test/src/junit.char-range.test.ts, written RED first. Four cases fail on main (U+FFFF, U+FFFE, and a lone surrogate either side), and the rest pin what must not change:

  • tab, newline and carriage return survive
  • U+FFFD, the last legal BMP code point, survives
  • an astral emoji survives, including when written as an explicit surrogate pair, because iteration has already resolved it to one code point
  • the five XML-significant characters are still escaped
  • the surrounding text survives every strip: this is a strip, not a rejection

The existing junit.test.ts passes unchanged.

The check stays stated as codes rather than a regex character class, for the reason the original comment gives: a class covering control characters trips no-control-regex and needs the rule turned off to read.

Gates run

  • pnpm lint && pnpm typecheck && pnpm test:unit
  • pnpm test:e2e
  • pnpm gate:install
  • pnpm test:e2e:desktop
  • None of the above tiers apply to this change

One pure function in the CI reporter: no tool surface, no wire contract, no observer, nothing an install runs. format:check, build, lint and typecheck are green, and packages/test is 18/18 files passing.

Checklist

  • Every commit is signed off
  • Tests added (RED then GREEN)
  • No any, no free strings, no non-null !
  • No console.log or internal tracking codes
  • Each changed file is under the 1000-line cap
  • CHANGELOG.md updated under [Unreleased]
  • Not security-affecting

`stripXmlIllegal` cites XML 1.0 section 2.2 and then enforces only the lower
bound of its `Char` production:

    #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]

That leaves the two upper holes open: the surrogate block, and the
noncharacters U+FFFE and U+FFFF.

U+FFFF is the one that actually breaks a reader. A `<testcase name="...">`
carrying it is rejected by a real parser with "disallowed character", which is
precisely the outcome this function exists to prevent: the function's own
comment says an unparseable document means "CI shows nothing instead of showing
the failure". A lone surrogate is the milder half, since writing the file as
UTF-8 turns it into U+FFFD, but it is illegal by the same clause and dropping it
keeps the rule one thing rather than two.

The check is now the whole production, still stated as codes rather than a regex
character class for the reason the original comment gives. Astral characters,
U+FFFD itself, tabs, newlines and carriage returns are untouched, and a valid
surrogate PAIR is unaffected because iteration has already resolved it to one
astral code point.

Signed-off-by: Abhijeet Sharma <meetabhijeet05@gmail.com>
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.

1 participant