Add test filter mechanism

[Link184]

Add a possibility to run all the tests by default using skippy without editing the tests(add rules to junit4 or annotations to junit5).

We can hide this feature behind a feature flag:

build.gradle

apply "io.skippy"

skippy {
    enableInAllTheTests = true/false // enable/disable filter logic in all the gradle test tasks
}

#189

[Link184]

@fmck3516 what is the best way to get a list of all the tests that we want to skip?

[Link184]

@fmck3516 I want to pass that list here:

testToSkipList.forEach(test::exclude)

[fmck3516]

skippy {
    enableInAllTheTests = true/false // enable/disable filter login in all the gradle test tasks
}

I really like the idea to enable Skippy per default like that. For JUnit 5, this could be some syntactic sugar on top of Automatic Extension Registration

I still have no idea how to do that for JUnit 4 w/o manipulating the bytecode of the tests.

[fmck3516]

@fmck3516 what is the best way to get a list of all the tests that we want to skip?

Shooting from the hip: I think some kind of predicate that the user can provide. The implementation (e.g., match against a list or matching via regular expressions) could be left as exercise to the user.

[Link184]

I did a small investigation, I am not sure if I am on the right way so let me know if I am missing something :)

As far as I understood here we are taking the decision to skip the test or not: SkippyTestApi.testNeedsToBeExecuted(Class<> clazz), also TestImpactAnalysis.predict() looks important.

Looks like it is easy to change/overload the first parameter of TestImpactAnalysis.predict() from Class to String.
We can parse all the compiled test file names and pass them to TestImpactAnalysis.predict(), if the response contains Prediction.SKIP we can exclude it using gradle api(Test.exclude(String testFileName))

[fmck3516]

I did a small investigation, I am not sure if I am on the right way so let me know if I am missing something :)

As far as I understood here we are taking the decision to skip the test or not: SkippyTestApi.testNeedsToBeExecuted(Class<> clazz), also TestImpactAnalysis.predict() looks important.

Looks like it is easy to change/overload the first parameter of TestImpactAnalysis.predict() from Class to String. We can parse all the compiled test file names and pass them to TestImpactAnalysis.predict(), if the response contains Prediction.SKIP we can exclude it using gradle api(Test.exclude(String testFileName))

Have you seen:

• https://www.skippy.io/docs/#opt-out-mechanism

• skippy/skippy-core/src/main/java/io/skippy/core/PredictionModifier.java

  Line 42 in 13403b0

  public interface PredictionModifier {

I already anticipated that users will want to have fine-grained control over when to apply Skippy.

Would you be able to utilize this extension point?

I'm open for other proposals if that's not a good fit. Just wanted to make sure it's on your radar.

[fmck3516]

Looks like it is easy to change/overload the first parameter of TestImpactAnalysis.predict() from Class to String. We can parse all the compiled test file names and pass them to TestImpactAnalysis.predict(), if the response contains Prediction.SKIP we can exclude it using gradle api(Test.exclude(String testFileName))

I've added a regression tests for the duplicate class names across output folders scenario: https://github.com/skippy-io/skippy-regression-suite/tree/main/test-projects/junit5-duplicate-class-names

        var loggingLog = projectDir.toPath().resolve(".skippy").resolve("logging.log");
        assertThat(readAllLines(loggingLog, StandardCharsets.UTF_8).toArray()).containsExactlyInAnyOrder(
                "Mapping class build/classes/java/debugTest/com.example.BarTest to AnalyzedTest[build/classes/java/debugTest/com.example.BarTest]",
                "Mapping class build/classes/java/debugTest/com.example.FooTest to AnalyzedTest[build/classes/java/debugTest/com.example.FooTest]",
                "Mapping class build/classes/java/releaseTest/com.example.BarTest to AnalyzedTest[build/classes/java/releaseTest/com.example.BarTest]",
                "Mapping class build/classes/java/releaseTest/com.example.FooTest to AnalyzedTest[build/classes/java/releaseTest/com.example.FooTest]",
                "Mapping class build/classes/java/test/com.example.BarTest to AnalyzedTest[build/classes/java/test/com.example.BarTest]",
                "Mapping class build/classes/java/test/com.example.FooTest to AnalyzedTest[build/classes/java/test/com.example.FooTest]"
        );

https://github.com/skippy-io/skippy-regression-suite/blob/435eab7173b85227f22e3f688ab7fd3b3e561d84/src/test/java/io/skippy/test/gradle/JUnit5DuplicateClassNamesTest.java#L139C1-L147C11

If you reason on the class name alone, you won't be able to map the test that is being executed to a test in test-impact-analysis.json without ambiguity. The benefit of the class file: You have the name, but you can also get the location.

[fmck3516]

@fmck3516 what is the best way to get a list of all the tests that we want to skip?

I'm thinking something like https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/util/PatternFilterable.html that is used by Gradle to filter tests:

test {
   include 'org/foo/**'
   exclude 'org/boo/**'
}

Possilbe Skippy equivalent:

skippy {    
    include 'com.foo.**'
    exclude 'com.bar.**'
}

Optionally with support for output folders:

skippy {    
    include 'build/classes/java/test:**.*'
    exclude 'build/classes/java/integrationTest:**.*'
}

The SkippyPlugin can pass the configuration as JVM parameter to the tests:

final class SkippyPlugin implements org.gradle.api.Plugin<Project> {

    @Override
    public void apply(Project project) {
        ...
        project.afterEvaluate(action -> {
            ...
            action.getTasks().withType(Test.class, testTask -> {
               ...
                testTask.jvmArgs("-Dskippy.filter=...");
            });
        });
    }

}

Reason I'm thinking about JVM parameter: This would allow me to use the same approach for the Gradle (Java) & Maven plugin.

[fmck3516]

That would definitely be 1000x more user friendly than registration of a custom PredictionModifier class (I dislike the name anyway).

[Link184]

Reason I'm thinking about JVM parameter: This would allow me to use the same approach for the Gradle (Java) & Maven plugin.

I just tested, it is possible to obtain the same behavior without involving the AGP:

project.getTasks().withType(Test.class).all(test -> {
            testToSkipList.forEach(test::exclude)
});

So it may work on android and jvm.

---

I'm thinking something like https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/util/PatternFilterable.html that is used by Gradle to filter tests:

I am not sure I understood what is the benefit.

[fmck3516]

Reason I'm thinking about JVM parameter: This would allow me to use the same approach for the Gradle (Java) & Maven plugin.

I just tested, it is possible to obtain the same behavior without involving the AGP:

project.getTasks().withType(Test.class).all(test -> {
            test.exclude("**/*");
});

In the above, don't you disable the tests alltogether? Instead of disabling Skippy?

[Link184]

In the above, don't you disable the tests alltogether? Instead of disabling Skippy?

sorry for confusion, I will remove that wildcard. The idea is that we can use the same logic for jvm and android, we just depend on gradle api, not on AGP api

[fmck3516]

In the above, don't you disable the tests alltogether? Instead of disabling Skippy?

sorry for confusion, I will remove that wildcard. The idea is that we can use the same logic for jvm and android, we just depend on gradle api, not on AGP api

Cool. Can you sketch how Skippy would consume the info that you pass to the test task? That would clear up things for me quite a bit.

[Link184]

Cool. Can you sketch how Skippy would consume the info that you pass to the test task? That would clear up things for me quite a bit.

In my head the consumer is the task, not skippy. The data that I want to pass to the task must come from skippy. So skippy has an algorithm to calculate which test must be skipped, I want to pass that list to the test task.

So the only thing I need and I don't know how to obtain is the list of tests that we want to skip.

As far as I understood right now skippy is doing all the calculations lazily, when the test is executed. I need to have everything already calculated before running the tests, to be able to filter the tests.

[fmck3516]

Ah, I was just about to stop the comment ping-pong and suggest a quick call.

I think I understand what you try to do. I will whip something up to fill in the missing pieces.

[fmck3516]

Now that I understand what you try to do: That's a really interesting approach to overcome the fact that JUnit 4 doesn't have a counterpart to JUnit 5's automatic extension registration.

[fmck3516]

K, I've sketched how to generate exclusions based on Skippy's predictions: https://github.com/skippy-io/skippy/pull/199/files

I didn't touch the ClassFileCollector interface and did some quick and dirty reverse-engineering of the Class objects. Have to think about whether the ClassFileCollector should return Class objects or -as you suggested- to provide overloads of TestImpactAnalysis#predict that can operate without a Class file. I now see the use case for that.

1st build:

./gradlew clean skippyClean test --console=plain  

> Task :test
com.example.LeftPadderTest > testPadLeft PASSED
com.example.RightPadderTest > testPadRight PASSED

> Task :skippyAnalyze
...

2nd build:

./gradlew test --rerun --console=plain 

> Task :test
Adding exclusion **/com/example/LeftPadderTest.*
Adding exclusion **/com/example/RightPadderTest.*

> Task :skippyAnalyze
...

Unfortunately, this only solves the issue partially: The test cases still need to use the Skippy rule. Otherwise the first build will create a test-impact-analysis.json with empty tests object. This in turn will result in the 2nd build not to exclude any test cases.

The above replaces the SkipOrExecuteRule. But there is still the CoverageFileRule.

Do you see a way to call the SkippyTestApi#before and SkippyTestApi#after callbacks w/o the need for a test rule?

[fmck3516]

I played around with Gradle's AbstractTestTask#addTestListener - without much success since the listener and the tests are executed in separate JVMs. That's why I can't use the TestListener to call the SkippyTestApi#before and SkippyTestApi#after callbacks.

[Link184]

I played around with Gradle's AbstractTestTask#addTestListener - without much success since the listener and the tests are executed in separate JVMs. That's why I can't use the TestListener to call the SkippyTestApi#before and SkippyTestApi#after callbacks.

The test listener is triggered when the tests are running. It is too late for us, we want to know the skippable items before we are running the tests. Filtering is applied before running(I am not 100% sure yet tbh ;) ), that means we must have the list before.

