From 649931e60fd4bcedc2c280917832a485ee012a77 Mon Sep 17 00:00:00 2001 From: Florian McKee <84742327+fmck3516@users.noreply.github.com> Date: Fri, 25 Oct 2024 11:44:42 -0500 Subject: [PATCH] Issues/159: Support Gradle configuration cache --- .../java/io/skippy/core/SkippyBuildApi.java | 5 ++- .../java/io/skippy/core/SkippyRepository.java | 38 +++++++++++++++- .../io/skippy/core/SkippyBuildApiTest.java | 4 ++ .../io/skippy/core/SkippyRepositoryTest.java | 18 ++++++++ .../io/skippy/gradle/CachableProperties.java | 44 ++++++++++++++++++ .../gradle/GradleClassFileCollector.java | 27 +++-------- .../io/skippy/gradle/SkippyAnalyzeTask.java | 45 +++---------------- .../io/skippy/gradle/SkippyCleanTask.java | 9 +++- .../io/skippy/gradle/SkippyGradleUtils.java | 16 +++---- .../java/io/skippy/gradle/SkippyPlugin.java | 45 ++++++++++++++++--- .../gradle/GradleClassFileCollectorTest.java | 7 ++- 11 files changed, 176 insertions(+), 82 deletions(-) create mode 100644 skippy-gradle/src/main/java/io/skippy/gradle/CachableProperties.java diff --git a/skippy-core/src/main/java/io/skippy/core/SkippyBuildApi.java b/skippy-core/src/main/java/io/skippy/core/SkippyBuildApi.java index 1e60139c..322e805a 100644 --- a/skippy-core/src/main/java/io/skippy/core/SkippyBuildApi.java +++ b/skippy-core/src/main/java/io/skippy/core/SkippyBuildApi.java @@ -34,7 +34,6 @@ public final class SkippyBuildApi { private final SkippyConfiguration skippyConfiguration; private final ClassFileCollector classFileCollector; private final SkippyRepository skippyRepository; - private final Set failedTests = new HashSet<>(); /** * C'tor. @@ -61,6 +60,7 @@ public void deleteSkippyFolder() { */ public void buildStarted() { skippyRepository.deleteLogFiles(); + skippyRepository.deleteListOfFailedTests(); skippyRepository.saveConfiguration(skippyConfiguration); } @@ -101,7 +101,7 @@ private void generateCoverageForSkippedTests(TestImpactAnalysis testImpactAnalys * @param className the class name of the failed tests */ public void testFailed(String className) { - failedTests.add(className); + skippyRepository.recordFailedTest(className); } private TestImpactAnalysis getTestImpactAnalysis() { @@ -117,6 +117,7 @@ private List getAnalyzedTests( TestWithJacocoExecutionDataAndCoveredClasses testWithExecutionData, ClassFileContainer classFileContainer ) { + var failedTests = skippyRepository.readListOfFailedTests(); var testResult = failedTests.contains(testWithExecutionData.testClassName()) ? TestResult.FAILED : TestResult.PASSED; var ids = classFileContainer.getIdsByClassName(testWithExecutionData.testClassName()); var executionId = skippyConfiguration.generateCoverageForSkippedTests() ? diff --git a/skippy-core/src/main/java/io/skippy/core/SkippyRepository.java b/skippy-core/src/main/java/io/skippy/core/SkippyRepository.java index 6bde47c6..1825fc09 100644 --- a/skippy-core/src/main/java/io/skippy/core/SkippyRepository.java +++ b/skippy-core/src/main/java/io/skippy/core/SkippyRepository.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -29,6 +30,7 @@ import static java.nio.file.Files.*; import static java.nio.file.StandardOpenOption.CREATE; import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; +import static java.util.Arrays.asList; import static java.util.Collections.emptyList; /** @@ -284,4 +286,38 @@ private void deleteTemporaryExecutionDataFilesForCurrentBuild() { } } -} \ No newline at end of file + void deleteListOfFailedTests() { + var failedTestsFile = SkippyFolder.get(projectDir).resolve("failed-tests.txt"); + if (exists(failedTestsFile)) { + try { + delete(failedTestsFile); + } catch (IOException e) { + throw new UncheckedIOException("Unable to delete %s.".formatted(failedTestsFile), e); + } + } + + } + + void recordFailedTest(String className) { + var failedTestsFile = SkippyFolder.get(projectDir).resolve("failed-tests.txt"); + try { + Files.write(failedTestsFile, asList(className), StandardCharsets.UTF_8, StandardOpenOption.APPEND, StandardOpenOption.CREATE); + } catch (IOException e) { + throw new UncheckedIOException("Unable record failed test %s in %s: %s.".formatted(className, failedTestsFile, e.getMessage()), e); + } + } + + List readListOfFailedTests() { + var failedTestsFile = SkippyFolder.get(projectDir).resolve("failed-tests.txt"); + try { + if (false == exists(failedTestsFile)) { + return emptyList(); + } + return Files.readAllLines(failedTestsFile, StandardCharsets.UTF_8); + + } catch (IOException e) { + throw new UncheckedIOException("Unable to read %s: %s.".formatted(failedTestsFile, e.getMessage()), e); + } + } +} + diff --git a/skippy-core/src/test/java/io/skippy/core/SkippyBuildApiTest.java b/skippy-core/src/test/java/io/skippy/core/SkippyBuildApiTest.java index b54769db..62c0fa40 100644 --- a/skippy-core/src/test/java/io/skippy/core/SkippyBuildApiTest.java +++ b/skippy-core/src/test/java/io/skippy/core/SkippyBuildApiTest.java @@ -300,6 +300,8 @@ void testEmptySkippyFolderWithTwoExecFilesOneFailedTests() throws JSONException )); buildApi.testFailed("com.example.FooTest"); + verify(skippyRepository).recordFailedTest("com.example.FooTest"); + when(skippyRepository.readListOfFailedTests()).thenReturn(asList("com.example.FooTest")); var tiaCaptor = ArgumentCaptor.forClass(TestImpactAnalysis.class); buildApi.buildFinished(); @@ -586,6 +588,8 @@ void testExistingJsonFileNewTestFailure() throws JSONException { )); buildApi.testFailed("com.example.FooTest"); + verify(skippyRepository).recordFailedTest("com.example.FooTest"); + when(skippyRepository.readListOfFailedTests()).thenReturn(asList("com.example.FooTest")); var tiaCaptor = ArgumentCaptor.forClass(TestImpactAnalysis.class); buildApi.buildFinished(); diff --git a/skippy-core/src/test/java/io/skippy/core/SkippyRepositoryTest.java b/skippy-core/src/test/java/io/skippy/core/SkippyRepositoryTest.java index 79a12b0e..781ce1d8 100644 --- a/skippy-core/src/test/java/io/skippy/core/SkippyRepositoryTest.java +++ b/skippy-core/src/test/java/io/skippy/core/SkippyRepositoryTest.java @@ -29,6 +29,7 @@ import static java.nio.file.Files.*; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.jupiter.api.Assertions.*; @@ -211,4 +212,21 @@ void readLatestTestImpactAnalysis_json_file_does_match_version_in_latest() throw assertEquals(testImpactAnalysis, skippyRepository.readLatestTestImpactAnalysis()); } + @Test + void testDeleteListOfFailedTests() { + skippyRepository.recordFailedTest("com.example.FooTest"); + assertTrue(exists(skippyFolder.resolve("failed-tests.txt"))); + skippyRepository.deleteListOfFailedTests();; + assertFalse(exists(skippyFolder.resolve("failed-tests.txt"))); + } + + @Test + void testRecordAndReadListOfFailedTest() throws IOException { + assertEquals(emptyList(), skippyRepository.readListOfFailedTests()); + skippyRepository.recordFailedTest("com.example.Test1"); + assertEquals(asList("com.example.Test1"), skippyRepository.readListOfFailedTests()); + skippyRepository.recordFailedTest("com.example.Test2"); + assertEquals(asList("com.example.Test1", "com.example.Test2"), skippyRepository.readListOfFailedTests()); + } + } \ No newline at end of file diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/CachableProperties.java b/skippy-gradle/src/main/java/io/skippy/gradle/CachableProperties.java new file mode 100644 index 00000000..3d7a940a --- /dev/null +++ b/skippy-gradle/src/main/java/io/skippy/gradle/CachableProperties.java @@ -0,0 +1,44 @@ +package io.skippy.gradle; + +import org.gradle.api.Project; +import org.gradle.api.tasks.SourceSetContainer; + +import java.io.File; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +/** + * A sub-set of relevant {@link Project} properties that are compatible with Gradle's Configuration Cache. + */ +class CachableProperties { + + final boolean sourceSetContainerAvailable; + final List classesDirs; + final SkippyPluginExtension skippyPluginExtension; + final Path projectDir; + final Path buildDir; + + private CachableProperties(boolean sourceSetContainerAvailable, List classesDirs, SkippyPluginExtension skippyExtension, Path projectDir, Path buildDir) { + this.sourceSetContainerAvailable = sourceSetContainerAvailable; + this.classesDirs = classesDirs; + this.skippyPluginExtension = skippyExtension; + this.projectDir = projectDir; + this.buildDir = buildDir; + } + + static CachableProperties from(Project project) { + var sourceSetContainer = project.getExtensions().findByType(SourceSetContainer.class); + if (sourceSetContainer != null) { + // new ArrayList<>() is a workaround for https://github.com/gradle/gradle/issues/26942 + var classesDirs = new ArrayList<>(sourceSetContainer.stream().flatMap(sourceSet -> sourceSet.getOutput().getClassesDirs().getFiles().stream()).toList()); + var skippyExtension = project.getExtensions().getByType(SkippyPluginExtension.class); + var projectDir = project.getProjectDir().toPath(); + var buildDir = project.getLayout().getBuildDirectory().getAsFile().get().toPath(); + return new CachableProperties(sourceSetContainer != null, classesDirs, skippyExtension, projectDir, buildDir); + } else { + return new CachableProperties(false, null, null, null, null); + } + } + +} \ No newline at end of file diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/GradleClassFileCollector.java b/skippy-gradle/src/main/java/io/skippy/gradle/GradleClassFileCollector.java index 4ac8fb6b..998bdd2d 100644 --- a/skippy-gradle/src/main/java/io/skippy/gradle/GradleClassFileCollector.java +++ b/skippy-gradle/src/main/java/io/skippy/gradle/GradleClassFileCollector.java @@ -20,14 +20,11 @@ import io.skippy.core.ClassFile; import io.skippy.core.Profiler; import org.gradle.api.tasks.SourceSet; -import org.gradle.api.tasks.SourceSetContainer; import java.io.File; import java.nio.file.Path; import java.util.*; -import static java.util.Comparator.comparing; - /** * Collects {@link ClassFile}s across all {@link SourceSet}s in a project. * @@ -36,16 +33,11 @@ final class GradleClassFileCollector implements ClassFileCollector { private final Path projectDir; - private final SourceSetContainer sourceSetContainer; + private final List classesDirs; - /** - * C'tor - * - * @param sourceSetContainer a {@link SourceSetContainer} - */ - GradleClassFileCollector(Path projectDir, SourceSetContainer sourceSetContainer) { + GradleClassFileCollector(Path projectDir, List classesDirs) { this.projectDir = projectDir; - this.sourceSetContainer = sourceSetContainer; + this.classesDirs = classesDirs; } /** @@ -57,22 +49,13 @@ final class GradleClassFileCollector implements ClassFileCollector { public List collect() { return Profiler.profile("GradleClassFileCollector#collect", () -> { var result = new ArrayList(); - for (var sourceSet : sourceSetContainer) { - result.addAll(collect(sourceSet)); + for (var classesDir : classesDirs) { + result.addAll(sort(collect(classesDir, classesDir))); } return result; }); } - private List collect(SourceSet sourceSet) { - var classesDirs = sourceSet.getOutput().getClassesDirs().getFiles(); - var result = new ArrayList(); - for (var classesDir : classesDirs) { - result.addAll(sort(collect(classesDir, classesDir))); - } - return result; - } - private List collect(File outputFolder, File directory) { var result = new LinkedList(); File[] files = directory.listFiles(); diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyAnalyzeTask.java b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyAnalyzeTask.java index 1eb3354a..175a1588 100644 --- a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyAnalyzeTask.java +++ b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyAnalyzeTask.java @@ -17,60 +17,29 @@ package io.skippy.gradle; import org.gradle.api.DefaultTask; -import org.gradle.api.tasks.testing.Test; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Internal; import javax.inject.Inject; -import org.gradle.api.tasks.testing.TestDescriptor; -import org.gradle.api.tasks.testing.TestListener; -import org.gradle.api.tasks.testing.TestResult; - -import java.util.ArrayList; -import java.util.List; - import static io.skippy.gradle.SkippyGradleUtils.*; /** * Informs Skippy that the relevant parts of the build (e.g., compilation and testing) have finished. */ -class SkippyAnalyzeTask extends DefaultTask { +abstract class SkippyAnalyzeTask extends DefaultTask { + + @Internal + abstract Property getSettings(); @Inject public SkippyAnalyzeTask() { setGroup("skippy"); - var testFailedListener = new TestFailedListener(); - getProject().getTasks().withType(Test.class, testTask -> testTask.addTestListener(testFailedListener)); doLast(task -> { - ifBuildSupportsSkippy(getProject(), skippyBuildApi -> { - for (var failedTest : testFailedListener.failedTests) { - skippyBuildApi.testFailed(failedTest.getClassName()); - } + ifBuildSupportsSkippy(getSettings().get(), skippyBuildApi -> { skippyBuildApi.buildFinished(); }); }); } - static private class TestFailedListener implements TestListener { - private final List failedTests = new ArrayList<>(); - - @Override - public void beforeSuite(TestDescriptor testDescriptor) { - } - - @Override - public void afterSuite(TestDescriptor testDescriptor, TestResult testResult) { - } - - @Override - public void beforeTest(TestDescriptor testDescriptor) { - } - - @Override - public void afterTest(TestDescriptor testDescriptor, TestResult testResult) { - if (testResult.getResultType() == TestResult.ResultType.FAILURE) { - failedTests.add(testDescriptor); - } - } - } - } \ No newline at end of file diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyCleanTask.java b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyCleanTask.java index 04ab705b..77ef060f 100644 --- a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyCleanTask.java +++ b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyCleanTask.java @@ -17,6 +17,8 @@ package io.skippy.gradle; import org.gradle.api.DefaultTask; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Input; import javax.inject.Inject; @@ -29,13 +31,16 @@ * * @author Florian McKee */ -class SkippyCleanTask extends DefaultTask { +abstract class SkippyCleanTask extends DefaultTask { + + @Input + abstract Property getSettings(); @Inject public SkippyCleanTask() { setGroup("skippy"); doLast(task -> { - ifBuildSupportsSkippy(getProject(), skippyBuildApi -> { + ifBuildSupportsSkippy(getSettings().get(), skippyBuildApi -> { skippyBuildApi.deleteSkippyFolder(); }); }); diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyGradleUtils.java b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyGradleUtils.java index 58e700dd..67b83171 100644 --- a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyGradleUtils.java +++ b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyGradleUtils.java @@ -18,26 +18,22 @@ import io.skippy.core.SkippyBuildApi; import io.skippy.core.SkippyRepository; -import org.gradle.api.Project; -import org.gradle.api.tasks.SourceSetContainer; import java.util.function.Consumer; final class SkippyGradleUtils { - static void ifBuildSupportsSkippy(Project project, Consumer action) { - var sourceSetContainer = project.getExtensions().findByType(SourceSetContainer.class); - if (sourceSetContainer != null) { - var skippyExtension = project.getExtensions().getByType(SkippyPluginExtension.class); - var skippyConfiguration = skippyExtension.toSkippyConfiguration(); - var projectDir = project.getProjectDir().toPath(); + static void ifBuildSupportsSkippy(CachableProperties settings, Consumer action) { + if (settings.sourceSetContainerAvailable) { + var skippyConfiguration = settings.skippyPluginExtension.toSkippyConfiguration(); + var projectDir = settings.projectDir; var skippyBuildApi = new SkippyBuildApi( skippyConfiguration, - new GradleClassFileCollector(projectDir, sourceSetContainer), + new GradleClassFileCollector(projectDir, settings.classesDirs), SkippyRepository.getInstance( skippyConfiguration, projectDir, - project.getLayout().getBuildDirectory().getAsFile().get().toPath() + settings.buildDir ) ); action.accept(skippyBuildApi); diff --git a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyPlugin.java b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyPlugin.java index b9ee39f7..09662939 100644 --- a/skippy-gradle/src/main/java/io/skippy/gradle/SkippyPlugin.java +++ b/skippy-gradle/src/main/java/io/skippy/gradle/SkippyPlugin.java @@ -19,9 +19,12 @@ import io.skippy.core.Profiler; import org.gradle.api.Project; import org.gradle.api.tasks.testing.Test; +import org.gradle.api.tasks.testing.TestDescriptor; +import org.gradle.api.tasks.testing.TestListener; +import org.gradle.api.tasks.testing.TestResult; import org.gradle.testing.jacoco.plugins.JacocoPlugin; -import static io.skippy.gradle.SkippyGradleUtils.*; +import static io.skippy.gradle.SkippyGradleUtils.ifBuildSupportsSkippy; /** * The Skippy plugin adds the @@ -40,14 +43,46 @@ final class SkippyPlugin implements org.gradle.api.Plugin { @Override public void apply(Project project) { Profiler.clear(); + project.getPlugins().apply(JacocoPlugin.class); project.getExtensions().create("skippy", SkippyPluginExtension.class); project.getTasks().register("skippyClean", SkippyCleanTask.class); project.getTasks().register("skippyAnalyze", SkippyAnalyzeTask.class); - project.getTasks().withType(Test.class, testTask -> testTask.finalizedBy("skippyAnalyze")); - project.afterEvaluate(action -> - ifBuildSupportsSkippy(action.getProject(), skippyBuildApi -> skippyBuildApi.buildStarted()) - ); + + project.afterEvaluate(action -> { + + var cachableProperties = CachableProperties.from(action); + + project.getTasks().withType(SkippyCleanTask.class).forEach( task -> task.getSettings().set(cachableProperties)); + project.getTasks().withType(SkippyAnalyzeTask.class).forEach( task -> task.getSettings().set(cachableProperties)); + + action.getTasks().withType(Test.class, testTask -> { + testTask.finalizedBy("skippyAnalyze"); + testTask.addTestListener(new TestListener() { + @Override + public void beforeSuite(TestDescriptor testDescriptor) { + } + + @Override + public void afterSuite(TestDescriptor testDescriptor, TestResult testResult) { + } + + @Override + public void beforeTest(TestDescriptor testDescriptor) { + } + + @Override + public void afterTest(TestDescriptor testDescriptor, TestResult testResult) { + if (testResult.getResultType() == TestResult.ResultType.FAILURE) { + ifBuildSupportsSkippy(cachableProperties, + skippyBuildApi -> skippyBuildApi.testFailed(testDescriptor.getClassName())); + } + } + }); + }); + + ifBuildSupportsSkippy(cachableProperties, skippyBuildApi -> skippyBuildApi.buildStarted()); + }); } } \ No newline at end of file diff --git a/skippy-gradle/src/test/java/io/skippy/gradle/GradleClassFileCollectorTest.java b/skippy-gradle/src/test/java/io/skippy/gradle/GradleClassFileCollectorTest.java index 10e623b6..29efc588 100644 --- a/skippy-gradle/src/test/java/io/skippy/gradle/GradleClassFileCollectorTest.java +++ b/skippy-gradle/src/test/java/io/skippy/gradle/GradleClassFileCollectorTest.java @@ -26,6 +26,8 @@ import java.net.URISyntaxException; import java.nio.file.Paths; import java.util.Set; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; @@ -44,10 +46,11 @@ public class GradleClassFileCollectorTest { void testCollect() throws URISyntaxException { var sourceSetContainer = mockSourceSetContainer("main", "test"); + var classesDirs = sourceSetContainer.stream().flatMap(sourceSet -> sourceSet.getOutput().getClassesDirs().getFiles().stream()).toList(); var projectDir = Paths.get(GradleClassFileCollectorTest.class.getResource("build").toURI()).getParent(); - var classFileCollector = new GradleClassFileCollector(projectDir, sourceSetContainer); + var classFileCollector = new GradleClassFileCollector(projectDir, classesDirs); var classFiles = classFileCollector.collect(); @@ -115,7 +118,7 @@ private static SourceSetContainer mockSourceSetContainer(String... sourceSetDire for (int i = 0; i < sourceSets.size(); i++) { when(sourceSetContainer.getByName(sourceSetDirectories[i])).thenReturn(sourceSets.get(i)); } - when(sourceSetContainer.iterator()).thenReturn(sourceSets.iterator()); + when(sourceSetContainer.stream()).thenReturn(sourceSets.stream()); return sourceSetContainer; }