mocha icon indicating copy to clipboard operation
mocha copied to clipboard

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

Open malloryavvir opened this issue 2 years ago • 1 comments

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.

malloryavvir avatar Jul 21 '23 19:07 malloryavvir

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: malloryavvir / name: Mallory Adams (c95387307288862095d8cc431fbc70f1d39ca1f6)

👋 ping @malloryavvir, is this still something you have time for?

JoshuaKGoldberg avatar Jul 02 '24 17:07 JoshuaKGoldberg

@JoshuaKGoldberg I've been pretty busy lately. I'm afraid I don't have the capacity to look at this. Thanks for reaching out though.

malloryavvir avatar Jul 02 '24 17:07 malloryavvir

No worries, thanks for letting us know! I'll close this out so that anybody who has time + energy can approach it.

If a PR is sent based on code form this one, it should include a co-authored-by attribution.

Cheers!

JoshuaKGoldberg avatar Jul 02 '24 18:07 JoshuaKGoldberg