🐛 Bug: xUnit XML format is actually JUnit's schema

[andyleejordan]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Hi there,

I feel like I'm crazy, but as far as I can tell, the XML output for Mocha's xUnit reporter is actually JUnit's schema. In short, it's emitting something like:

<testsuite ... >
  <testcase ... />
<testsuite />

Which is most definitely the documented JUnit XML schema. Mocha's own tests for the xUnit output confirm this what it's intended to emit:

mocha/test/reporters/xunit.spec.js

Lines 322 to 453 in 0ea732c

	describe('on test failure', function() {
	it('should write expected tag with error details', function() {
	var xunit = new XUnit(runner);
	var expectedTest = {
	state: STATE_FAILED,
	title: expectedTitle,
	parent: {
	fullTitle: function() {
	return expectedClassName;
	}
	},
	duration: 1000,
	err: {
	actual: 'foo',
	expected: 'bar',
	message: expectedMessage,
	stack: expectedStack
	}
	};
	xunit.test.call(fakeThis, expectedTest);
	sinon.restore();
	var expectedTag =
	'<testcase classname="' +
	expectedClassName +
	'" name="' +
	expectedTitle +
	'" time="1"><failure>' +
	expectedMessage +
	'\n' +
	expectedDiff +
	'\n' +
	expectedStack +
	'</failure></testcase>';
	expect(expectedWrite, 'to be', expectedTag);
	});
	it('should handle non-string diff values', function() {
	var runner = new EventEmitter();
	createStatsCollector(runner);
	var xunit = new XUnit(runner);
	var expectedTest = {
	state: STATE_FAILED,
	title: expectedTitle,
	parent: {
	fullTitle: function() {
	return expectedClassName;
	}
	},
	duration: 1000,
	err: {
	actual: 1,
	expected: 2,
	message: expectedMessage,
	stack: expectedStack
	}
	};
	sinon.stub(xunit, 'write').callsFake(function(str) {
	expectedWrite += str;
	});
	runner.emit(EVENT_TEST_FAIL, expectedTest, expectedTest.err);
	runner.emit(EVENT_RUN_END);
	sinon.restore();
	var expectedDiff =
	'\n + expected - actual\n\n -1\n +2\n ';
	expect(expectedWrite, 'to contain', expectedDiff);
	});
	});
	describe('on test pending', function() {
	it('should write expected tag', function() {
	var xunit = new XUnit(runner);
	var expectedTest = {
	isPending: function() {
	return true;
	},
	title: expectedTitle,
	parent: {
	fullTitle: function() {
	return expectedClassName;
	}
	},
	duration: 1000
	};
	xunit.test.call(fakeThis, expectedTest);
	sinon.restore();
	var expectedTag =
	'<testcase classname="' +
	expectedClassName +
	'" name="' +
	expectedTitle +
	'" time="1"><skipped/></testcase>';
	expect(expectedWrite, 'to be', expectedTag);
	});
	});
	describe('on test in any other state', function() {
	it('should write expected tag', function() {
	var xunit = new XUnit(runner);
	var expectedTest = {
	isPending: function() {
	return false;
	},
	title: expectedTitle,
	parent: {
	fullTitle: function() {
	return expectedClassName;
	}
	},
	duration: false
	};
	xunit.test.call(fakeThis, expectedTest);
	sinon.restore();
	var expectedTag =
	'<testcase classname="' +
	expectedClassName +
	'" name="' +
	expectedTitle +
	'" time="0"/>';
	expect(expectedWrite, 'to be', expectedTag);
	});
	});

However, the two versions of xUnit XML schema (v1 and v2) look radically different from this! For one, the top-level element must be <assemblies>, which doesn't exist in Mocha's codebase. And neither use <testcase> nor <testsuite> (they both use a <test> element with further information provided by subelements).

I don't know how I'm the first to notice this when this project is used by 1.4 million other things on GitHub, which is why I think I must be wrong, despite staring at the documentation and tests I've just linked that justify this bug is real.

[andyleejordan]

Hey @juergba, any follow up on this, am I crazy?

[miensol]

I'm relieved. I thought I'm missing something as well. Naming things is indeed hard.

[miensol]

What's even more interesting is that apparently this is not the only case where the names about xunit and junit aren't correct.
I was trying to verify if Reportportal supports xunit XML files import. I found this class junit/XunitImportHandler.java which only supports junit XML format...

[EnricoMi]

Yes, this is JUnit XML, not XUnit XML.

[EnricoMi]

@jkrall @tj @boneskull what do you think?

[JoshuaKGoldberg]

cc @mochajs/maintenance-crew - looks like this is a long-standing issue that has tripped quite a few folks up. My intuition is we'd want to make a breaking change fix in the next major version that:

• Renames the xunit reporter to junit, since it's emitting jUnit XML
• Creates a new xunit reporter that actually emits xUnit XML

Thoughts?

[Maverick099]

Do we have a timeline for when this issue will be closed?

[JoshuaKGoldberg]

Status update on the issue itself: nobody has protested, so I just marked it as status: accepting prs. It'd be great if someone could send a PR for this!

As for timelines, it'll be a while. We've got to get through:

1. A small major version release with v11 first: 🧹 Versioning: Bump minimum Node.js version from 14.0.0 to 18.18.0 #5206 -> fix!: adapt new engine range for Mocha 11 #5216
2. Migrating the old hard-to-work-with website to the new one: 📝 Docs: Overhaul website generation #5207 -> docs: add new website using Astro Starlight #5246

But, we can still review a PR before those two are done! We just won't be able to merge & release it till then.