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

Support checking of test classes #5

Closed
THausherr opened this issue Jul 20, 2015 · 13 comments · Fixed by #80 or #81
Closed

Support checking of test classes #5

THausherr opened this issue Jul 20, 2015 · 13 comments · Fixed by #80 or #81
Milestone

Comments

@THausherr
Copy link

Our problem (see at https://issues.apache.org/jira/browse/PDFBOX-2891) is that we'd like test classes to be checked too. This was proposed before:
http://mojo.10943.n7.nabble.com/Fwd-animal-sniffer-for-maven-test-classes-td43156.html
I don't know if "binkley" ever followed up on his patch proposal. I built the plugin locally and used this pom:

<plugin>
    <groupId>org.codehaus.mojo</groupId>
    <artifactId>animal-sniffer-maven-plugin</artifactId>
    <version>1.15-SNAPSHOT</version>
    <executions>
        <execution>
            <id>check-java-version</id>
            <phase>test</phase>
            <goals>
                <goal>check</goal>
            </goals>
            <configuration>
                <signature>
                    <groupId>org.codehaus.mojo.signature</groupId>
                    <artifactId>java15</artifactId>
                    <version>1.0</version>
                </signature>
            </configuration>
        </execution>
    </executions>
</plugin>

Here's the "bad" code:

package animalsnifferdemo;

import org.junit.Test;

public class MainTest
{
    @Test
    public void testMain()
    {
        String str = "huhu";
        boolean empty = str.isEmpty();
        System.out.println(empty);
    }
}

The error (the use of isEmpty() which is not supported by JDK5) was not detected.

olivierdagenais pushed a commit to jenkinsci/tfs-plugin that referenced this issue Jan 12, 2016
Apparently, animal-sniffer does not yet check test code:
mojohaus/animal-sniffer#5
olivierdagenais pushed a commit to jenkinsci/tfs-plugin that referenced this issue Jan 13, 2016
Apparently, animal-sniffer does not yet check test code:
mojohaus/animal-sniffer#5
@sharwell
Copy link

I was hoping to use this project as a way to finally move away from -bootclasspath (and requiring users to download a very old JDK), but until this issue is fixed that will not be possible.

@karianna
Copy link

Would love to see this resolved as well

@nmatt
Copy link

nmatt commented Feb 20, 2019

Ditto.

@famod
Copy link
Contributor

famod commented Feb 16, 2020

@olamy I'd take a stab on this, but first we need to decide whether this should become a new mojo or not.

Since the usage site recommends <phase>test</phase>, we could simply extend the existing mojo instead of adding a new one.

@olamy
Copy link
Member

olamy commented Mar 15, 2020

@famod yes try that. sorry for long response time

famod added a commit to famod/animal-sniffer that referenced this issue Mar 20, 2020
famod added a commit to famod/animal-sniffer that referenced this issue Mar 20, 2020
@famod
Copy link
Contributor

famod commented Mar 20, 2020

@olamy Please see PR #80. Thanks!

@olamy olamy closed this as completed in #80 Apr 7, 2020
olamy pushed a commit that referenced this issue Apr 7, 2020
* Bump enforcer-plugin version to support mvn -f or -pl

* Fix #5: Check test classes

* Fix #5: align phase and dep resolution to test class check
@THausherr
Copy link
Author

THausherr commented Apr 7, 2020

I then built locally and my test code is detected. However I tried to use the snapshot on one of my projects and it failed. This code reproduces the problem:



import org.junit.Assert;
import org.junit.Test;


public class MainTest
{
    
    public MainTest()
    {
    }

    @Test
    public void testMainBad2()
    {
        int totalAmountRead = 10;
        Assert.assertEquals(10, totalAmountRead);
    }
}

I get

--- animal-sniffer-maven-plugin:1.19-SNAPSHOT:check (check-java-version) @ MavenAnimalSnifferTest ---
Checking unresolved references to org.codehaus.mojo.signature:java18:1.0
XXXX\MavenAnimalSnifferTest\src\test\java\com\mycompany\mavenanimalsniffertest\MainTest.java:NNN: Undefined reference: void org.junit.Assert.assertEquals(long, long)

here's the pom, this time all set for java 8:

<?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>
    <groupId>com.mycompany</groupId>
    <artifactId>MavenAnimalSnifferTest</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>jar</packaging>
    <dependencies>
        <dependency>
            <groupId>junit</groupId>
            <artifactId>junit</artifactId>
            <version>4.13</version>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.hamcrest</groupId>
            <artifactId>hamcrest-core</artifactId>
            <version>1.3</version>
            <scope>test</scope>
        </dependency>
    </dependencies>
    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <maven.compiler.source>1.8</maven.compiler.source>
        <maven.compiler.target>1.8</maven.compiler.target>
    </properties>
    
    <build>
        <plugins>
            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>animal-sniffer-maven-plugin</artifactId>
                <version>1.19-SNAPSHOT</version>
                <executions>
                    <execution>
                        <id>check-java-version</id>
                        <phase>test</phase>
                        <goals>
                            <goal>check</goal>
                        </goals>
                        <configuration>
                            <signature>
                                <groupId>org.codehaus.mojo.signature</groupId>
                                <artifactId>java18</artifactId>
                                <version>1.0</version>
                            </signature>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>
</project>

@famod
Copy link
Contributor

famod commented Apr 7, 2020

Confirmed. I get loads of such errors in https://github.com/vackosar/gitflow-incremental-builder

@famod
Copy link
Contributor

famod commented Apr 7, 2020

Ok, first problem found! At least scope test is missing here: https://github.com/mojohaus/animal-sniffer/blob/master/animal-sniffer-maven-plugin/src/main/java/org/codehaus/mojo/animal_sniffer/maven/CheckSignatureMojo.java#L387-L388

That reduces the number of false negatives but it is not sufficient. Investigating...

@famod
Copy link
Contributor

famod commented Apr 7, 2020

Ok, I found the second problem. I'll create another PR but that will take some time because I need to look at the enforcer rule and I'd also like to add a test.

@olamy Can we reopen this? Thanks.

@famod
Copy link
Contributor

famod commented Apr 7, 2020

PR #81 should fix the false negatives.

@famod
Copy link
Contributor

famod commented May 10, 2020

@THausherr This should now work as expected (built from master).

@THausherr
Copy link
Author

Yes, thank you. I was able to successfully verify Apache PDFBox trunk, 2.0 branch and 1.8 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants