Skip to content

Added support for scalac-scoverage-plugin > 2.0.0 #107

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

Merged
merged 5 commits into from
Jul 18, 2023
Merged

Added support for scalac-scoverage-plugin > 2.0.0 #107

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
213462f
Added support for scoverage-compiler-plugin > 2.0.0
samantmaharaj Dec 14, 2022
ea48709
Explicitly handled the cases for the different versions of scalac-sco…
samantmaharaj Feb 13, 2023
63e0174
Fixed typo where the major version check should have been greater tha…
samantmaharaj Feb 13, 2023
034e40c
Modified animal sniffer to target java 1.8 APIs
samantmaharaj Feb 14, 2023
6ec7b6a
Made plugin artifacts path separator platform independent
samantmaharaj May 23, 2023
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
12 changes: 6 additions & 6 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ under the License.
<maven.version>2.2.1</maven.version>
<maven-plugin-plugin.version>3.6.0</maven-plugin-plugin.version>

<scalac-scoverage-plugin.version>1.4.11</scalac-scoverage-plugin.version>
<scalac-scoverage-plugin.scala.version>2.12.15</scalac-scoverage-plugin.scala.version>
<scalac-scoverage-plugin.version>2.0.7</scalac-scoverage-plugin.version>
<scalac-scoverage-plugin.scala.version>2.13</scalac-scoverage-plugin.scala.version>
Copy link

Choose a reason for hiding this comment

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

I think this may have to be pinned to e.g. 2.13.10, to actually work? (going by available maven coordinates)
You may also want to bump the plugin version to 2.0.8 already

Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is only being used by the reporter down below, which doesn't need to be the exact version of Scala, only the binary 2.12, 2.13, or 3. The actual compiler plugin is what needs to be the full version.

Copy link

Choose a reason for hiding this comment

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

referencing that in the variable name would make it unmistakable.

</properties>

<dependencies>
<dependency>
<groupId>org.scoverage</groupId>
<artifactId>scalac-scoverage-plugin_${scalac-scoverage-plugin.scala.version}</artifactId>
<artifactId>scalac-scoverage-reporter_${scalac-scoverage-plugin.scala.version}</artifactId>
<version>${scalac-scoverage-plugin.version}</version>
</dependency>

Expand Down Expand Up @@ -264,12 +264,12 @@ under the License.
<artifactId>umlgraph</artifactId>
<version>5.6.6</version>
</docletArtifact>
<additionalparam>
<additionalOptions>
-inferrel -inferdep -quiet -hide (java|javax)\..*
-collpackages java\.util\..* -qualify
-postfixpackage -nodefontsize 9
-nodefontpackagesize 7
</additionalparam>
</additionalOptions>
</configuration>
</plugin>

Expand Down Expand Up @@ -380,7 +380,7 @@ under the License.
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java16</artifactId>
<artifactId>java18</artifactId>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to targeting Java 1.8

<version>1.0</version>
</signature>
</configuration>
Expand Down
23 changes: 16 additions & 7 deletions src/main/java/org/scoverage/plugin/SCoverageCheckMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
import org.apache.maven.project.MavenProject;

import scala.Tuple2;
import scala.collection.JavaConverters;
import scala.jdk.javaapi.CollectionConverters;

import scoverage.Coverage;
import scoverage.IOUtils;
import scoverage.Serializer;
import scoverage.domain.Coverage;
import scoverage.reporter.IOUtils;
import scoverage.serialize.Serializer;

/**
* Checks if minimum code coverage by unit tests reached
Expand Down Expand Up @@ -93,6 +93,15 @@ public class SCoverageCheckMojo
@Parameter( property = "scoverage.failOnMinimumCoverage", defaultValue = "false" )
private boolean failOnMinimumCoverage;

/**
* The file encoding to use when reading Scala sources.
* <br>
*
* @since 1.4.12
*/
@Parameter( property = "encoding", defaultValue = "${project.build.sourceEncoding}" )
private String encoding;

/**
* Maven project to interact with.
*/
Expand Down Expand Up @@ -148,10 +157,10 @@ public void execute() throws MojoFailureException
return;
}