[fmck3516]

I played around with Gradle's AbstractTestTask#addTestListener - without much success since the listener and the tests are executed in separate JVMs. That's why I can't use the TestListener to call the SkippyTestApi#before and SkippyTestApi#after callbacks.

The test listener is triggered when the tests are running. It is too late for us, we want to know the skippable items before we are running the tests. Filtering is applied before running(I am not 100% sure yet tbh ;) ), that means we must have the list before.

The TestListener was an attempt to replace the CoverageFileRule (not the SkipOrExecuteRule).

[Link184]

The TestListener was an attempt to replace the CoverageFileRule (not the SkipOrExecuteRule).

The listener is not working in all the cases? Or only under some specific circumstances?

[fmck3516]

The TestListener was an attempt to replace the CoverageFileRule (not the SkipOrExecuteRule).

The listener is not working in all the cases? Or only under some specific circumstances?

What you suggested takes care of the functionality that is provided by the SkipOrExecuteRule. But: In order to make proper predictions that are then fed as exclusions to the test tasks, we need test impact data.

And I still have not figured out how to generate coverage for JUnit 4 w/o the need for a @TestRule within the test classes.

[fmck3516]

The TestListener was an attempt to replace the CoverageFileRule (not the SkipOrExecuteRule).

The listener is not working in all the cases? Or only under some specific circumstances?

It doesn't work at all, since it is executed in a different JVM. Hence, it does not see any coverage data from the tests.

[Link184]

Can you please share some code because I don't really understand how coverage can help us with test filter mechanism?

[fmck3516]

Can you please share some code because I don't really understand how coverage can help us with test filter mechanism?

Sure - I will whip up a sample project to demonstrate the issue I'm talking about some time in the evening.

[fmck3516]

Build skippy branch issues/189-exclusion-poc:

git clone [email protected]:skippy-io/skippy.git
cd skippy
git checkout issues/189-exclusion-poc
./gradlew publishToMavenLocal  -x test

Checkout skippy-tutorials branch issues/189-exclusion-poc:

cd ..
git clone [email protected]:skippy-io/skippy-tutorials.git
cd skippy-tutorials
git checkout issues/189-exclusion-poc

Go to getting-started-with-skippy-and-junit4 and run the tests:

cd getting-started-with-skippy-and-junit4

 ./gradlew test --rerun
 
com.example.LeftPadderTest > testPadLeft PASSED
com.example.RightPadderTest > testPadRight PASSED

Next, check the content of test-impact-analysis.json:

tail .skippy/test-impact-analysis.json
			"name": "com.example.TestConstants",
			"path": "com/example/TestConstants.class",
			"outputFolder": "build/classes/java/test",
			"hash": "119F463C"
		}
	},
    "tests": [

    ]
}

The key here is the empty tests array. It is empty because the CoverageFileRule is not executed. As a result, the SkippyPlugin will never generate any exclusions:

./gradlew test --rerun

> Task :test
Exclusions that will be added: 0

Next, add the Skippy rule to LeftPadderTest:

vi src/test/java/com/example/LeftPadderTest.java

package com.example;

import io.skippy.junit4.Skippy;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;

import static org.junit.Assert.assertEquals;

public class LeftPadderTest {

    @Rule
    public TestRule skippyRule = Skippy.predictWithSkippy();


    @Test
    public void testPadLeft() {
        var input = TestConstants.HELLO;
        assertEquals(" hello", LeftPadder.padLeft(input, 6));
    }

}

Run the tests again:

./gradlew test --rerun

Check the content of test-impact-analysis.json:

tail .skippy/test-impact-analysis.json 
                }
        },
    "tests": [
                {
                        "class": 1,
                        "tags": ["PASSED"],
                        "coveredClasses": [0,1,4]
                }
    ]
}

The CoverageFileRule has populated the tests array. This will result in exclusions to be added when the tests are executed the next time:

./gradlew test --rerun                

> Task :test
Exclusions that will be added: 1
Adding exclusion **/com/example/LeftPadderTest.*

What I try to demonstrate here: The SkippyPlugin will never pass any exclusions to the test task unless a test contains the CoverageFileRule. Using the exclusion mechanism to "roll out" Skippy only makes sense if we find a way to do something similar for the CoverageFileRule.