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

Make jacoco work with integration tests #459

Open
jonesbusy opened this issue Dec 15, 2024 · 22 comments
Open

Make jacoco work with integration tests #459

jonesbusy opened this issue Dec 15, 2024 · 22 comments
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted

Comments

@jonesbusy
Copy link
Collaborator

jonesbusy commented Dec 15, 2024

What feature do you want to see added?

Right now coverage is only computed on unit tests

When we run integration tests with failsafe and 'java -jar..' we should add the argLine to the execution and make sure the execution is merged with previous jacoco execution.

Possible code change on CommandLineIT if we need to pass the jacoco argLine ?

Properties properties = new Properties();
String changeList = System.getProperty("set.changelist");
if (changeList != null) {
    properties.put("set.changelist", "true");
}
properties.put("exec.executable", javaHome.resolve("bin/java").toString());
properties.put("test.cliArgs", args);
request.setProperties(properties);
``

### Upstream changes

No code change expected, and maven config
@jonesbusy jonesbusy added the enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Dec 15, 2024
@jonesbusy jonesbusy added the good first issue Good for newcomers label Dec 28, 2024
@jonesbusy
Copy link
Collaborator Author

Good first issue if someone interested by Maven config and jacoco coverage

@jonesbusy
Copy link
Collaborator Author

@jonesbusy
Copy link
Collaborator Author

Dis some quick test without maven

curl -O https://repo1.maven.org/maven2/org/jacoco/org.jacoco.cli/0.8.12/org.jacoco.cli-0.8.12-nodeps.jar 
curl -O https://repo1.maven.org/maven2/org/jacoco/org.jacoco.agent/0.8.12/org.jacoco.agent-0.8.12-runtime.jar

# Run with agent
java '-javaagent:org.jacoco.agent-0.8.12-runtime.jar=port=6300,address=0.0.0.0,destfile=jacoco.exec,includes=io.jenkins.tools.pluginmodernizer.*,append=true,output=tcpserver' -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar build-metadata --plugins echarts-api --debug

# On other shell
java -jar org.jacoco.cli-0.8.12-nodeps.jar dump --address localhost --port 6300 --destfile jacoco.exec --reset
java -jar org.jacoco.cli-0.8.12-nodeps.jar report jacoco.exec --classfiles plugin-modernizer-core/target/classes --sourcefiles plugin-modernizer-core/src/main/java --html coverage_report

Screenshot from 2024-12-28 10-58-11

Now we need to integration with failsafe, maven exec and jacoco plugin.

And merge all together with previous unit tests

@Yash-jaiswal2509
Copy link

Hi, as I was working on this. I wanted to know if I will have to also change pom.xml and pom-it.xml to integrate jacoco.

@jonesbusy
Copy link
Collaborator Author

Most likely yes.

@ahmad-kashkoush
Copy link

ahmad-kashkoush commented Feb 6, 2025

@jonesbusy I'm currently working in this issue, What I've done is:
update pom.xml and pom-it.xml in plugin-modernizer-cli.

but what happened is that some tests failed in commandLineITCase, so should I update those tests or update the buildRequest function itself?

Image

@jonesbusy
Copy link
Collaborator Author

jonesbusy commented Feb 6, 2025

Test are working. Double check your environment and check logs. You might have invalid JAVA_HOME and or MAVEN_HOME.

If you search carefully it will most likely indicate what is the issue

@jonesbusy jonesbusy removed the good first issue Good for newcomers label Feb 6, 2025
@jonesbusy
Copy link
Collaborator Author

I've removed the good-first issue. Even if its mostly configuration, it need good understanding on Maven and Jacoco and understanding how plugin modernizer jar is invoked during tests.

I don't exclude the fact we might need an other module only to aggregate tests results.

Feel free to continue on it if you are interested by the topic.

@ahmad-kashkoush
Copy link

ahmad-kashkoush commented Feb 7, 2025

I've removed the good-first issue. Even if its mostly configuration, it need good understanding on Maven and Jacoco and understanding how plugin modernizer jar is invoked during tests.

I don't exclude the fact we might need an other module only to aggregate tests results.

Feel free to continue on it if you are interested by the topic.

thanks for reply, I'm going to continue a little bit more.

What I'm currently doing is configuring pom.xml and pom-it.xml files such that I get :

  1. plugin-modernizer-cli: Coverage from CLI module .
  2. plugin-modernizer-core → Coverage from Core module .
  3. and merge coverage of both

@ahmad-kashkoush
Copy link

@jonesbusy I've typed the bash script that gather it, that is what it does, is it the solution we want?

Image

When clicking on io.jenkins.pluginmodernizer.cli:

Image

If that is what we want, I'll just configure pom and pom-it files to satisfy that

@jonesbusy
Copy link
Collaborator Author

This is incorrect to have coverage on test class. We want coverage on application code

@jonesbusy
Copy link
Collaborator Author

What are those screenshot? Final result? If yes this looks very incorrect. I expect the total coverage to be much higher than the current one of 60% only for unit test

@ahmad-kashkoush
Copy link

What are those screenshot? Final result? If yes this looks very incorrect. I expect the total coverage to be much higher than the current one of 60% only for unit test

No, it is not the final result.
I'm just assuring that the second screenshot have the integration tests needed.

@jonesbusy
Copy link
Collaborator Author

What are those screenshot? Final result? If yes this looks very incorrect. I expect the total coverage to be much higher than the current one of 60% only for unit test

No, it is not the final result.
I'm just assuring that the second screenshot have the integration tests needed.

