Fixed JUnit test-by-config reporting #1160

bitwiseman · 2016-08-24T22:27:28Z

Without some way to distinguish between different config runs of the same tests,
Unit reporters will tend to treat multiple results for the same test case of the duplicates.

Jenkins JUnit report will ignore these "duplicates" producing very confusing test results.

The change add a "class path" the results based on configuration and file path. This allows
reporter consumers to process the results correctly as different results instances.

bitwiseman · 2016-08-24T23:31:35Z

Here's what the output on a sample project with 5 tests on 4 configurations does with this change:

Before this change the multiple configs are treated as one config. After they are seent as separate results.

bitwiseman · 2016-08-26T20:34:36Z

@beatfactor - Should I go back and file an issue for this or can we go with just the pull request?

beatfactor · 2016-08-26T20:41:24Z

For now it's ok, but maybe in the future, it would be appreciated.

bitwiseman · 2016-08-26T20:52:52Z

@beatfactor - if you'd like I can add a few tests for file content to make sure it includes the classname in future. I won't get to it for a couple days though.

beatfactor · 2016-08-26T22:06:13Z

Sure, tests are always welcome.

bitwiseman · 2016-08-31T21:34:13Z

@beatfactor - okay, added content tests.

drptbl · 2016-09-04T13:53:23Z

