From d13c9641a187b5a3026ebb9093aeb480fdfe1838 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 27 Jan 2023 12:47:45 -0800 Subject: [PATCH] Make testLabelsOperator not depend on Python rule internals. The testLabelsOperator test was testing a case for implicit attributes by using the "$py_toolchain_type" attribute, which is only present for this test to pass. To fix, the test writes a simple Starlark rule with an implicit attribute that it queries instead of using the Python rules. PiperOrigin-RevId: 505187391 Change-Id: I8ac3f83320392f9f1a92e9f9559f41c7beaeda5a --- .../query2/testutil/AbstractQueryTest.java | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java index 50014a78c8b8cd..fe9c07eac4f8fd 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java @@ -1072,17 +1072,17 @@ private void runNodepDepsTest(boolean expectVisibilityDep, Setting... settings) @Test public void testNodepDeps_defaultIsTrue() throws Exception { - runNodepDepsTest(/*expectVisibilityDep=*/ true); + runNodepDepsTest(/* expectVisibilityDep= */ true); } @Test public void testNodepDeps_false() throws Exception { - runNodepDepsTest(/*expectVisibilityDep=*/ false, Setting.NO_NODEP_DEPS); + runNodepDepsTest(/* expectVisibilityDep= */ false, Setting.NO_NODEP_DEPS); } @Test public void testCycleInStarlark() throws Exception { - runCycleInStarlarkTest(/*checkFailureDetail=*/ true); + runCycleInStarlarkTest(/* checkFailureDetail= */ true); } protected void runCycleInStarlarkTest(boolean checkFailureDetail) throws Exception { @@ -1103,8 +1103,7 @@ protected void runCycleInStarlarkTest(boolean checkFailureDetail) throws Excepti public void testLabelsOperator() throws Exception { writeBuildFiles3(); writeBuildFilesWithConfigurableAttributes(); - writeFile("k/BUILD", "py_binary(name='k', srcs=['k.py'])"); - analysisMock.pySupport().setup(mockToolsConfig); + writeBuildFilesWithImplicitAttribute(); // srcs: assertThat(eval("labels(srcs, //a)")).isEqualTo(eval("//b + //c")); @@ -1123,13 +1122,12 @@ public void testLabelsOperator() throws Exception { assertThat(eval("labels(no_such_attr, //b)")).isEqualTo(EMPTY); // singleton LABEL: - assertThat(eval("labels(srcs, //k)")).isEqualTo(eval("//k:k.py")); + assertThat(eval("labels(srcs, //k)")).isEqualTo(eval("//k:k.txt")); // Works for implicit edges too. This is for consistency with --output - // xml, which exposes them too. - RepositoryName toolsRepository = helper.getToolsRepository(); - assertThat(eval("labels(\"$py_toolchain_type\", //k)")) - .isEqualTo(eval(toolsRepository + "//tools/python:toolchain_type")); + // xml, which exposes them too. Note that, for whatever reason, the + // implicit attribute must be referenced using "$" instead of "_". + assertThat(eval("labels('$implicit', //k)")).isEqualTo(eval("//k:implicit")); // Configurable deps: if (testConfigurableAttributes()) { @@ -1138,6 +1136,25 @@ public void testLabelsOperator() throws Exception { } } + private void writeBuildFilesWithImplicitAttribute() throws Exception { + writeFile( + "k/defs.bzl", + "def impl(ctx):", + " return [DefaultInfo()]", + "has_implicit_attr = rule(", + " implementation=impl,", + " attrs = {", + " 'srcs': attr.label_list(),", + " '_implicit': attr.label(default='//k:implicit')", + " },", + ")"); + writeFile( + "k/BUILD", + "load(':defs.bzl', 'has_implicit_attr')", + "has_implicit_attr(name='k', srcs=['k.txt'])", + "filegroup(name='implicit')"); + } + /* tests(x) operator */ @Test @@ -1886,7 +1903,7 @@ protected final void runBadRuleInDeps(Object code) throws Exception { writeFile("foo/BUILD", "sh_library(name = 'foo', deps = ['//bar:bar'])"); writeFile("bar/BUILD", "sh_library(name = 'bar', srcs = 'bad_single_file')"); EvalThrowsResult evalThrowsResult = - evalThrows("deps(//foo:foo)", /*unconditionallyThrows=*/ false); + evalThrows("deps(//foo:foo)", /* unconditionallyThrows= */ false); FailureDetail.Builder failureDetailBuilder = FailureDetail.newBuilder(); if (code instanceof FailureDetails.PackageLoading.Code) { failureDetailBuilder.setPackageLoading( @@ -1979,7 +1996,7 @@ public void boundedRdepsWithError() throws Exception { "sh_library(name = 'foo', deps = [':dep'])", "sh_library(name = 'dep', deps = ['//bar:missing'])"); assertThat( - evalThrows("rdeps(//foo:foo, //foo:dep, 1)", /*unconditionallyThrows=*/ false) + evalThrows("rdeps(//foo:foo, //foo:dep, 1)", /* unconditionallyThrows= */ false) .getMessage()) .contains("preloading transitive closure failed: no such package 'bar':"); }