Skip to content

Conversation

pascalwolterink
Copy link

Hi,

First of all we love your work! We are using your framework for some time now and found a problem with the xml results.

Assert Messages and TestNames are not properly escaped in the xunit_testresults.xml

If you would add a " character in either the Assert Message or TestName the xunit_testresults.xml cannot be properly parsed by xml parsers. To fix this problem I've implemented a F_XmlEscapeString function. For now I followed the 'safe' approach as suggested in this link https://stackoverflow.com/a/1091953.

Sample test code that will create the problem in the xml file:

TEST('Test_EscapedFailedMessage');
AssertTrue(FALSE, 'This "string" <should> be $'escaped$' & parsed properly');
TEST_FINISHED();

TEST('Test_EscapedFunctionName$'"<>&');
TEST_FINISHED();

These tests result in:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites disabled="" failures="1" tests="2" time="8.38e-5">
	<testsuite id="26" name="PRG_TEST.TestXUnitPublisher" tests="2" failures="1" time="7.47e-5">
		<testcase name="Test_EscapedFailedMessage" classname="PRG_TEST.TestXUnitPublisher" time="6.15e-5" status="FAIL">
			<failure message="This " string"
			<should> be 'escaped' & parsed properly" type="BOOL"/></testcase>
			<testcase name="Test_EscapedFunctionName'"
			<>&" classname="PRG_TEST.TestXUnitPublisher" time="1.0e-7" status="PASS"></testcase>
		</testsuite>
	</testsuites>

But should be:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites disabled="" failures="1" tests="2" time="3.602e-4">
	<testsuite id="26" name="PRG_TEST.TestXUnitPublisher" tests="2" failures="1" time="3.602e-4">
		<testcase name="Test_EscapedFailedMessage" classname="PRG_TEST.TestXUnitPublisher" time="3.351e-4" status="FAIL">
			<failure message="This &quot;string&quot; &lt;should&gt; be &apos;escaped&apos; &amp; parsed properly" type="BOOL"/>
		</testcase>
		<testcase name="Test_EscapedFunctionName&apos;&quot;&lt;&gt;&amp;" classname="PRG_TEST.TestXUnitPublisher" time="1.0e-7" status="PASS"/>
	</testsuite>
</testsuites>

I've tried to add some basic tests, but I'm not happy with the requirement that is now should be run on a local machine for this to work. I'm happy to omit these tests or adjust this if needed.

Please let me know what you think of this change.

(Note: this is the first time I'm actively contributing to a project, so please let me know if I should do anything different next time :) )

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.

2 participants