Bug 473251 - Jubula JaCoCo Coverage builder tolerates class file ID collisions
Summary: Jubula JaCoCo Coverage builder tolerates class file ID collisions
Status: CLOSED FIXED
Alias: None
Product: Jubula (Archived)
Classification: Technology
Component: RC (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 critical (vote)
Target Milestone: 4.0   Edit
Assignee: Bela Bara CLA
QA Contact: Oliver Goetz CLA
URL: https://git.eclipse.org/r/#/c/62465/
Whiteboard:
Keywords:
Depends on:
Blocks: 470373
  Show dependency tree
 
Reported: 2015-07-22 02:40 EDT by Markus Tiede CLA
Modified: 2017-01-13 03:54 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Tiede CLA 2015-07-22 02:40:59 EDT
Our JaCoCo Coverage builder logs but tolerates - see [1] - class file ID collisions instead of throwing exceptions - see [2].

This problem and symptoms have already been discussed within the context of bug 470374 comment 9 and following.

I'd strongly recommend to switch to the default JaCoCo Coverage Builder and communicate (configuration) problems like this, that prevent a proper report from being build, to the end-user.

[1] https://bitbucket.org/jubula/com.bredexsw.jubula.core/src/af1406d60fd79f8c35868834109bb58be7e81189/com.bredexsw.guidancer.autagent.monitoring.jacoco/src/com/bredexsw/guidancer/autagent/monitoring/jacoco/CoverageBuilder.java?at=master#CoverageBuilder.java-119

[2] http://www.eclemma.org/jacoco/trunk/doc/api/org/jacoco/core/analysis/CoverageBuilder.html
Comment 1 Markus Tiede CLA 2016-01-05 08:04:00 EST
bela: Just a question. Have you discussed this issue? About, how should we manage the class id collision? https://jubula.atlassian.net/browse/JUB-1372 

[...]

alex: we didn't discuss it yet. My idea for now is to do the following: 
- switch the behaviour to not tolerate class ID collisions, with appropriate console output / log entries
- add an option to the jacoco configuration area that allows the id collisions to be permitted (its default should be "off" - so that the default behaviour is correct, but users have the option to switch back to the incorrect (but maybe working) version.

bela: alright
Comment 2 Markus Tiede CLA 2016-01-05 08:06:44 EST
@Bela: Could you please make sure that your gerrit contribution(s) do not have the state "Merge Conflict" and? Thanks!
Comment 3 Markus Tiede CLA 2016-01-05 08:19:51 EST
I've added comment 1 as we've discussed during todays (daily standup) meeting that our internal chat (which comment 1 is an excerpt of) is not a medium that's supposed to be used in any mean of permanent documentation (as it's e.g. not backed up).
Comment 4 Bela Bara CLA 2016-01-06 02:24:04 EST
(In reply to Markus Tiede from comment #2)
> @Bela: Could you please make sure that your gerrit contribution(s) do not
> have the state "Merge Conflict" and? Thanks!

fixed
Comment 5 Markus Tiede CLA 2016-01-06 04:44:27 EST
"Logging" configuration problems like this as a warning - see [1] - to the AUT-agent log file is not exactly what I had in mind with "communicate (configuration) problems to the end-user".

Am I right with the assumption that the JaCoCo report generation after an ITE / testexec test run execution now silently fails and no report is getting written if such a collision occurs?

In my opinion we should communicate this problem back (by adding the exception information to the corresponding response message - see [2]) to the ITE and at least properly fail the test execution core runtime job with an IStatus - see [3] - that's != OK; containing the descriptions that otherwise only getting logged.

[1] https://bitbucket.org/jubula/com.bredexsw.jubula.core/commits/badf26b72153b4b75a534dbc4cd72d68666616e1?at=master

[2] http://git.eclipse.org/c/jubula/org.eclipse.jubula.core.git/tree/org.eclipse.jubula.communication/src/org/eclipse/jubula/communication/internal/message/GetMonitoringDataResponseMessage.java

[3] http://help.eclipse.org/mars/topic/org.eclipse.platform.doc.isv/reference/api/org/eclipse/core/runtime/IStatus.html
Comment 6 Alexandra Schladebeck CLA 2016-01-06 10:19:16 EST
I agree that the user should receive the information that the report failed with this reason in the ITE (and via the testexec).
Comment 7 Bela Bara CLA 2016-01-06 10:24:12 EST
Added notification, when the report failed.
Comment 8 Alexandra Schladebeck CLA 2016-01-06 10:29:41 EST
For testexec, the exit code should not be changed, but an error line should be produced for the code coverage report building.
Comment 9 Bela Bara CLA 2016-01-07 08:57:45 EST
In ITE a dialog is shown in case of class file id collision. If I see well, in testexec the code coverage is not called.
Comment 10 Alexandra Schladebeck CLA 2016-01-14 06:37:10 EST
Hi Bela,  code coverage should be called in testexec when it is specified for the AUT configuration started. Can you check this?
Comment 11 Bela Bara CLA 2016-01-14 09:03:12 EST
Code coverage is called in testexec.
Comment 12 Alexandra Schladebeck CLA 2016-02-11 02:52:38 EST
TO my knowledge, this has been resolved.
Comment 13 Elena Pister CLA 2017-01-13 03:54:12 EST
closing as resolved and no activity on this bug