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

Added delegates for modifying the JUnit testcase classname and name #186

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
46 changes: 46 additions & 0 deletions spec/JUnitXmlReporterSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,52 @@ describe("JUnitXmlReporter", function(){
itShouldIncludeXmlPreambleInAllFiles();
});

describe("modifyClassName", () => {
var suites;
var modification = "modified";
beforeEach(function () {
setupReporterWithOptions({
// consolidateAll: true,
// consolidate: true,
modifyClassName: function () {
return modification;
}
});
triggerRunnerEvents();
suites = writeCalls[0].xmldoc.getElementsByTagName("testcase");
});
it("should construct classnames that are the provided modification", () => {
expect(suites).not.toBeNull();
expect(suites.length).toBeGreaterThan(0);
for (var i = 0; i < suites.length; i++) {
expect(suites[i].getAttribute("classname")).toBe(modification);
}
});
});

describe("modifySpecName", () => {
var suites;
var modification = "modified";
beforeEach(function () {
setupReporterWithOptions({
// consolidateAll: true,
// consolidate: true,
modifySpecName: function () {
return modification;
}
});
triggerRunnerEvents();
suites = writeCalls[0].xmldoc.getElementsByTagName("testcase");
});
it("should construct spec names that are the provided modification", () => {
expect(suites).not.toBeNull();
expect(suites.length).toBeGreaterThan(0);
for (var i = 0; i < suites.length; i++) {
expect(suites[i].getAttribute("name")).toBe(modification);
}
});
});

describe("captures stdout in <xml-output>", function(){
var specOutputs;
const testOutput = "I'm generating test output.";
Expand Down
15 changes: 12 additions & 3 deletions src/junit_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@
* This is useful when running a test suite against multiple capabilities
* because the report can have unique names for each combination of suite/spec
* and capability/test environment.
* @param {function} [modifyClassName] a delegate to modify spec's classname in the xml report.
+ * @param {function} [modifySpecName] a delegate to modify the spec's name in the xml report.
* @param {string} [stylesheetPath] is the string value that specifies a path
* to an XSLT stylesheet to add to the junit XML file so that it can be
* opened directly in a browser. (default: none, no xml-stylesheet tag is added)
Expand Down Expand Up @@ -195,6 +197,12 @@
if(options.systemOut && typeof options.systemOut !== "function") {
throw new Error('option "systemOut" must be a function');
}
if (options.modifyClassName && typeof options.modifyClassName !== "function") {
throw new Error('option "modifyClassName" must be a function');
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does modifyClassName provide that isn't already available through modifySuiteName? The primary difference appears to be that modifySuiteName receives the suite as input, whereas modifyClassName receives the spec—is that the important distinction? It gives me pause to have two separate options which both are ultimately meant for modifying the classname property in the XML document, but providing one of the options prevents the other from being useful.

Since modifySuiteName is already available and is used to modify the classname property—it is called from within getFullyQualifiedSuiteName—I'd prefer a solution that doesn't introduce new ways of modifying the same property. If you need the spec available to your function, I'd propose to change the interface of modifySuiteName to also receive the spec itself as the second parameter, thus retaining backwards compatibility but presumably also giving you what you're looking for with this new option.

}
if (options.modifySpecName && typeof options.modifySpecName !== "function") {
throw new Error('option "modifySpecName" must be a function');
}

self.captureStdout = options.captureStdout || false;
if(self.captureStdout && !options.systemOut) {
Expand All @@ -208,6 +216,8 @@
delegates.modifySuiteName = options.modifySuiteName;
delegates.modifyReportFileName = options.modifyReportFileName;
delegates.systemOut = options.systemOut;
delegates.modifyClassName = options.modifyClassName;
delegates.modifySpecName = options.modifySpecName;

self.logEntries = [];

Expand Down Expand Up @@ -454,9 +464,8 @@
}
function specAsXml(spec) {
var testName = self.useFullTestName ? spec.fullName : spec.description;

var xml = '\n <testcase classname="' + getFullyQualifiedSuiteName(spec._suite) + '"';
xml += ' name="' + escapeInvalidXmlChars(testName) + '"';
var xml = '\n <testcase classname="' + escapeInvalidXmlChars(delegates.modifyClassName ? delegates.modifyClassName(spec) : getFullyQualifiedSuiteName(spec._suite)) + '"';
xml += ' name="' + escapeInvalidXmlChars(delegates.modifySpecName ? delegates.modifySpecName(spec) : testName) + '"';
xml += ' time="' + elapsed(spec._startTime, spec._endTime) + '"';

var testCaseBody = "";
Expand Down