From c0512e1c7e65441f499e673daf1080ca6aefba59 Mon Sep 17 00:00:00 2001 From: Scott Pollom Date: Fri, 15 Mar 2024 14:41:42 -0700 Subject: [PATCH] Remove GithubPresubmitValidator Previously, the convert tool could either be used to validate a single recipe in workingcopy mode or all recipes in source mode. There was no real use case for the latter validation (checking all recipes) because our CI tests validate each recipe one at a time, but checking a single recipe in source mode seems like a reasonable use case. This change removes the old support for checking all recipes in source mode and adds support for checking a single recipe in source mode. Fixes: 328820202: Test: GradleRecipeTest Change-Id: Ie99d1ca4501b7126e919415a84386cf2533f24cf --- .../gradle_recipe/converter/ConvertTool.kt | 81 ++++++------------- .../validators/GithubPresubmitValidator.kt | 34 -------- .../android/tools/gradle/GradleRecipeTest.kt | 32 ++++++++ recipes.bzl | 1 + 4 files changed, 57 insertions(+), 91 deletions(-) delete mode 100644 convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/validators/GithubPresubmitValidator.kt diff --git a/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/ConvertTool.kt b/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/ConvertTool.kt index 81721334..9ecc2db0 100644 --- a/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/ConvertTool.kt +++ b/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/ConvertTool.kt @@ -27,7 +27,7 @@ import com.google.android.gradle_recipe.converter.converters.RecipeConverter.Mod import com.google.android.gradle_recipe.converter.converters.RecipeConverter.Mode.SOURCE import com.google.android.gradle_recipe.converter.converters.RecipeConverter.Mode.WORKINGCOPY import com.google.android.gradle_recipe.converter.converters.RecursiveConverter -import com.google.android.gradle_recipe.converter.validators.GithubPresubmitValidator +import com.google.android.gradle_recipe.converter.validators.SourceValidator import com.google.android.gradle_recipe.converter.validators.WorkingCopyValidator import java.nio.file.Files import java.nio.file.Path @@ -161,65 +161,32 @@ fun main(args: Array) { // ensure no extra/unused values validateNullArg(destination, "'destination' must not be provided for subcommand '$COMMAND_VALIDATE'") validateNullArg(gradleVersion, "'gradleVersion' must not be provided for subcommand '$COMMAND_VALIDATE'") + validateNullArg( + sourceAll, + "'sourceAll' must not be provided for subcommand '$COMMAND_VALIDATE'" + ) + if (source == null) { + printErrorAndTerminate( + "'source' must not be null with subcommand '$COMMAND_VALIDATE'" + ) + } - if (mode != null) { - val cliMode = mode - if (cliMode != WORKINGCOPY) { - printErrorAndTerminate(""" - '$COMMAND_VALIDATE' command with a mode, requires value '$WORKINGCOPY'. - To convert all recipes from '$SOURCE' mode, omit the argument - """.trimIndent()) + when (mode) { + WORKINGCOPY -> { + val validator = + WorkingCopyValidator(context, agpVersion?.let { FullAgpVersion.of(it) }) + validator.validate(Path.of(source)) } - - // ensure no extra/unused values - validateNullArg( - sourceAll, - "'sourceAll' must not be provided for subcommand '$COMMAND_VALIDATE' and 'mode=$WORKINGCOPY'" - ) - - val validator = - WorkingCopyValidator(context, agpVersion?.let { FullAgpVersion.of(it) }) - validator.validate( - Path.of( - source - ?: printErrorAndTerminate("Source must not be null with subcommand '$COMMAND_VALIDATE' and 'mode=$WORKINGCOPY'") - ) - ) - } else { - // TODO(b/328820202) modify this else block to check a single recipe in source mode. - // ensure no extra/unused values - validateNullArg( - source, - "'source' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - validateNullArg( - agpVersion, - "'agpVersion' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - validateNullArg( - repoLocation, - "'repoLocation' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - validateNullArg( - gradlePath, - "'gradlePath' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - validateNullArg( - javaHome, - "'javaHome' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - validateNullArg( - androidHome, - "'androidHome' must not be provided for subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument" - ) - - val validator = GithubPresubmitValidator(context) - validator.validateAll( - Path.of( - sourceAll - ?: printErrorAndTerminate("SourceAll must not be null with subcommand '$COMMAND_VALIDATE' when not providing 'mode' argument") + SOURCE -> { + val validator = + SourceValidator(context, agpVersion?.let { FullAgpVersion.of(it) }) + validator.validate(Path.of(source)) + } + else -> { + printErrorAndTerminate( + "'$COMMAND_VALIDATE' command requires a 'mode' value of '$WORKINGCOPY' or '$SOURCE'" ) - ) + } } } }, diff --git a/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/validators/GithubPresubmitValidator.kt b/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/validators/GithubPresubmitValidator.kt deleted file mode 100644 index 8bfdf163..00000000 --- a/convert-tool/app/src/main/kotlin/com/google/android/gradle_recipe/converter/validators/GithubPresubmitValidator.kt +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2022 Google, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.android.gradle_recipe.converter.validators - -import com.google.android.gradle_recipe.converter.context.Context -import com.google.android.gradle_recipe.converter.recipe.visitRecipes -import java.nio.file.Path - -/** This will test all recipes found against both they min and - * current/max version of AGP - */ -class GithubPresubmitValidator(private val context: Context) { - - fun validateAll(rootFolder: Path) { - val recipeValidator = SourceValidator(context) - visitRecipes(rootFolder) { recipeFolder: Path -> - recipeValidator.validate(recipeFolder) - } - } -} \ No newline at end of file diff --git a/convert-tool/integTest/src/main/kotlin/com/android/tools/gradle/GradleRecipeTest.kt b/convert-tool/integTest/src/main/kotlin/com/android/tools/gradle/GradleRecipeTest.kt index 49617db9..d965c0a6 100644 --- a/convert-tool/integTest/src/main/kotlin/com/android/tools/gradle/GradleRecipeTest.kt +++ b/convert-tool/integTest/src/main/kotlin/com/android/tools/gradle/GradleRecipeTest.kt @@ -51,6 +51,10 @@ class GradleRecipeTest { System.getProperty("gradle_path") ?: error("Missing required system property \"gradle_path\".") val jdkVersion = System.getProperty("jdk_version") + val validateSource = + System.getProperty("validate_source")?.toBooleanStrict() + ?: error("Missing required system property \"validate_source\".") + val context = DefaultContext.createDefaultContext(Paths.get("tools/gradle-recipes")) val fullAgpVersion = FullAgpVersion.of(agpVersion) @@ -71,6 +75,34 @@ class GradleRecipeTest { val repos = System.getProperty("repos").split(",").map { File(it) } repos.forEach { Gradle.addRepo(it, repoDir) } + // Validate with --mode source to test that code path, but the validation itself is + // redundant because the recipe is validated with --mode workingcopy below. + // Because of this redundancy, we only validate the source copy for "ToT" AGP. + if (validateSource) { + main( + arrayOf( + "validate", + "--mode", + "source", + "--source", + source.toFile().absolutePath, + "--gradleRecipesFolder", + Paths.get("tools/gradle-recipes").toFile().absolutePath, + "--agpVersion", + agpVersion, + "--repoLocation", + FileUtils.toSystemIndependentPath(repoDir.absolutePath), + "--gradlePath", + File(gradlePath).toURI().toString(), + "--javaHome", + getJDKPath(jdkVersion).toFile().absolutePath, + "--androidHome", + File(TestUtils.getRelativeSdk()).absolutePath, + "--ci" + ) + ) + } + // Convert to working copy main( arrayOf( diff --git a/recipes.bzl b/recipes.bzl index 64771714..4e3d63c3 100644 --- a/recipes.bzl +++ b/recipes.bzl @@ -96,6 +96,7 @@ def recipe_test( "-Dversion_mappings_file=$(location :version_mappings.txt)", "-Dall_tested_agp_versions=" + ",".join(test_scenarios), "-Dconvert_debug=true", + "-Dvalidate_source=" + ("true" if agp_version == "ToT" else "false"), ] + (["-Djdk_version=" + str(test_scenarios[agp_version].get("jdk_version"))] if test_scenarios[agp_version].get("jdk_version") else []) + (select({