Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure that the xunit reporter emits well-formed XML #5001

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malloryavvir
Copy link

@malloryavvir malloryavvir commented Jul 21, 2023

Edit: I have updated this PR to fix the bug in addition to creating a failing test.

Description of the Change

From the PR:

XML does not allow certain characters to appear in documents. This
change ensures that those restricted characters are replaced by legal
characters.

This bug means that when a test ends up containing, for example, ANSI
color codes (i.e. `[36m<div[39m`) then the generated
XML file is invalid and some consumers (such as CircleCI) are unable to
parse the contents.

Link: https://www.w3.org/TR/2006/REC-xml11-20060816/#charsets

Alternate Designs

This is a lossy conversion. An alternative to displaying the replacement character would be to display some message explaining what character has been replaced. This is a lot less verbose and will be easier to read when your tests are outputting ANSI control codes, and if the original text is needed you can rerun the tests locally.

Why should this be in core?

This is a bug. Mocha is generating malformed XML documents that cannot be parsed.

Benefits

XML documents generated by the XUnit reporter are now well-formed and thus can be parsed.

Possible Drawbacks

It's a lossy conversion. Probably 99% of the time it's going to be an ANSI control code being removed.

Applicable issues

This is a bug fix.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 21, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: malloryavvir / name: Mallory Adams (c953873)

XML does not allow certain characters to appear in documents. This
change ensures that those restricted characters are replaced by legal
characters.

This bug means that when a test ends up containing, for example, ANSI
color codes (i.e. `[36m<div[39m`) then the generated
XML file is invalid and some consumers (such as CircleCI) are unable to
parse the contents.

Link: https://www.w3.org/TR/2006/REC-xml11-20060816/#charsets
@malloryavvir malloryavvir changed the title Add a test to ensure that valid XML is produced by the xunit reporter Ensure that the xunit reporter emits well-formed XML Jul 25, 2023
@malloryavvir malloryavvir marked this pull request as ready for review July 25, 2023 18:43
@@ -35,6 +36,7 @@ exports.inherits = util.inherits;
* @return {string}
*/
exports.escape = function (html) {
html = html.toString().replaceAll(RESTRICTED_CHAR, "�");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 this looks like a duplicate of #4527. @malloryavvir do you have thoughts on https://github.com/mochajs/mocha/pull/4527/files#r1511131215 ?

@JoshuaKGoldberg JoshuaKGoldberg added status: waiting for author waiting on response from OP - more information needed semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Mar 4, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title Ensure that the xunit reporter emits well-formed XML fix: ensure that the xunit reporter emits well-formed XML Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch implementation requires increase of "patch" version number; "bug fixes" status: waiting for author waiting on response from OP - more information needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants