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

Add support for absolute paths for jacoco exec and report #34544

Merged
merged 1 commit into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.nio.file.Paths;
import java.util.Collections;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;

Expand Down Expand Up @@ -35,6 +36,9 @@

public class JacocoProcessor {

public static final String JACOCO_QUARKUS_EXEC = "jacoco-quarkus.exec";
public static final String JACOCO_REPORT = "jacoco-report";

@BuildStep(onlyIf = IsTest.class)
FeatureBuildItem feature() {
return new FeatureBuildItem("jacoco");
Expand All @@ -53,10 +57,8 @@ void transformerBuildItem(BuildProducer<BytecodeTransformerBuildItem> transforme
return;
}

String dataFile = outputTargetBuildItem.getOutputDirectory().toAbsolutePath().toString() + File.separator
+ config.dataFile;
System.setProperty("jacoco-agent.destfile",
dataFile);
String dataFile = getFilePath(config.dataFile, outputTargetBuildItem.getOutputDirectory(), JACOCO_QUARKUS_EXEC);
System.setProperty("jacoco-agent.destfile", dataFile);
if (!config.reuseDataFile) {
Files.deleteIfExists(Paths.get(dataFile));
}
Expand Down Expand Up @@ -95,8 +97,7 @@ public byte[] apply(String className, byte[] bytes) {
info.dataFile = dataFile;

File targetdir = new File(
outputTargetBuildItem.getOutputDirectory().toAbsolutePath().toString() + File.separator
+ config.reportLocation);
getFilePath(config.reportLocation, outputTargetBuildItem.getOutputDirectory(), JACOCO_REPORT));
info.reportDir = targetdir.getAbsolutePath();
String includes = String.join(",", config.includes);
String excludes = String.join(",", config.excludes.orElse(Collections.emptyList()));
Expand All @@ -122,7 +123,8 @@ public byte[] apply(String className, byte[] bytes) {

private void addProjectModule(ResolvedDependency module, JacocoConfig config, ReportInfo info, String includes,
String excludes, Set<String> classes, Set<String> sources) throws Exception {
info.savedData.add(new File(module.getWorkspaceModule().getBuildDir(), config.dataFile).getAbsolutePath());
String dataFile = getFilePath(config.dataFile, module.getWorkspaceModule().getBuildDir().toPath(), JACOCO_QUARKUS_EXEC);
info.savedData.add(new File(dataFile).getAbsolutePath());
if (module.getSources() == null) {
return;
}
Expand All @@ -140,4 +142,8 @@ private void addProjectModule(ResolvedDependency module, JacocoConfig config, Re
}
}
}

private String getFilePath(Optional<String> path, Path outputDirectory, String defaultRelativePath) {
return path.orElse(outputDirectory.toAbsolutePath() + File.separator + defaultRelativePath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
public class JacocoConfig {

/**
* The jacoco data file
* The jacoco data file. By default this will be target/jacoco-quarkus.exec.
Copy link
Member

Choose a reason for hiding this comment

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

Could we use defaultValueForDocumentation instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't remember that at all. @vsevel fancy doing a new PR with this improvement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I will add some info on the guide as well

* The path can be relative (to the module) or absolute.
*/
@ConfigItem(defaultValue = "jacoco-quarkus.exec")
public String dataFile;
@ConfigItem
public Optional<String> dataFile;
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the rationale of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the default to be the equivalent of target/jacoco-quarkus.exec, but since I wanted to reuse outputTargetBuildItem.getOutputDirectory() and module.getWorkspaceModule().getBuildDir() in the processor, I could not use a WithDefault anymore.
so now it is optional, but a default is calculated in the processor directly.
there are no changes for people who did not set data-file.
for people that did set data-file=foo, they need to change it to target/foo.
your advice more than welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks.

What I would like however is some Javadoc explaining this process so users who might want to configure it can find this information on our config page (or their IDE if it supports it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing, which I did I believe:

    /**
     * The jacoco data file. By default this will be target/jacoco-quarkus.exec.
     * The path can be relative (to the module) or absolute.
     */
    @ConfigItem
    public Optional<String> dataFile;

do you think this should be more detailed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's fine.


/**
* Whether to reuse ({@code true}) or delete ({@code false}) the jacoco
Expand Down Expand Up @@ -82,8 +83,9 @@ public class JacocoConfig {
public Optional<List<String>> excludes;

/**
* The location of the report files.
* The location of the report files. By default this will be target/jacoco-report.
* The path can be relative (to the module) or absolute.
*/
@ConfigItem(defaultValue = "jacoco-report")
public String reportLocation;
@ConfigItem
public Optional<String> reportLocation;
Comment on lines +89 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same reason as for data file

}