Coverage checking causes tests to run twice

[jqno]

I'd like to check the minimum test coverage level in the mvn verify step of my build. I followed the instructions on the webpage: https://github.com/scoverage/scoverage-maven-plugin#checking-minimum-test-coverage-level

This works fine, but it also causes our tests (and scala-fmt) to run twice, which is very annoying and time-consuming.

In scoverage/scoverage-maven-samples#2 you say it's better to split the call to maven into two separate ones, for example mvn clean verify -DskipTests and mvn scoverage:check, which also works I suppose, but is also very unsatisfying: team members will have to learn to use Maven in a non-standward way if they want to verify their work before submitting it to the CI server. (Either having to remember both incantations and having to remember to type both, or having to use a shell script and remembering not to use Maven directly.)

In the mean time, JaCoCo can do coverage level checking directly from mvn verify without running the tests twice, so our team members are very much used to only having to type mvn verify or mvn install to execute all of the checks that the CI server also executes.

Would it be possible to add this behavior to scoverage-maven-plugin as well? It would make Scala adoption on our team so much easier.

[gslowikowski]

Hi @jqno. Sorry for late reply.

JaCoCo uses Java agent to instruments classes while loading them. Scoverage instruments classed during compilation. Instrumented classes are written to disk.

I've implemented plugin in this way to avoid deploying applications with instrumented classes. This is very common problem reported by sbt-scoverage plugin users.

I know, how to do what you want, but I don't know, how to do it in a safe way. How to prevent users from accidentally deploying instrumented artifacts.

[gslowikowski]

OK, maybe I found a solution.

Checkout and build add-additionalForkedProjectProperties-configuration-parameter branch locally.

Change plugin version to 1.4.0-SNAPSHOT add two things to the project (in root module):

1. project property:

    <properties>
        ...
        <skipTests>true</skipTests>
    </properties>

2. scoverage plugin configuration parameter:

                <plugin>
                    <groupId>org.scoverage</groupId>
                    <artifactId>scoverage-maven-plugin</artifactId>
                    <version>${scoverage.plugin.version}</version>
                    <configuration>
                        ...
                        <additionalForkedProjectProperties>skipTests=false</additionalForkedProjectProperties>
                    </configuration>
                </plugin>

Tests will be executed only in forked scoverage life cycle.

[jqno]

Hi!

Thanks for the background about agents vs instrumented bytecode, that explains a lot.

I've tried out your solution on our codebase, and it works allright.

A question though: can you have multiple settings inside <additionalForkedProjectProperties>? While I'm at it, I'd like to disable some other plugins as well, for example the scalafmt pluging which is also run twice. If that's not possible it's not such a big deal, scalafmt is fast and not having to run the tests twice is already such a great improvement.

Thank you!

[gslowikowski]

Yes, separate them with semicolon:

<additionalForkedProjectProperties>skipTests=false;prop2=val2</additionalForkedProjectProperties>

[jqno]

Awesome!

[jozic]

@gslowikowski
Can this new additionalForkedProjectProperties be released? at least as a milestone?

[gslowikowski]

Did you guys test your projects with Scoverage plugin version from add-additionalForkedProjectProperties-configuration-parameter branch?

Does it work without problems?

[jozic]

I didn't but AFAIU it worked for @jqno based on his comment

I've tried out your solution on our codebase, and it works allright.

[jqno]

Indeed. In fact, I'd also like a release if possible 😄

[jqno]

Is there something I can do to help get this released?

[gslowikowski]

Hi @jqno

Actually yes. I thought that there should be documentation page about this feature. It's not straightforward, so should be described.

I don't have enough time to sit down, think about it and write it down in a clear, understandable way (I'm not good when it comes to documentation, especially in English).

Could you help me with this task?

[jqno]

Of course! I'll send a PR your way, hopefully some time this week. I'll just add a little chapter to the README.

Do we also need an update to the sample projects?

[jqno]

I've opened a PR: #67. Please let me know if there is anything that I didn't describe correctly or in sufficient detail, or if there's anything else that can be improved.

[NoamShaish]

A release will be great 👍

[gslowikowski]

Hi @jqno . I've merged my branch to master.