Coverage coverage = Serializer.deserialize( coverageFile );
Coverage coverage = Serializer.deserialize( coverageFile, project.getBasedir() );
List<File> measurementFiles = Arrays.asList( IOUtils.findMeasurementFiles( dataDirectory ) );
scala.collection.Set<Tuple2<Object, String>> measurements =
IOUtils.invoked( JavaConverters.asScalaBuffer( measurementFiles ) );
IOUtils.invoked( CollectionConverters.asScala( measurementFiles ).toSeq(), encoding );
coverage.apply( measurements );

int branchCount = coverage.branchCount();
Expand All @@ -165,7 +174,7 @@ public void execute() throws MojoFailureException
invokedBranchesCount, branchCount, invokedStatementCount, statementCount ) );
if ( minimumCoverage > 0.0 )
{
String minimumCoverageFormatted = scoverage.DoubleFormat.twoFractionDigits( minimumCoverage );
String minimumCoverageFormatted = scoverage.domain.DoubleFormat.twoFractionDigits( minimumCoverage );
if ( is100( minimumCoverage ) && is100( coverage.statementCoveragePercent() ) )
{
getLog().info( "100% Coverage !" );
Expand Down
80 changes: 48 additions & 32 deletions src/main/java/org/scoverage/plugin/SCoveragePreCompileMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,19 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import edu.emory.mathcs.backport.java.util.Arrays;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.factory.ArtifactFactory;
import org.apache.maven.artifact.repository.ArtifactRepository;
import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
import org.apache.maven.artifact.resolver.ArtifactResolutionException;
import org.apache.maven.artifact.resolver.ArtifactResolver;
import org.apache.maven.artifact.versioning.ArtifactVersion;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.model.Dependency;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
Expand Down Expand Up @@ -286,20 +286,19 @@ else if ( "2.13".equals( resolvedScalaVersion ) || resolvedScalaVersion.startsWi

try
{
Artifact pluginArtifact = getScalaScoveragePluginArtifact( resolvedScalaVersion, scalaBinaryVersion );
List<Artifact> pluginArtifacts = getScalaScoveragePluginArtifacts( resolvedScalaVersion, scalaBinaryVersion );
Artifact runtimeArtifact = getScalaScoverageRuntimeArtifact( scalaBinaryVersion );

if ( pluginArtifact == null )
{
return; // scoverage plugin will not be configured
}

addScoverageDependenciesToClasspath( runtimeArtifact );

String arg = DATA_DIR_OPTION + dataDirectory.getAbsolutePath();
String _scalacOptions = quoteArgument( arg );
String addScalacArgs = arg;

arg = SOURCE_ROOT_OPTION + project.getBasedir().getAbsolutePath();
_scalacOptions = _scalacOptions + SPACE + quoteArgument( arg );
addScalacArgs = addScalacArgs + PIPE + arg;

if ( !StringUtils.isEmpty( excludedPackages ) )
{
arg = EXCLUDED_PACKAGES_OPTION + excludedPackages.replace( "(empty)", "<empty>" );
Expand All @@ -320,11 +319,10 @@ else if ( "2.13".equals( resolvedScalaVersion ) || resolvedScalaVersion.startsWi
addScalacArgs = addScalacArgs + PIPE + "-Yrangepos";
}

String _scalacPlugins =
String.format( "%s:%s:%s", pluginArtifact.getGroupId(), pluginArtifact.getArtifactId(),
pluginArtifact.getVersion() );
String _scalacPlugins = pluginArtifacts.stream()
.map(x -> String.format("%s:%s:%s", x.getGroupId(), x.getArtifactId(),x.getVersion())).collect(Collectors.joining(" "));

arg = PLUGIN_OPTION + pluginArtifact.getFile().getAbsolutePath();
arg = PLUGIN_OPTION + pluginArtifacts.stream().map(x -> x.getFile().getAbsolutePath()).collect(Collectors.joining(":"));
Copy link

@RPCMoritz RPCMoritz Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Make this platform independent:

Suggested change
arg = PLUGIN_OPTION + pluginArtifacts.stream().map(x -> x.getFile().getAbsolutePath()).collect(Collectors.joining(":"));
arg = PLUGIN_OPTION + pluginArtifacts.stream().map(x -> x.getFile().getAbsolutePath()).collect(Collectors.joining(String.valueOf(java.io.File.pathSeparatorChar)));

Copy link

Choose a reason for hiding this comment

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

@samantmaharaj could you amend this change to the PR?

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've pushed that change now.

addScalacArgs = addScalacArgs + PIPE + arg;

Properties projectProperties = project.getProperties();
Expand Down Expand Up @@ -371,6 +369,7 @@ else if ( "2.13".equals( resolvedScalaVersion ) || resolvedScalaVersion.startsWi
private static final String SCALA_LIBRARY_ARTIFACT_ID = "scala-library";

private static final String DATA_DIR_OPTION = "-P:scoverage:dataDir:";
private static final String SOURCE_ROOT_OPTION = "-P:scoverage:sourceRoot:";
private static final String EXCLUDED_PACKAGES_OPTION = "-P:scoverage:excludedPackages:";
private static final String EXCLUDED_FILES_OPTION = "-P:scoverage:excludedFiles:";
private static final String PLUGIN_OPTION = "-Xplugin:";
Expand Down Expand Up @@ -427,7 +426,7 @@ private void setProperty( Properties projectProperties, String propertyName, Str
}
}

private Artifact getScalaScoveragePluginArtifact( String resolvedScalaVersion, String scalaMainVersion )
private List<Artifact> getScalaScoveragePluginArtifacts(String resolvedScalaVersion, String scalaMainVersion )
throws ArtifactNotFoundException, ArtifactResolutionException
{
String resolvedScalacPluginVersion = scalacPluginVersion;
Expand All @@ -444,22 +443,39 @@ private Artifact getScalaScoveragePluginArtifact( String resolvedScalaVersion, S
}
}

try
// There are 3 cases depending on the version of scalac-scoverage-plugin
// * Version 2.0.0 onwards - 3 artifacts, plugin using resolvedScalaVersion
// * Version 1.4.2 - 1 artifact, plugin using resolvedScalaVersion falling back to scalaMainVersion
// * Version 1.4.1 older - 1 artifact, plugin using scalaMainVersion
//
final ArtifactVersion pluginArtifactVersion = new DefaultArtifactVersion(resolvedScalacPluginVersion);
if(pluginArtifactVersion.getMajorVersion() >= 2) {
Comment on lines +451 to +452
Copy link

@RPCMoritz RPCMoritz Feb 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
final ArtifactVersion pluginArtifactVersion = new DefaultArtifactVersion(resolvedScalacPluginVersion);
if(pluginArtifactVersion.getMajorVersion() >= 2) {
final ArtifactVersion pluginArtifactVersion = new DefaultArtifactVersion(resolvedScalacPluginVersion);
if(pluginArtifactVersion.getMajorVersion() > 2) {
throw new IllegalArgumentException("Currently Scala version 3 or above are not supported by this Maven plugin. Detected Scala version from scoverage-plugin-version: " + pluginArtifactVersion.getMajorVersion());
}
if(pluginArtifactVersion.getMajorVersion() == 2) {

I guess something like this would make the failure more evident?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given there's no support for Scala 3 at the moment, it would probably be better to implement that explicitly in another PR.

Correct, but we should still warn right now in the case that someone tries this with Scala 3. Or what is the behavior currently? The worse thing would be silent failure because it will just lead to more issues being created and confusion about why it's not working.

The current behaviour will be to fail to resolve the scoverage compiler plugin for Scala 3 as no plugins are currently published newer than 2.13.10

(Link to MVNRepository here)

This will cause a build failure. It's not as nice as explicitly notifying the user.

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 guess something like this would make the failure more evident?

The artifact version in the test you referenced is the version of the scoverage plugin. The scala version is calculated through resolveScalaVersion().

That function will default to an explicitly configured scala version or a search for the scala standard library. If I understand correctly, the Scala 3 standard library uses different coordinates from the earlier versions org.scala-lang:scala3-library.

If the user is depending on the scala 3 standard library resolveScalaVersion() will fail to detect the scala version resulting in a warning being emitted and the execution of the mojo being skipped.

Copy link

Choose a reason for hiding this comment

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

To me that sounds like acceptable behavior - @ckipp01 since an explicit scala-3-warning would need explicit scala-3 detection, I would leave that to a separate fix?

Copy link
Member

Choose a reason for hiding this comment

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

To me that sounds like acceptable behavior - @ckipp01 since an explicit scala-3-warning would need explicit scala-3 detection, I would leave that to a separate fix?

Sure that would be fine, but I'd for sure want to try to get that in before we attempt to do another release.

List<Artifact> resolvedArtifacts = new ArrayList<>();
resolvedArtifacts.add(getResolvedArtifact("org.scoverage", "scalac-scoverage-plugin_" + resolvedScalaVersion, resolvedScalacPluginVersion));
resolvedArtifacts.add(getResolvedArtifact("org.scoverage", "scalac-scoverage-domain_" + scalaMainVersion, resolvedScalacPluginVersion));
resolvedArtifacts.add(getResolvedArtifact("org.scoverage", "scalac-scoverage-serializer_" + scalaMainVersion, resolvedScalacPluginVersion));
return resolvedArtifacts;
} else if (pluginArtifactVersion.getMajorVersion() == 1 && pluginArtifactVersion.getMinorVersion() == 4 && pluginArtifactVersion.getIncrementalVersion() == 2)
{
return getResolvedArtifact(
"org.scoverage", "scalac-scoverage-plugin_" + resolvedScalaVersion,
resolvedScalacPluginVersion );
}
catch ( ArtifactNotFoundException | ArtifactResolutionException e )
{
getLog().warn( String.format( "Artifact \"org.scoverage:scalac-scoverage-plugin_%s:%s\" not found, " +
"falling back to \"org.scoverage:scalac-scoverage-plugin_%s:%s\"",
resolvedScalaVersion, resolvedScalacPluginVersion, scalaMainVersion, resolvedScalacPluginVersion ) );

// for scalac-scoverage-plugin versions up to 1.4.1
return getResolvedArtifact(
"org.scoverage", "scalac-scoverage-plugin_" + scalaMainVersion,
resolvedScalacPluginVersion );
try
{
return Collections.singletonList(
getResolvedArtifact("org.scoverage", "scalac-scoverage-plugin_" + resolvedScalaVersion, resolvedScalacPluginVersion )
);
}
catch ( ArtifactNotFoundException | ArtifactResolutionException e2 )
{
getLog().warn( String.format( "Artifact \"org.scoverage:scalac-scoverage-plugin_%s:%s\" not found, " +
"falling back to \"org.scoverage:scalac-scoverage-plugin_%s:%s\"",
resolvedScalaVersion, resolvedScalacPluginVersion, scalaMainVersion, resolvedScalacPluginVersion ) );
return Collections.singletonList(
getResolvedArtifact("org.scoverage", "scalac-scoverage-plugin_" + scalaMainVersion, resolvedScalacPluginVersion )
);
}
} else {
return Collections.singletonList(
getResolvedArtifact("org.scoverage", "scalac-scoverage-plugin_" + scalaMainVersion, resolvedScalacPluginVersion )
);
}
}

Expand Down
66 changes: 47 additions & 19 deletions src/main/java/org/scoverage/plugin/SCoverageReportMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.List;
import java.util.Locale;
import java.util.ResourceBundle;
import java.util.concurrent.atomic.AtomicInteger;

import org.apache.maven.doxia.module.xhtml.decoration.render.RenderingContext;
import org.apache.maven.doxia.sink.Sink;
Expand All @@ -46,17 +47,18 @@

import scala.Option;
import scala.Tuple2;
import scala.collection.JavaConverters;
import scala.collection.Seq;

import scoverage.Constants;
import scoverage.Coverage;
import scoverage.IOUtils;
import scoverage.Serializer;
import scoverage.report.CoberturaXmlWriter;
import scoverage.report.CoverageAggregator;
import scoverage.report.ScoverageHtmlWriter;
import scoverage.report.ScoverageXmlWriter;
import scala.collection.immutable.Seq;
import scala.jdk.javaapi.CollectionConverters;

import scoverage.domain.Constants;
import scoverage.domain.Coverage;
import scoverage.domain.Statement;
import scoverage.reporter.IOUtils;
import scoverage.serialize.Serializer;
import scoverage.reporter.CoberturaXmlWriter;
import scoverage.reporter.CoverageAggregator;
import scoverage.reporter.ScoverageHtmlWriter;
import scoverage.reporter.ScoverageXmlWriter;

/**
* Generates code coverage by unit tests report in forked {@code scoverage} life cycle.
Expand Down Expand Up @@ -419,13 +421,13 @@ private void generateReports()

File coverageFile = Serializer.coverageFile( dataDirectory );
getLog().info( String.format( "Reading scoverage instrumentation [%s]...", coverageFile.getAbsolutePath() ) );
Coverage coverage = Serializer.deserialize( coverageFile );
Coverage coverage = Serializer.deserialize( coverageFile, project.getBasedir() );

getLog().info( String.format( "Reading scoverage measurements [%s*]...",
new File( dataDirectory, Constants.MeasurementsPrefix() ).getAbsolutePath() ) );
List<File> measurementFiles = Arrays.asList( IOUtils.findMeasurementFiles( dataDirectory ) );
scala.collection.Set<Tuple2<Object, String>> measurements =
IOUtils.invoked( JavaConverters.asScalaBuffer( measurementFiles ) );
IOUtils.invoked( CollectionConverters.asScala( measurementFiles ).toSeq(), encoding );
coverage.apply( measurements );

getLog().info( "Generating coverage reports..." );
Expand All @@ -436,6 +438,8 @@ private void generateReports()
private void generateAggregatedReports()
throws MavenReportException
{
Coverage coverage = new Coverage();
AtomicInteger id = new AtomicInteger();
List<File> scoverageDataDirs = new ArrayList<File>();
List<File> sourceRoots = new ArrayList<File>();
MavenProject topLevelModule = null;
Expand All @@ -451,6 +455,33 @@ else if ( !module.getPackaging().equals( "pom" ) )
if ( scoverageDataDir.isDirectory() )
{
scoverageDataDirs.add( scoverageDataDir );
File coverageFile = Serializer.coverageFile(scoverageDataDir);
if (coverageFile.exists()) {
Coverage subCoverage = Serializer.deserialize(coverageFile, module.getBasedir());
List<File> measurementFiles = Arrays.asList( IOUtils.findMeasurementFiles( scoverageDataDir ) );
scala.collection.Set<Tuple2<Object, String>> measurements =
IOUtils.invoked( CollectionConverters.asScala( measurementFiles ).toSeq(), encoding );
subCoverage.apply( measurements );
subCoverage.statements().foreach(statement -> {
int statementId = id.getAndIncrement();
Statement copy = statement.copy(
statement.location(),
statementId,
statement.start(),
statement.end(),
statement.line(),
statement.desc(),
statement.symbolName(),
statement.treeName(),
statement.branch(),
statement.count(),
statement.ignored(),
statement.tests()
);
coverage.add(copy);
return null;
});
}

File sourceRootsFile = new File( scoverageDataDir, "source.roots" );
if ( sourceRootsFile.isFile() )
Expand Down Expand Up @@ -510,9 +541,6 @@ else if ( !module.getPackaging().equals( "pom" ) )
mkdirs( topLevelModuleOutputDirectory );
mkdirs( topLevelModuleXmlOutputDirectory );

Coverage coverage =
CoverageAggregator.aggregatedCoverage( JavaConverters.asScalaBuffer( scoverageDataDirs ).toSeq() );

getLog().info( "Generating coverage aggregated reports..." );
writeReports( coverage, sourceRoots, topLevelModuleXmlOutputDirectory, topLevelModuleXmlOutputDirectory,
topLevelModuleOutputDirectory );
Expand All @@ -522,13 +550,13 @@ else if ( !module.getPackaging().equals( "pom" ) )
private void writeReports( Coverage coverage, List<File> sourceRoots, File coberturaXmlOutputDirectory,
File scoverageXmlOutputDirectory, File scoverageHtmlOutputDirectory )
{
Seq<File> sourceRootsAsScalaSeq = JavaConverters.asScalaBuffer( sourceRoots );
Seq<File> sourceRootsAsScalaSeq = CollectionConverters.asScala( sourceRoots ).toSeq();

new CoberturaXmlWriter( sourceRootsAsScalaSeq, coberturaXmlOutputDirectory ).write( coverage );
new CoberturaXmlWriter( sourceRootsAsScalaSeq, coberturaXmlOutputDirectory, Option.<String>apply( encoding ) ).write( coverage );
getLog().info( String.format( "Written Cobertura XML report [%s]",
new File( coberturaXmlOutputDirectory, "cobertura.xml" ).getAbsolutePath() ) );

new ScoverageXmlWriter( sourceRootsAsScalaSeq, scoverageXmlOutputDirectory, false ).write( coverage );
new ScoverageXmlWriter( sourceRootsAsScalaSeq, scoverageXmlOutputDirectory, false, Option.<String>apply( encoding ) ).write( coverage );
getLog().info( String.format( "Written XML coverage report [%s]",
new File( scoverageXmlOutputDirectory, "scoverage.xml" ).getAbsolutePath() ) );

Expand Down