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.

Sign up for GitHub

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

Jump to bottom

fix or remove jacoco IllegalClassFormatException: Error while instrumenting #4985

Closed
keturn opened this issue Jan 19, 2022 · 5 comments
Closed

fix or remove jacoco IllegalClassFormatException: Error while instrumenting #4985

keturn opened this issue Jan 19, 2022 · 5 comments
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance Good First Issue Good for learners that are new to Terasology Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Topic: Stabilization Requests, Issues and Changes related to improving stablity and reducing flakyness Type: Bug Issues reporting and PRs fixing problems Type: Chore Request for or implementation of maintenance changes

Comments

@keturn
Copy link
Member

keturn commented Jan 19, 2022 •
edited by jdrueckert
Loading

When running tests, we get a lot of noise in the output that looks like this:

java.lang.instrument.IllegalClassFormatException: Error while instrumenting org/terasology/engine/logic/console/ConsoleSystemMethodAccess.
at org.jacoco.agent.rt.internal_43f5073.CoverageTransformer.transform(CoverageTransformer.java:94)

as seen on https://jenkins.terasology.io/teraorg/job/Terasology/job/Modules/job/B/job/Behaviors/view/change-requests/job/PR-101/lastCompletedBuild/testReport/org.terasology.module.behaviors/MovementTests/_walking__leaping__swimming___leap/ :

java.lang.instrument.IllegalClassFormatException: Error while instrumenting org/terasology/engine/logic/console/ConsoleSystemMethodAccess.
	at org.jacoco.agent.rt.internal_43f5073.CoverageTransformer.transform(CoverageTransformer.java:94)
	at java.instrument/java.lang.instrument.ClassFileTransformer.transform(Unknown Source)
	at java.instrument/sun.instrument.TransformerManager.transform(Unknown Source)
	at java.instrument/sun.instrument.InstrumentationImpl.transform(Unknown Source)
	at java.bas
...[truncated 1257624 chars]...
de(ClassReader.java:1855)
	at org.jacoco.agent.rt.internal_43f5073.asm.ClassReader.readMethod(ClassReader.java:1284)
	at org.jacoco.agent.rt.internal_43f5073.asm.ClassReader.accept(ClassReader.java:688)
	at org.jacoco.agent.rt.internal_43f5073.asm.ClassReader.accept(ClassReader.java:400)
	at org.jacoco.agent.rt.internal_43f5073.core.instr.Instrumenter.instrument(Instrumenter.java:89)
	at org.jacoco.agent.rt.internal_43f5073.core.instr.Instrumenter.instrument(Instrumenter.java:107)
	... 180 more

Jacoco is a thing that's supposed to be giving us metrics of the code coverage of our tests.

This falls under the category of #3859.

My hunch is that some Java permissions thing changed between versions and it can't snoop on execution in the way that it used to.

It's probably fixable, since code coverage metrics are a very standard thing for a project to want.

But if it looks like a complicated fix, I don't think we'd miss much by dropping the jacoco integration from our gradle configuration. In our current usage, it's not much more than a historical footnote on the charts of our CI builds.
@keturn keturn added Type: Bug Issues reporting and PRs fixing problems Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Good First Issue Good for learners that are new to Terasology Topic: Stabilization Requests, Issues and Changes related to improving stablity and reducing flakyness Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance Type: Chore Request for or implementation of maintenance changes labels Jan 19, 2022
@VVR20
Copy link

VVR20 commented Jan 24, 2022

I would like to work on this issue . Could someone point to where in the source code i should start looking?
@keturn

@DarkWeird
Copy link
Contributor

I would like to work on this issue . Could someone point to where in the source code i should start looking?
@keturn

There no source code.

Jacoco cannot transform generated bytecode for inserting self counting code.

Bytecode generator is reflectasm. using placed at EventSystem or near.

There generating bytecode method and field accessor, which have more performance then reflection access.

Imho, right fix is adding generated bytecode in exludes of jacoco.

@bakersemail
Copy link

Link no longer works but still an issue from what I can tell:
https://jenkins.terasology.io/teraorg/job/Terasology/job/Modules/job/B/job/Behaviors/view/change-requests/job/PR-101/lastCompletedBuild/testReport/org.terasology.module.behaviors/MovementTests/_walking__leaping__swimming___leap/

@jdrueckert
Copy link
Member

Link no longer works but still an issue from what I can tell: jenkins.terasology.io/teraorg/job/Terasology/job/Modules/job/B/job/Behaviors/view/change-requests/job/PR-101/lastCompletedBuild/testReport/org.terasology.module.behaviors/MovementTests/_walking__leaping__swimming___leap

@bakersemail Indeed, thanks for the note on the broken link and this still being an issue!
I replaced the link in the issue description and added the log output so in case the jenkins job is cleaned up at some point in the future, we still have the log information.

If you would be interested in helping fix this, it would be highly appreciated! If there's questions, feel free to ask here or on our Discord 😉

keturn added a commit that referenced this issue May 12, 2022
@keturn
fix(test): set jacoco excludes for engine-test's unitTest and integra…
ea430c3 
…tionTest tasks

upgrade to jacoco 0.8.8 (from 0.8.5)

relates to #4985
@keturn keturn mentioned this issue May 12, 2022
Merged
keturn added a commit that referenced this issue May 12, 2022
@keturn
fix(test): set jacoco excludes for engine-test's unitTest and integra…
c415b6c 
…tionTest tasks

upgrade to jacoco 0.8.8 (from 0.8.5)

relates to #4985
keturn added a commit that referenced this issue May 13, 2022
@keturn
fix(test): set jacoco excludes for engine-test's unitTest and integra…
d4a8b22 
…tionTest tasks

upgrade to jacoco 0.8.8 (from 0.8.5)

relates to #4985
@keturn keturn mentioned this issue May 21, 2022
Merged
@keturn
Copy link
Member Author

keturn commented May 25, 2022

Fixed by #5020, which removed jacoco from the build configuration.

@keturn keturn closed this as completed May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance Good First Issue Good for learners that are new to Terasology Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Topic: Stabilization Requests, Issues and Changes related to improving stablity and reducing flakyness Type: Bug Issues reporting and PRs fixing problems Type: Chore Request for or implementation of maintenance changes
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@keturn @bakersemail @DarkWeird @jdrueckert @VVR20