@bitwiseman
Using Jenkins 2.7.3 (LTS + freestyle project), this pull request breaks parsing (or creation, didn't test it further) of JUnit reports.

15:31:51 ERROR: Step ‘Publish JUnit test result report’ failed: No test report files were found. Configuration error?

bitwiseman · 2016-09-04T17:26:31Z

@drptbl - It changes the file path that the file are written to. Check the glob pattern you're using for finding the files. Make sure it is something like <outputpath>/**/*.xml.

drptbl · 2016-09-04T20:23:24Z

@bitwiseman Sorry, I wasn't aware of that, will do further testing. Is JUnit reports file path change needed and intended? It will break most Jenkins builds after merge to master.

bitwiseman · 2016-09-05T02:48:05Z

@beatfactor @drptbl
It was intentional, and I would argue needed. It still obeys the output directory, it just adds one more level to the tree by putting the config string on the front.

see https://github.com/nightwatchjs/nightwatch/pull/1160/files#diff-11f6da1d76940abb03b522fcd51edb9fR112

I would argue that Nightwatch already makes an arbitrary depth tree for output files, so one more level that adds clarity is a good thing, and patterns that look for these files should already be loading that full tree (via ** glob). However, I'm fine doing something else too.

beatfactor · 2016-09-05T10:51:59Z

Maybe we should just add the classname attribute for now. Would that make any difference?

bitwiseman · 2016-09-06T00:49:44Z

Okay, I removed the file path change. It there is still a change to the module name inside the file but that is required as before the module name wasn't including the configuration.

bitwiseman · 2016-09-09T19:29:06Z

@beatfactor - I think this is all set.

beatfactor · 2016-09-15T08:21:32Z

lib/runner/reporters/junit.js

@@ -56,6 +56,22 @@ module.exports = new (function() {

    adaptAssertions(module);

+    var filename = moduleName + '.xml';


I'm not really in favour of this change, I don't think it's necessary and the potential impact is unknown. I'd rather just stick to the classname attribute change without anymore changes.

I'm going to redo this with tests that verify the current output and then test the change.
I'm going to follow the template of https://github.com/karma-runner/karma-junit-reporter

bitwiseman · 2016-09-15T17:30:13Z

@beatfactor
I've went back and changed this to three commits.
The first adds tests for the existing content.
The second is a pure refactor (which the tests guarantee is non-breaking).
The third is the actual change, which I've re-done to only add the classpath (and the tests show that the classpath is the only change to the output).

Thanks for your patience though these multiple iterations.

bitwiseman · 2016-09-15T17:34:30Z

@beatfactor - It is not clear to me what check failed. The build reported success and then the process in travis exited 1.

beatfactor · 2016-09-15T19:13:35Z

jshint validation failed.

bitwiseman · 2016-09-15T20:16:26Z

@beatfactor - Ah, okay, fixed.

bitwiseman · 2016-09-27T19:32:08Z

@beatfactor ? Ping?

bitwiseman · 2016-10-03T20:41:15Z

@beatfactor - Is there anything else for me to do here?

bitwiseman · 2016-11-29T01:03:07Z

@beatfactor - I've made the changes your requested and all tests are passing. Can we merge?

bitwiseman · 2017-01-03T21:59:55Z

@beatfactor - Hello? Are you there?

Preparing for adding classname to junit file

Without some way to distinguish between different config runs of the same tests, Unit reporters will tend to treat multiple results for the same test case of the duplicates. Jenkins JUnit report will ignore these "duplicates" producing very confusing test results. The change add a "class path" the results based on configuration and file path. This allows reporter consumers to process the results correctly as different results instances.

The combination of testsuite name and package must be unique in the case of multiple junit files or reporting systems may ignore later occurrences as duplicates. It turns out that the testsuite name should be the full class name anyway. https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd#L157

bitwiseman · 2017-01-04T01:50:52Z

@beatfactor - Rebased on HEAD and added one more changed based on feedback from @jk171505.

jk171505 · 2017-01-04T15:26:44Z

@beatfactor, this feature seems to be crucial when testing multi envs, is there a chance to merge it into master?

beatfactor · 2017-01-05T09:01:23Z

Sorry for the delay, I'll look into it in the next few days.

bitwiseman · 2017-02-04T00:37:08Z

@beatfactor - I totally understand getting busy and not having time to review. Whenever you get the chance.

bitwiseman · 2017-02-21T20:46:37Z

@beatfactor - Ping?

beatfactor · 2017-03-11T23:26:08Z

Added classname attribute in v0.9.13

bitwiseman force-pushed the junit-classname branch 2 times, most recently from 232f542 to 42a945a Compare August 24, 2016 23:22

bitwiseman changed the title ~~Fixed JUnit tests by config reporting~~ Fixed JUnit test-by-config reporting Aug 25, 2016

bitwiseman force-pushed the junit-classname branch from 42a945a to d8e3e17 Compare August 31, 2016 21:33

bitwiseman force-pushed the junit-classname branch from d8e3e17 to 9fe7ff5 Compare September 6, 2016 00:47

beatfactor requested changes Sep 15, 2016

View reviewed changes

bitwiseman force-pushed the junit-classname branch from 9fe7ff5 to a9927f9 Compare September 15, 2016 17:25

bitwiseman force-pushed the junit-classname branch from a9927f9 to ed4ac20 Compare September 15, 2016 17:30

bitwiseman force-pushed the junit-classname branch from ed4ac20 to fec898a Compare September 15, 2016 17:35

bitwiseman force-pushed the junit-classname branch from fec898a to d3528fe Compare September 15, 2016 20:14

bitwiseman added 3 commits January 3, 2017 16:27

Add tests for junit file content

3de0cd2

Refactored junit file name calculation

17093e0

Preparing for adding classname to junit file

bitwiseman force-pushed the junit-classname branch from d3528fe to 9dbf21e Compare January 4, 2017 01:40

bitwiseman force-pushed the junit-classname branch from 9dbf21e to 370a3a6 Compare January 4, 2017 01:44

beatfactor added a commit that referenced this pull request Mar 11, 2017

Added classname attribute to junit report, based on #1160

50a42c7

beatfactor closed this Mar 11, 2017

saschagehlich pushed a commit to saschagehlich/nightwatch that referenced this pull request May 16, 2017

Added classname attribute to junit report, based on nightwatchjs#1160

58f20fa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed JUnit test-by-config reporting #1160

Fixed JUnit test-by-config reporting #1160

bitwiseman commented Aug 24, 2016

bitwiseman commented Aug 24, 2016

bitwiseman commented Aug 26, 2016

beatfactor commented Aug 26, 2016

bitwiseman commented Aug 26, 2016

beatfactor commented Aug 26, 2016

bitwiseman commented Aug 31, 2016

drptbl commented Sep 4, 2016 •

edited

Loading

bitwiseman commented Sep 4, 2016

drptbl commented Sep 4, 2016 •

edited

Loading

bitwiseman commented Sep 5, 2016

beatfactor commented Sep 5, 2016

bitwiseman commented Sep 6, 2016

bitwiseman commented Sep 9, 2016

beatfactor Sep 15, 2016

bitwiseman Sep 15, 2016 •

edited

Loading

bitwiseman commented Sep 15, 2016

bitwiseman commented Sep 15, 2016

beatfactor commented Sep 15, 2016

bitwiseman commented Sep 15, 2016 •

edited

Loading

bitwiseman commented Sep 27, 2016

bitwiseman commented Oct 3, 2016

bitwiseman commented Nov 29, 2016

bitwiseman commented Jan 3, 2017

bitwiseman commented Jan 4, 2017

jk171505 commented Jan 4, 2017

beatfactor commented Jan 5, 2017

bitwiseman commented Feb 4, 2017

bitwiseman commented Feb 21, 2017

beatfactor commented Mar 11, 2017

		@@ -56,6 +56,22 @@ module.exports = new (function() {

		adaptAssertions(module);

		var filename = moduleName + '.xml';

Fixed JUnit test-by-config reporting #1160

Fixed JUnit test-by-config reporting #1160

Conversation

bitwiseman commented Aug 24, 2016

bitwiseman commented Aug 24, 2016

bitwiseman commented Aug 26, 2016

beatfactor commented Aug 26, 2016

bitwiseman commented Aug 26, 2016

beatfactor commented Aug 26, 2016

bitwiseman commented Aug 31, 2016

drptbl commented Sep 4, 2016 • edited Loading

bitwiseman commented Sep 4, 2016

drptbl commented Sep 4, 2016 • edited Loading

bitwiseman commented Sep 5, 2016

beatfactor commented Sep 5, 2016

bitwiseman commented Sep 6, 2016

bitwiseman commented Sep 9, 2016

beatfactor Sep 15, 2016

Choose a reason for hiding this comment

bitwiseman Sep 15, 2016 • edited Loading

Choose a reason for hiding this comment

bitwiseman commented Sep 15, 2016

bitwiseman commented Sep 15, 2016

beatfactor commented Sep 15, 2016

bitwiseman commented Sep 15, 2016 • edited Loading

bitwiseman commented Sep 27, 2016

bitwiseman commented Oct 3, 2016

bitwiseman commented Nov 29, 2016

bitwiseman commented Jan 3, 2017

bitwiseman commented Jan 4, 2017

jk171505 commented Jan 4, 2017

beatfactor commented Jan 5, 2017

bitwiseman commented Feb 4, 2017

bitwiseman commented Feb 21, 2017

beatfactor commented Mar 11, 2017

drptbl commented Sep 4, 2016 •

edited

Loading

drptbl commented Sep 4, 2016 •

edited

Loading

bitwiseman Sep 15, 2016 •

edited

Loading

bitwiseman commented Sep 15, 2016 •

edited

Loading