Please rebase your branch to master (I've already changed base branch in PR). Can you add #61 - prefix in a commits and PR name so everything will be linked to this issue?

[jqno]

Done!

[gslowikowski]

Thank you guys for your help and patience.

1.4.0-SNAPSHOT snapshot deployed. Verify if it works, please.

If everything is OK I will perform release very soon.

[jqno]

I tested it, and it (still) works! 👍

[gslowikowski]

Version 1.4.0-M5 released. M5, because it supports Scala 2.13.0-M5.

[jqno]

Awesome, thanks!

[NoamShaish]

Thats great, but doesnt seem to appear on maven central

[gslowikowski]

It's there - http://repo1.maven.org/maven2/org/scoverage/scoverage-maven-plugin/1.4.0-M5/

Probably https://mvnrepository.com needs some time to refresh.

[NoamShaish]

impatience me 👿

[jozic]

Version 1.4.0-M5 released. M5, because it supports Scala 2.13.0-M5.

2.13.0-RC1 is already out

Can we please cross publish for it?

[gslowikowski]

It's not possible. I found some incompatibilities between M5 and RC1.

[gslowikowski]

Version 1.4.0-RC1 was released. Please upgrade. Version 1.4.0-M5 has a bug #72.

[kitbellew]

@jqno @gslowikowski which version of maven is this new feature known to work? or is there an example of the pom.xml which shows how this works?

we are running maven 3.6.1, the plugins and the properties are defined in a parent pom, and this new configuration property is not affecting the run. "mvn -X" shows identical plugin configurations for scalatest and scalafmt in regular and scoverage-forked versions.

[jqno]

We're using Maven 3.6.1 as well, at first with Scala 2.12 and currently Scala 2.13. It works fine for us.

Did you forget to add <skipTests>true</skipTests> to your <properties> section?

[kitbellew]

@jqno thanks; i did include the skipTests=true. unfortunately, the scoverage run has the same exact plugin configurations (according to mvn -X), and ends up skipping tests as well, while running protoc, scalafmt and scalastyle etc.

hence my question about your exact pom configuration which shows how this property override might work. perhaps my pom is just too complex (with parent pom, profiles, plugin management etc.), but i'd like to have a working example.

[jqno]

Our build is quite complex as well, with a parent, a bunch of plugins including scalafmt and scalastyle, etc. No profiles though.

Not sure if I can help you any further. These issues are always nasty to figure out. I think I'd just start over with a clean pom.xml and start adding complexity until it doesn't work anymore. At least then you know for sure where the culprit is.

[berardino]

I ran into this issue as well, posting what worked for me, hopefully, can help others.
I have a fairly complex multi-module scala project that uses scoverage and scalatest.
I ran into two issues:
1 - tests ran twice (this issue)
2 - no tests were discovered by scalatest but the first module

The first issue is solved by setting both properties <maven.test.skip>true</maven.test.skip> and <skipTests>true</skipTests> and then set them to false in additionalForkedProjectProperties.
The second issue is solved by making sure that scalatest is configured to fork.

This the configurations I have in the parent pom:

<properties>
    <!-- Apparently you need to set these two props -->
    <maven.test.skip>true</maven.test.skip>
    <skipTests>true</skipTests>
</properties>    

<plugin>
    <groupId>org.scoverage</groupId>
    <artifactId>scoverage-maven-plugin</artifactId>
    <configuration>
        <additionalForkedProjectProperties>maven.test.skip=false;skipTests=false</additionalForkedProjectProperties>
    </configuration>
</plugin>

<plugin>
    <groupId>org.scalatest</groupId>
    <artifactId>scalatest-maven-plugin</artifactId>
    <configuration>
        <filereports>WDFTestSuite.txt</filereports>        
        <skipTests>false</skipTests>
        <!-- If set to never, scalatest, in a multi modules project, will only discover tests for the first maven module oO -->
        <forkMode>once</forkMode>
    </configuration>
</plugin>    

[gliu-nova]

none of the solutions above worked for me but this did:
remove scoverage:report goal, then run
mvn scoverage:report AFTER your mvn clean test/verify

[nblagodarnyi]

Unfortunately, skip/non-skip options set by additionalForkedProjectProperties are ignored when using explicit option setting in command line. For example, if I run mvn clean verify -DskipTests=false settings in <additionalForkedProjectProperties>skipTests=true</additionalForkedProjectProperties> are not considered and tests run twice.