I don't get it. You are not supposed to see any test class on jacoco report. We are interested by the total coverage for both unit and integration tests.

@jonesbusy
Copy link
Collaborator Author

#459 (comment)

This show the 'build-metadata- command. This is typically one test on CommandLineIT (which the only class for now with failsafe test)

So the total suite would be much higher than that. Specially if after we combinate with surefire execution

@ahmad-kashkoush
Copy link

What are those screenshot? Final result? If yes this looks very incorrect. I expect the total coverage to be much higher than the current one of 60% only for unit test

No, it is not the final result.
I'm just assuring that the second screenshot have the integration tests needed.

I don't get it. You are not supposed to see any test class on jacoco report. We are interested by the total coverage for both unit and integration tests.

got it, sorry for my misunderstanding

@ahmad-kashkoush
Copy link

ahmad-kashkoush commented Feb 8, 2025

#459 (comment)

This show the 'build-metadata- command. This is typically one test on CommandLineIT (which the only class for now with failsafe test)

So the total suite would be much higher than that. Specially if after we combinate with surefire execution

Ok, I'll work based on that.

Should I be working with same plugin mentioned, or it is general to any plugin e.g. git-plugin?

@jonesbusy
Copy link
Collaborator Author

Check CommandLineIT. They are using plugin from resources. Not real plugin

@ahmad-kashkoush
Copy link

@jonesbusy is the current coverage correct this way? I made to run only on application code
Image

Image

@ahmad-kashkoush
Copy link

I've typed a small bashscript specifing as a prototype, if the above correct, I'll configure in pom.xml and pom-it.xml files

#!/bin/bash

java "-javaagent:org.jacoco.agent-0.8.12-runtime.jar=port=6300,address=0.0.0.0,destfile=target/jacoco.exec,includes=io.jenkins.tools.pluginmodernizer.*,append=true,output=tcpserver" -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar build-metadata --plugins git --debug

java '-javaagent:org.jacoco.agent-0.8.12-runtime.jar=port=6300,address=0.0.0.0,destfile=target/jacoco-it.exec,includes=io.jenkins.tools.pluginmodernizer.*,append=true,output=tcpserver' -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar build-metadata  --plugins git --debug

to dump data

java -jar org.jacoco.cli-0.8.12-nodeps.jar dump  \
	--address localhost \
	--port 6300 \
	--destfile target/jacoco.exec  \
	--reset \
        ;
java -jar org.jacoco.cli-0.8.12-nodeps.jar dump  \
        --address localhost \
        --port 6300 \
        --destfile target/jacoco-it.exec  \
        --reset

to generate report

#!/bin/bash

java -jar org.jacoco.cli-0.8.12-nodeps.jar report target/jacoco.exec target/jacoco-it.exec \
    --classfiles plugin-modernizer-core/target/classes \
    --sourcefiles plugin-modernizer-core/src/main/java \
    --classfiles plugin-modernizer-cli/target/classes \
    --sourcefiles plugin-modernizer-cli/src/main/java \
    --html coverage_report

@jonesbusy
Copy link
Collaborator Author

I've typed a small bashscript specifing as a prototype, if the above correct, I'll configure in pom.xml and pom-it.xml files

#!/bin/bash

java "-javaagent:org.jacoco.agent-0.8.12-runtime.jar=port=6300,address=0.0.0.0,destfile=target/jacoco.exec,includes=io.jenkins.tools.pluginmodernizer.*,append=true,output=tcpserver" -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar build-metadata --plugins git --debug

java '-javaagent:org.jacoco.agent-0.8.12-runtime.jar=port=6300,address=0.0.0.0,destfile=target/jacoco-it.exec,includes=io.jenkins.tools.pluginmodernizer.*,append=true,output=tcpserver' -jar plugin-modernizer-cli/target/jenkins-plugin-modernizer-999999-SNAPSHOT.jar build-metadata --plugins git --debug

to dump data

java -jar org.jacoco.cli-0.8.12-nodeps.jar dump
--address localhost
--port 6300
--destfile target/jacoco.exec
--reset
;
java -jar org.jacoco.cli-0.8.12-nodeps.jar dump
--address localhost
--port 6300
--destfile target/jacoco-it.exec
--reset

to generate report

#!/bin/bash

java -jar org.jacoco.cli-0.8.12-nodeps.jar report target/jacoco.exec target/jacoco-it.exec
--classfiles plugin-modernizer-core/target/classes
--sourcefiles plugin-modernizer-core/src/main/java
--classfiles plugin-modernizer-cli/target/classes
--sourcefiles plugin-modernizer-cli/src/main/java
--html coverage_report

No so different from #459 (comment)?

Using some combinaison of dumpOnExit and output I guess we should be able to get the report when the VM exit (https://www.eclemma.org/jacoco/trunk/doc/prepare-agent-integration-mojo.html)

The whole point here is to make the maven configuration to pass the existing argLine and merge reports

@ahmad-kashkoush
Copy link

ahmad-kashkoush commented Feb 10, 2025

No so different from #459 (comment)?

It includes all coverage for application code, I wanted to know If paths I've specified in the script are correct to move to the next step.

Using some combinaison of dumpOnExit and output I guess we should be able to get the report when the VM exit (https://www.eclemma.org/jacoco/trunk/doc/prepare-agent-integration-mojo.html)

Thanks for that, I've been trying to figure out how to make dump work after agent preparation

The whole point here is to make the maven configuration to pass the existing argLine and merge reports

I'll do that, thanks for your quick replies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants