From ed6193e731a8e4ca15d6defcc160eb23af23d3aa Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 20 Jan 2023 10:29:34 -0800 Subject: [PATCH] python: Skip tests not supported by the Google Starlark rule implementation. This facilitates deletion of Google-specific Java code for the Python rules. The skipped tests all relate to Python 2 support, which the Google rules have no support for. Since this also isn't implemented in the Bazel Starlark rules, the overrides are left in place for that case. Also enables the test for toolchain resolution with the Starlark rule implementation since that feature has been implemented. PiperOrigin-RevId: 503473007 Change-Id: I57feb0e1d12bc8b4d48d686684eeb8db959fd66d --- .../google/devtools/build/lib/rules/python/BUILD | 6 ++++++ .../python/PyBinaryConfiguredTargetTest.java | 3 +++ .../PyExecutableConfiguredTargetTestBase.java | 16 +++++++++++++--- .../python/PythonSrcsVersionAspectTest.java | 11 ++++++++++- .../lib/rules/python/PythonStarlarkApiTest.java | 2 -- .../build/lib/rules/python/PythonTestUtils.java | 9 +++++++++ 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD index 3748af438636b7..5c06e17a729d46 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD @@ -34,6 +34,10 @@ test_suite( java_library( name = "PythonTestUtils", srcs = ["PythonTestUtils.java"], + deps = [ + "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", + "//third_party:junit4", + ], ) java_test( @@ -98,6 +102,7 @@ java_test( tags = ["no_windows"], deps = [ ":PyExecutableTestBase", + ":PythonTestUtils", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", @@ -162,6 +167,7 @@ java_test( name = "PythonSrcsVersionAspectTest", srcs = ["PythonSrcsVersionAspectTest.java"], deps = [ + ":PythonTestUtils", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyBinaryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyBinaryConfiguredTargetTest.java index e3896648cb1134..c2000566ea9aea 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PyBinaryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyBinaryConfiguredTargetTest.java @@ -51,6 +51,7 @@ private void declareBinDependingOnLibWithVersions(String binVersion, String libS @Test public void python2WithPy3SrcsVersionDependency() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); declareBinDependingOnLibWithVersions("PY2", "PY3"); @@ -62,6 +63,7 @@ public void python2WithPy3SrcsVersionDependency() throws Exception { @Test public void python2WithPy3OnlySrcsVersionDependency() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); declareBinDependingOnLibWithVersions("PY2", "PY3ONLY"); @@ -71,6 +73,7 @@ public void python2WithPy3OnlySrcsVersionDependency() throws Exception { @Test public void python3WithPy2OnlySrcsVersionDependency_NewSemantics() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); declareBinDependingOnLibWithVersions("PY3", "PY2ONLY"); diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java index fa667c98f1defd..cbcffeeb91dbe7 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java @@ -169,6 +169,7 @@ public void versionAttr_GoodValue() throws Exception { @Test public void py3IsDefaultFlag_SetsDefaultPythonVersion() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file( @@ -191,6 +192,7 @@ public void py3IsDefaultFlag_SetsDefaultPythonVersion() throws Exception { @Test public void py3IsDefaultFlag_DoesntOverrideExplicitVersion() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2")); @@ -206,6 +208,7 @@ public void py3IsDefaultFlag_DoesntOverrideExplicitVersion() throws Exception { @Test public void versionAttrWorks_WhenNotDefaultValue() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2")); @@ -222,7 +225,9 @@ public void versionAttrWorks_WhenSameAsDefaultValue() throws Exception { @Test public void versionAttrTakesPrecedence_NonDefaultValue() throws Exception { - setBuildLanguageOptions("--experimental_builtins_injection_override=-py_test,-py_binary"); + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled + setBuildLanguageOptions( + "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3")); assertPythonVersionIs_UnderNewConfig("//pkg:foo", PythonVersion.PY3, "--python_version=PY2"); @@ -230,7 +235,9 @@ public void versionAttrTakesPrecedence_NonDefaultValue() throws Exception { @Test public void versionAttrTakesPrecedence_DefaultValue() throws Exception { - setBuildLanguageOptions("--experimental_builtins_injection_override=-py_test,-py_binary"); + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled + setBuildLanguageOptions( + "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3")); assertPythonVersionIs_UnderNewConfig("//pkg:foo", PythonVersion.PY3, "--python_version=PY2"); @@ -238,6 +245,7 @@ public void versionAttrTakesPrecedence_DefaultValue() throws Exception { @Test public void canBuildWithDifferentVersionAttrs() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file( @@ -251,7 +259,9 @@ public void canBuildWithDifferentVersionAttrs() throws Exception { @Test public void incompatibleSrcsVersion() throws Exception { - setBuildLanguageOptions("--experimental_builtins_injection_override=-py_test,-py_binary"); + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled + setBuildLanguageOptions( + "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); reporter.removeHandler(failFastHandler); // We assert below that we don't fail at analysis. scratch.file( "pkg/BUILD", diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java index cd3cffd78f455b..d328d1eea7f6e3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java @@ -58,6 +58,7 @@ private String evaluateAspectFor(String label) throws Exception { @Test public void simpleCase() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file( @@ -117,6 +118,7 @@ public void noRequirements() throws Exception { @Test public void twoContradictoryRequirements() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file( @@ -154,6 +156,7 @@ public void twoContradictoryRequirements() throws Exception { @Test public void toplevelSelfContradictory() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); scratch.file( @@ -181,6 +184,7 @@ public void toplevelSelfContradictory() throws Exception { @Test public void indirectDependencies() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); // A <- B <- C <- bin, where only B has the constraint. @@ -223,6 +227,7 @@ public void indirectDependencies() throws Exception { @Test public void onlyReportTopmost() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); // A <- B <- C <- bin, where A and C have the constraint. @@ -266,6 +271,7 @@ public void onlyReportTopmost() throws Exception { @Test public void oneTopmostReachesAnother() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); // A <- B <- C, where A and C have the constraint. @@ -312,6 +318,7 @@ public void oneTopmostReachesAnother() throws Exception { @Test public void multiplePathsToRequirement() throws Exception { + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. setBuildLanguageOptions( "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); // Diamond graph A <- B, A <- C, B <- bin, C <- bin, where only A has the constraint. @@ -355,7 +362,9 @@ public void multiplePathsToRequirement() throws Exception { @Test public void noSrcsVersionButIntroducesRequirement() throws Exception { - setBuildLanguageOptions("--experimental_builtins_injection_override=-py_test,-py_binary"); + PythonTestUtils.assumeIsBazel(); // Google has py2 disabled. + setBuildLanguageOptions( + "--experimental_builtins_injection_override=-py_test,-py_binary,-py_library"); // A <- B <- C <- bin, B introduces the requirement but not via srcs_version. // dummy_rule propagates sources and sets the py3-only bit. scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java index d82059f4184ff1..08ed13b7d78084 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java @@ -167,8 +167,6 @@ public void runtimeSandwich() throws Exception { ")"); useConfiguration( "--extra_toolchains=//pkg:usertoolchain", "--incompatible_use_python_toolchains=true"); - // Starlark implementation doesn't yet support toolchain resolution - setBuildLanguageOptions("--experimental_builtins_injection_override=-py_test,-py_binary"); ConfiguredTarget target = getConfiguredTarget("//pkg:pybin"); assertThat(collectRunfiles(target).toList()) .containsAtLeast(getSourceArtifact("pkg/data.txt"), getSourceArtifact("pkg/userdata.txt")); diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonTestUtils.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonTestUtils.java index 8db0e89058239f..7e58f261aebfe6 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonTestUtils.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonTestUtils.java @@ -14,7 +14,9 @@ package com.google.devtools.build.lib.rules.python; +import static org.junit.Assume.assumeTrue; +import com.google.devtools.build.lib.testutil.TestConstants; /** Helpers for Python tests. */ public class PythonTestUtils { @@ -22,6 +24,13 @@ public class PythonTestUtils { // Static utilities class. private PythonTestUtils() {} + /** + * Skips the test if the product isn't bazel. This is mostly to skip tests for py2 support that + * the Google implementation would otherwise fail on. + */ + public static void assumeIsBazel() { + assumeTrue(TestConstants.PRODUCT_NAME.equals("bazel")); // Google has py2 disabled. + } /** * Stub method that is used to annotate that the calling test case assumes the default Python * version is PY2.