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

Include runtime dependencies in aggregated reports #498

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@okutane

okutane commented Mar 16, 2017

It's necessary to also check coverage of runtime dependencies.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Mar 16, 2017

Member

@okutane Can you please describe your use case a bit and how your modules are structured? So far we assumed that there will be a separate module for the aggregate report which explicitely declares dependencies to include into the report.

Member

marchof commented Mar 16, 2017

@okutane Can you please describe your use case a bit and how your modules are structured? So far we assumed that there will be a separate module for the aggregate report which explicitely declares dependencies to include into the report.

@okutane

This comment has been minimized.

Show comment
Hide comment
@okutane

okutane Mar 16, 2017

Hello! I've used project structure similar to following.

Initial structure

root pom:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
...
    <modules>
        <module>api</module>
        <module>core</module>
        <module>api-impl1</module>
        <module>api-impl2</module>
    </modules>

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <configuration>
                    ...
                    <includes>
                        <include>**/AllTests.class</include>
                    </includes>
                    <argLine>${argLine}</argLine>
                </configuration>
            </plugin>
            <plugin>
                <groupId>org.jacoco</groupId>
                <artifactId>jacoco-maven-plugin</artifactId>
                <version>0.7.9</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>prepare-agent</goal>
                        </goals>
                    </execution>
                    <execution>
                        <id>report</id>
                        <phase>test</phase>
                        <goals>
                            <goal>report</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>
...
</project>

api just declares some interfaces, api-implN implements them in a different ways and core binds everything together (now only by integration tests).

core pom:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <parent>...</parent>

    ...
    <dependencies>
        <dependency>
            <groupId>...</groupId>
            <artifactId>api</artifactId>
            <version>...</version>
        </dependency>
    </dependencies>

    <profiles>
        <profile>
            <id>impl1</id>
            <dependencies>
                <dependency>
                    <groupId>...</groupId>
                    <artifactId>api-impl1</artifactId>
                    <version>...</version>
                    <scope>runtime</scope>
                </dependency>
            </dependencies>
        </profile>
    </profiles>
</project>

How I used it

mvn clean test -P impl1

Since the tests are only in core module I've got everything built, executed and tested, but the coverage report was only for the core module itself.

How I fixed it

Then I switched to report-aggregate and had coverage only for api. data for api-impl1 has been ignored because the scope wasn't either of 'compile' or 'test'. as for the core itself, I guess the data has been ignored because aggregate reports don't include data for the current module. Anyway, this all was fixed by introduction of tests module which included everything needed as compile dependencies. sanity-tool/sanity@cbcdd0d

Pull request

It's OK to me to keep that structure, but I would like to switch api-impl scopes to runtime and still have the coverage in integration tests.

okutane commented Mar 16, 2017

Hello! I've used project structure similar to following.

Initial structure

root pom:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
...
    <modules>
        <module>api</module>
        <module>core</module>
        <module>api-impl1</module>
        <module>api-impl2</module>
    </modules>

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <configuration>
                    ...
                    <includes>
                        <include>**/AllTests.class</include>
                    </includes>
                    <argLine>${argLine}</argLine>
                </configuration>
            </plugin>
            <plugin>
                <groupId>org.jacoco</groupId>
                <artifactId>jacoco-maven-plugin</artifactId>
                <version>0.7.9</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>prepare-agent</goal>
                        </goals>
                    </execution>
                    <execution>
                        <id>report</id>
                        <phase>test</phase>
                        <goals>
                            <goal>report</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>
...
</project>

api just declares some interfaces, api-implN implements them in a different ways and core binds everything together (now only by integration tests).

core pom:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <parent>...</parent>

    ...
    <dependencies>
        <dependency>
            <groupId>...</groupId>
            <artifactId>api</artifactId>
            <version>...</version>
        </dependency>
    </dependencies>

    <profiles>
        <profile>
            <id>impl1</id>
            <dependencies>
                <dependency>
                    <groupId>...</groupId>
                    <artifactId>api-impl1</artifactId>
                    <version>...</version>
                    <scope>runtime</scope>
                </dependency>
            </dependencies>
        </profile>
    </profiles>
</project>

How I used it

mvn clean test -P impl1

Since the tests are only in core module I've got everything built, executed and tested, but the coverage report was only for the core module itself.

How I fixed it

Then I switched to report-aggregate and had coverage only for api. data for api-impl1 has been ignored because the scope wasn't either of 'compile' or 'test'. as for the core itself, I guess the data has been ignored because aggregate reports don't include data for the current module. Anyway, this all was fixed by introduction of tests module which included everything needed as compile dependencies. sanity-tool/sanity@cbcdd0d

Pull request

It's OK to me to keep that structure, but I would like to switch api-impl scopes to runtime and still have the coverage in integration tests.

@okutane okutane closed this Mar 16, 2017

@okutane okutane reopened this Mar 16, 2017

@okutane

This comment has been minimized.

Show comment
Hide comment
@okutane

okutane Mar 16, 2017

@marchof, I guess I can add a few parameters for report-aggregate goal if you don't want the default behavior to be changed.

okutane commented Mar 16, 2017

@marchof, I guess I can add a few parameters for report-aggregate goal if you don't want the default behavior to be changed.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Mar 20, 2017

Member

@okutane Thanks for the detailed explanation! In your scenario the "core" module is a good place to also create the coverage report. Also I think it is reasonable to include runtime dependencies for the scope. For the exec file collection I'm not sure though but at least this will not hurt.

@Godin WDYT? Update documentation and include this PR?

Member

marchof commented Mar 20, 2017

@okutane Thanks for the detailed explanation! In your scenario the "core" module is a good place to also create the coverage report. Also I think it is reasonable to include runtime dependencies for the scope. For the exec file collection I'm not sure though but at least this will not hurt.

@Godin WDYT? Update documentation and include this PR?

marchof added a commit that referenced this pull request Mar 20, 2017

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Mar 20, 2017

Member

@okutane Finalized paperwork for this in PR #502.

Member

marchof commented Mar 20, 2017

@okutane Finalized paperwork for this in PR #502.

@marchof marchof closed this Mar 20, 2017

@Godin Godin added this to the 0.7.10 milestone Mar 25, 2017

@jacoco jacoco locked as resolved and limited conversation to collaborators Jan 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.