From 384823e0ef5ec82cf4092a52fe035cd0d86ec953 Mon Sep 17 00:00:00 2001 From: Joaquim Alvino de Mesquita Neto Date: Tue, 18 Jan 2022 13:59:57 +0100 Subject: [PATCH] Test task up-to-date fix (#137) ## Description Unity Test task inputs weren't taking as input anything related to the project file structure, so it ended up being up-to-date even when there were relevant changes to the project. the `inputFiles` Input was create on the task in order to resolve this. ## Changes * ![FIX] Test task being up-to-date in wrong situations --- build.gradle | 2 +- .../tasks/TestTaskIntegrationSpec.groovy | 130 +++++++++++++++++- .../wooga/gradle/unity/UnityPlugin.groovy | 5 +- .../internal/InputFileTreeFactory.groovy | 69 ++++++++++ .../wooga/gradle/unity/tasks/Test.groovy | 1 - .../gradle/unity/traits/UnityTestSpec.groovy | 9 ++ 6 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 src/main/groovy/wooga/gradle/unity/internal/InputFileTreeFactory.groovy diff --git a/build.gradle b/build.gradle index a541b67d..8f4985d5 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ */ plugins { - id 'net.wooga.plugins' version '2.2.2' + id 'net.wooga.plugins' version '2.2.3' } group 'net.wooga.gradle' diff --git a/src/integrationTest/groovy/wooga/gradle/unity/tasks/TestTaskIntegrationSpec.groovy b/src/integrationTest/groovy/wooga/gradle/unity/tasks/TestTaskIntegrationSpec.groovy index 44126ba3..0be480a9 100644 --- a/src/integrationTest/groovy/wooga/gradle/unity/tasks/TestTaskIntegrationSpec.groovy +++ b/src/integrationTest/groovy/wooga/gradle/unity/tasks/TestTaskIntegrationSpec.groovy @@ -18,8 +18,8 @@ package wooga.gradle.unity.tasks import com.wooga.spock.extensions.unity.UnityPluginTestOptions -import kotlin.Unit import nebula.test.functional.ExecutionResult +import spock.lang.Shared import spock.lang.Unroll import wooga.gradle.unity.UnityPlugin import wooga.gradle.unity.UnityTaskIntegrationSpec @@ -256,7 +256,7 @@ class TestTaskIntegrationSpec extends UnityTaskIntegrationSpec { enableTestCodeCoverage.set(false) } """ - + and: "a mocked unity project with enabled playmode tests" setProjectSettingsFile(ProjectSettingsFile.TEMPLATE_CONTENT_ENABLED) @@ -270,6 +270,132 @@ class TestTaskIntegrationSpec extends UnityTaskIntegrationSpec { !editModeResult.args.contains("-enableCodeCoverage") } + @Shared + def mockProjectFiles = [ + [new File("Assets/Plugins.meta"), false], + [new File("Library/SomeCache.asset"), true], + [new File("ProjectSettings/SomeSettings.asset"), false], + [new File("UnityPackageManager/manifest.json"), false], + [new File("Assets/Plugins/iOS.meta"), true], + [new File("Assets/Plugins/iOS/somefile.m"), true], + [new File("Assets/Plugins/iOS/somefile.m.meta"), true], + [new File("Assets/Nested.meta"), false], + [new File("Assets/Nested/Plugins.meta"), false], + [new File("Assets/Nested/Plugins/iOS.meta"), true], + [new File("Assets/Nested/Plugins/iOS/somefile.m"), true], + [new File("Assets/Nested/Plugins/iOS/somefile.m.meta"), true], + [new File("Assets/Plugins/WebGL.meta"), true], + [new File("Assets/Plugins/WebGL/somefile.ts"), true], + [new File("Assets/Plugins/WebGL/somefile.ts.meta"), true], + [new File("Assets/Nested/Plugins/WebGL.meta"), true], + [new File("Assets/Nested/Plugins/WebGL/somefile.ts"), true], + [new File("Assets/Nested/Plugins/WebGL/somefile.ts.meta"), true], + [new File("Assets/Editor.meta"), false], + [new File("Assets/Editor/somefile.cs"), false], + [new File("Assets/Editor/somefile.cs.meta"), false], + [new File("Assets/Nested/Editor/somefile.cs"), false], + [new File("Assets/Source.cs"), false], + [new File("Assets/Source.cs.meta"), false], + [new File("Assets/Nested/LevelEditor.meta"), false], + [new File("Assets/Nested/LevelEditor/somefile.cs"), false], + [new File("Assets/Nested/LevelEditor/somefile.cs.meta"), false], + [new File("Assets/Plugins/Android.meta"), false], + [new File("Assets/Plugins/Android/somefile.java"), false], + [new File("Assets/Plugins/Android/somefile.java.meta"), false], + [new File("Assets/Nested/Plugins/Android.meta"), false], + [new File("Assets/Nested/Plugins/Android/s.java"), false], + [new File("Assets/Nested/Plugins/Android/s.java.meta"), false], + ] + + @Unroll + def "task #statusMessage up-to-date when #file changed with default inputFiles"() { + given: "a mocked unity project" + //need to convert the relative files to absolute files + def (_, File testFile) = prepareMockedProject(projectDir, files as Iterable, file as File) + and: "a unity test task" + buildFile << """ + tasks.register("unityTest", wooga.gradle.unity.tasks.Test) { + it.buildTarget = "${buildTarget}" + } + """.stripIndent() + + and: "a up-to-date project state" + def result = runTasksSuccessfully("unityTest") + assert !result.wasUpToDate('unityTest') + + result = runTasksSuccessfully("unityTest") + assert result.wasUpToDate('unityTest') + + when: "change content of one source file" + testFile.text = "new content" + + result = runTasksSuccessfully("unityTest") + + then: + result.wasUpToDate('unityTest') == upToDate + + where: + files = mockProjectFiles.collect { it[0] } + [file, upToDate] << mockProjectFiles + buildTarget = "android" + statusMessage = upToDate ? "is" : "is not" + } + + @Unroll + def "can set custom inputFiles for up-to-date check #type"() { + given: "a mocked unity project" + //need to convert the relative files to absolute files + def (_, File testFile) = prepareMockedProject(projectDir, files as Iterable, file as File) + + and: "a custom inputCollection" + buildFile << """ + tasks.register("unityTest", wooga.gradle.unity.tasks.Test) { + it.inputFiles.setFrom(${value}) + } + """.stripIndent() + + and: "a up-to-date project state" + def result = runTasksSuccessfully("unityTest") + assert !result.wasUpToDate('unityTest') + + result = runTasksSuccessfully("unityTest") + assert result.wasUpToDate('unityTest') + + when: "change content of one source file" + testFile.text = "new content" + + result = runTasksSuccessfully("unityTest") + + then: + result.wasUpToDate('unityTest') == upToDate + + where: + file | upToDate | type | value + new File("Assets/Plugins/iOS/somefile.m") | true | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/iOS/**")}' + new File("Assets/Plugins/Android/somefile.m") | false | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/iOS/**")}' + new File("Assets/Source.cs") | false | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/iOS/**")}' + new File("Assets/Plugins/iOS/somefile.m") | false | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/Android/**")}' + new File("Assets/Plugins/Android/somefile.m") | true | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/Android/**")}' + new File("Assets/Source.cs") | false | 'FileTree' | 'project.fileTree(project.projectDir){include("Assets/**"); exclude("**/Plugins/Android/**")}' + new File("Assets/Editor/somefile.cs") | true | 'FileCollection' | 'project.files("Assets/Editor/anyfile.cs","Assets/Source.cs")' + new File("Assets/Source.cs") | false | 'FileCollection' | 'project.files("Assets/Editor/anyfile.cs","Assets/Source.cs")' + + files = mockProjectFiles.collect { it[0] } + statusMessage = (upToDate) ? "is" : "is not" + } + + Tuple prepareMockedProject(File projectDir, Iterable files, File testFile) { + files = files.collect { new File(projectDir, it.path) } + testFile = new File(projectDir, testFile.path) + + //create directory structure + files.each { f -> + f.parentFile.mkdirs() + f.text = "some content" + } + new Tuple(files, testFile) + } + boolean matchesExpectedCoverageArgs(GradleRunResult taskResult) { return taskResult.args.contains("-enableCodeCoverage") && taskResult.args.contains("-coverageResultsPath") && diff --git a/src/main/groovy/wooga/gradle/unity/UnityPlugin.groovy b/src/main/groovy/wooga/gradle/unity/UnityPlugin.groovy index e0218b69..b6fd6f08 100644 --- a/src/main/groovy/wooga/gradle/unity/UnityPlugin.groovy +++ b/src/main/groovy/wooga/gradle/unity/UnityPlugin.groovy @@ -21,6 +21,7 @@ package wooga.gradle.unity import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.file.FileTreeElement import org.gradle.api.plugins.BasePlugin import org.gradle.api.plugins.ReportingBasePlugin import org.gradle.api.provider.Provider @@ -28,6 +29,7 @@ import org.gradle.api.reporting.ReportingExtension import org.gradle.api.specs.Spec import org.gradle.api.tasks.TaskProvider import org.gradle.language.base.plugins.LifecycleBasePlugin +import wooga.gradle.unity.internal.InputFileTreeFactory import wooga.gradle.unity.models.APICompatibilityLevel import wooga.gradle.unity.models.DefaultUnityAuthentication import wooga.gradle.unity.internal.DefaultUnityPluginExtension @@ -197,7 +199,6 @@ class UnityPlugin implements Plugin { // Override the batchmode property for edit/playmode test tasks by that of the extension project.tasks.withType(Test.class).configureEach({ Test t -> - Provider testBatchModeProvider = project.provider { def tp = t.testPlatform.getOrNull() try { @@ -215,6 +216,8 @@ class UnityPlugin implements Plugin { } true } + + t.inputFiles.from({ -> InputFileTreeFactory.inputFilesForUnityTask(project, extension, t) }) t.batchMode.set(testBatchModeProvider) t.reports.xml.outputLocation.convention(extension.reportsDir.file(t.name + "/" + t.name + "." + reports.xml.name)) t.enableCodeCoverage.convention(extension.enableTestCodeCoverage) diff --git a/src/main/groovy/wooga/gradle/unity/internal/InputFileTreeFactory.groovy b/src/main/groovy/wooga/gradle/unity/internal/InputFileTreeFactory.groovy new file mode 100644 index 00000000..45855c1a --- /dev/null +++ b/src/main/groovy/wooga/gradle/unity/internal/InputFileTreeFactory.groovy @@ -0,0 +1,69 @@ +package wooga.gradle.unity.internal + +import org.gradle.api.Project +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.Directory +import org.gradle.api.file.FileTreeElement +import org.gradle.api.provider.Provider +import wooga.gradle.unity.UnityPluginExtension +import wooga.gradle.unity.UnityTask + +class InputFileTreeFactory { + + private final Project project + private final UnityPluginExtension extension + + InputFileTreeFactory(Project project, UnityPluginExtension extension) { + this.project = project + this.extension = extension + } + + static ConfigurableFileCollection inputFilesForUnityTask(Project project, UnityPluginExtension extension, UnityTask task) { + return new InputFileTreeFactory(project, extension).inputFilesForUnityTask(task) + } + + ConfigurableFileCollection inputFilesForBuildTarget(Provider projectDirectory, Provider buildTarget) { + def assetsDir = extension.assetsDir + def assetsFileTree = project.fileTree(assetsDir) + assetsFileTree.include { FileTreeElement elem -> + def path = elem.getRelativePath().getPathString().toLowerCase() + def name = elem.name.toLowerCase() + if (path.contains("plugins") && !((name == "plugins") || (name == "plugins.meta"))) { + return isPluginElemFromBuildTarget(elem, buildTarget) + } else { + return true + } + } + + def projectSettingsDir = projectDirectory.map {it.dir("ProjectSettings") } + def projectSettingsFileTree = project.fileTree(projectSettingsDir) + + def packageManagerDir = projectDirectory.map{it.dir("UnityPackageManager") } + def packageManagerDirFileTree = project.fileTree(packageManagerDir) + + return project.files(assetsFileTree, projectSettingsFileTree, packageManagerDirFileTree) + } + + ConfigurableFileCollection inputFilesForUnityTask(UnityTask unityTask) { + return inputFilesForBuildTarget(unityTask.projectDirectory, unityTask.buildTarget) + } + + private static boolean isPluginElemFromBuildTarget(FileTreeElement element, Provider buildTarget) { + /* + Why can we use / here? Because {@code element} is a {@code FileTreeElement} object. + The getPath() method is not the same as {@code File.getPath()} + From the docs: + + * Returns the path of this file, relative to the root of the containing file tree. Always uses '/' as the hierarchy + * separator, regardless of platform file separator. Same as calling getRelativePath().getPathString(). + * + * @return The path. Never returns null. + */ + def path = element.getRelativePath().getPathString().toLowerCase() + if (buildTarget.isPresent()) { + return path.contains("plugins/" + buildTarget.get()) + } else { + return true + } + } +} diff --git a/src/main/groovy/wooga/gradle/unity/tasks/Test.groovy b/src/main/groovy/wooga/gradle/unity/tasks/Test.groovy index 83676310..10609d53 100644 --- a/src/main/groovy/wooga/gradle/unity/tasks/Test.groovy +++ b/src/main/groovy/wooga/gradle/unity/tasks/Test.groovy @@ -1,6 +1,5 @@ package wooga.gradle.unity.tasks -import org.gradle.api.provider.Property import org.gradle.api.tasks.Nested import org.gradle.api.tasks.StopExecutionException import org.gradle.internal.reflect.Instantiator diff --git a/src/main/groovy/wooga/gradle/unity/traits/UnityTestSpec.groovy b/src/main/groovy/wooga/gradle/unity/traits/UnityTestSpec.groovy index 427322a2..17ab6594 100644 --- a/src/main/groovy/wooga/gradle/unity/traits/UnityTestSpec.groovy +++ b/src/main/groovy/wooga/gradle/unity/traits/UnityTestSpec.groovy @@ -16,7 +16,16 @@ package wooga.gradle.unity.traits +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.tasks.InputFiles + trait UnityTestSpec extends UnityBaseSpec { + private final ConfigurableFileCollection inputFiles = objects.fileCollection() + + @InputFiles + ConfigurableFileCollection getInputFiles() { + return inputFiles + } }