diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 1c90805a6ac4c9..5cfabf53d8c501 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -100,7 +100,7 @@ import com.google.errorprone.annotations.FormatMethod; import java.util.Collection; import java.util.Map; -import java.util.regex.Pattern; +import java.util.Objects; import javax.annotation.Nullable; import net.starlark.java.eval.Debug; import net.starlark.java.eval.Dict; @@ -112,7 +112,6 @@ import net.starlark.java.eval.StarlarkCallable; import net.starlark.java.eval.StarlarkFunction; import net.starlark.java.eval.StarlarkInt; -import net.starlark.java.eval.StarlarkList; import net.starlark.java.eval.StarlarkThread; import net.starlark.java.eval.Tuple; import net.starlark.java.syntax.Identifier; @@ -140,7 +139,6 @@ public Label load(String from) throws Exception { } } }); - private static final Pattern RULE_NAME_PATTERN = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*"); // TODO(bazel-team): Remove the code duplication (BaseRuleClasses and this class). /** Parent rule class for non-executable non-test Starlark rules. */ @@ -308,6 +306,51 @@ public StarlarkRuleFunction rule( Object name, StarlarkThread thread) throws EvalException { + return createRule( + implementation, + test, + attrs, + implicitOutputs, + executable, + outputToGenfiles, + fragments, + hostFragments, + starlarkTestable, + toolchains, + useToolchainTransition, + providesArg, + execCompatibleWith, + analysisTest, + buildSetting, + cfg, + execGroups, + compileOneFiletype, + name, + thread); + } + + public static StarlarkRuleFunction createRule( + StarlarkFunction implementation, + boolean test, + Object attrs, + Object implicitOutputs, + boolean executable, + boolean outputToGenfiles, + Sequence fragments, + Sequence hostFragments, + boolean starlarkTestable, + Sequence toolchains, + boolean useToolchainTransition, + Sequence providesArg, + Sequence execCompatibleWith, + Object analysisTest, + Object buildSetting, + Object cfg, + Object execGroups, + Object compileOneFiletype, + Object name, + StarlarkThread thread) + throws EvalException { BazelStarlarkContext bazelContext = BazelStarlarkContext.from(thread); bazelContext.checkLoadingOrWorkspacePhase("rule"); // analysis_test=true implies test=true. @@ -510,81 +553,6 @@ public StarlarkRuleFunction rule( return starlarkRuleFunction; } - @Override - public void analysisTest( - String name, - StarlarkFunction implementation, - Object attrs, - Sequence fragments, - Sequence toolchains, - Object attrValuesApi, - StarlarkThread thread) - throws EvalException, InterruptedException { - if (!RULE_NAME_PATTERN.matcher(name).matches()) { - throw Starlark.errorf("'name' is limited to Starlark identifiers, got %s", name); - } - Dict attrValues = - Dict.cast(attrValuesApi, String.class, Object.class, "attr_values"); - if (attrValues.containsKey("name")) { - throw Starlark.errorf("'name' cannot be set or overridden in 'attr_values'"); - } - - StarlarkRuleFunction starlarkRuleFunction = - rule( - implementation, - /*test=*/ true, - attrs, - /*implicitOutputs=*/ Starlark.NONE, - /*executable=*/ false, - /*outputToGenfiles=*/ false, - /*fragments=*/ fragments, - /*hostFragments=*/ StarlarkList.empty(), - /*starlarkTestable=*/ false, - /*toolchains=*/ toolchains, - /*useToolchainTransition=*/ false, - /*doc=*/ "", - /*providesArg=*/ StarlarkList.empty(), - /*execCompatibleWith=*/ StarlarkList.empty(), - /*analysisTest=*/ Boolean.TRUE, - /*buildSetting=*/ Starlark.NONE, - /*cfg=*/ Starlark.NONE, - /*execGroups=*/ Starlark.NONE, - /*compileOneFiletype=*/ Starlark.NONE, - /*name=*/ Starlark.NONE, - thread); - - // Export the rule - // Because exporting can raise multiple errors, we need to accumulate them here into a single - // EvalException. This is a code smell because any non-ERROR events will be lost, and any - // location information in the events will be overwritten by the location of this rule's - // definition. - // However, this is currently fine because StarlarkRuleFunction#export only creates events that - // are ERRORs and that have the rule definition as their location. - // TODO(brandjon): Instead of accumulating events here, consider registering the rule in the - // BazelStarlarkContext, and exporting such rules after module evaluation in - // BzlLoadFunction#execAndExport. - PackageContext pkgContext = thread.getThreadLocal(PackageContext.class); - StoredEventHandler handler = new StoredEventHandler(); - starlarkRuleFunction.export( - handler, pkgContext.getLabel(), name + "_test"); // export in BUILD thread - if (handler.hasErrors()) { - StringBuilder errors = - handler.getEvents().stream() - .filter(e -> e.getKind() == EventKind.ERROR) - .reduce( - new StringBuilder(), - (sb, ev) -> sb.append("\n").append(ev.getMessage()), - StringBuilder::append); - throw Starlark.errorf("Errors in exporting %s: %s", name, errors.toString()); - } - - // Instantiate the target - Dict.Builder args = Dict.builder(); - args.put("name", name); - args.putAll(attrValues); - starlarkRuleFunction.call(thread, Tuple.of(), args.buildImmutable()); - } - /** * Returns the module (file) of the outermost enclosing Starlark function on the call stack or * null if none of the active calls are functions defined in Starlark. @@ -1002,6 +970,7 @@ private void errorf(EventHandler handler, String format, Object... args) { handler.handle(Event.error(definitionLocation, String.format(format, args))); } + @Override public RuleClass getRuleClass() { Preconditions.checkState(ruleClass != null && builder == null); return ruleClass; diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD index cf54a4361d4704..6e68b6343c8001 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -131,6 +131,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider", "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test", "//src/main/java/com/google/devtools/build/lib/util", diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java index 65d345e360def9..265d449595d4dc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java @@ -68,6 +68,7 @@ public ConfigurationTransitionApi transition( moduleContext.repoMapping()); } + // TODO(b/237422931): move into testing module @Override public ConfigurationTransitionApi analysisTestTransition( Dict changedSettings, // expected diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java index 5373dd0eb6dedf..96ac7a056b4e81 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java @@ -14,15 +14,26 @@ package com.google.devtools.build.lib.rules.test; import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; +import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions; +import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.StarlarkRuleFunction; import com.google.devtools.build.lib.analysis.test.ExecutionInfo; +import com.google.devtools.build.lib.events.EventKind; +import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.packages.PackageFactory.PackageContext; import com.google.devtools.build.lib.starlarkbuildapi.test.TestingModuleApi; +import java.util.regex.Pattern; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkFunction; import net.starlark.java.eval.StarlarkList; +import net.starlark.java.eval.StarlarkThread; +import net.starlark.java.eval.Tuple; /** A class that exposes testing infrastructure to Starlark. */ public class StarlarkTestingModule implements TestingModuleApi { + private static final Pattern RULE_NAME_PATTERN = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*"); @Override public ExecutionInfo executionInfo(Dict requirements /* */) @@ -41,4 +52,78 @@ public RunEnvironmentInfo testEnvironment( Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")), /* shouldErrorOnNonExecutableRule */ false); } + + @Override + public void analysisTest( + String name, + StarlarkFunction implementation, + Object attrs, + Sequence fragments, + Sequence toolchains, + Object attrValuesApi, + StarlarkThread thread) + throws EvalException, InterruptedException { + if (!RULE_NAME_PATTERN.matcher(name).matches()) { + throw Starlark.errorf("'name' is limited to Starlark identifiers, got %s", name); + } + Dict attrValues = + Dict.cast(attrValuesApi, String.class, Object.class, "attr_values"); + if (attrValues.containsKey("name")) { + throw Starlark.errorf("'name' cannot be set or overridden in 'attr_values'"); + } + + StarlarkRuleFunction starlarkRuleFunction = + StarlarkRuleClassFunctions.createRule( + implementation, + /*test=*/ true, + attrs, + /*implicitOutputs=*/ Starlark.NONE, + /*executable=*/ false, + /*outputToGenfiles=*/ false, + /*fragments=*/ fragments, + /*hostFragments=*/ StarlarkList.empty(), + /*starlarkTestable=*/ false, + /*toolchains=*/ toolchains, + /*useToolchainTransition=*/ false, + /*providesArg=*/ StarlarkList.empty(), + /*execCompatibleWith=*/ StarlarkList.empty(), + /*analysisTest=*/ Boolean.TRUE, + /*buildSetting=*/ Starlark.NONE, + /*cfg=*/ Starlark.NONE, + /*execGroups=*/ Starlark.NONE, + /*compileOneFiletype=*/ Starlark.NONE, + /*name=*/ Starlark.NONE, + thread); + + // Export the rule + // Because exporting can raise multiple errors, we need to accumulate them here into a single + // EvalException. This is a code smell because any non-ERROR events will be lost, and any + // location information in the events will be overwritten by the location of this rule's + // definition. + // However, this is currently fine because StarlarkRuleFunction#export only creates events that + // are ERRORs and that have the rule definition as their location. + // TODO(brandjon): Instead of accumulating events here, consider registering the rule in the + // BazelStarlarkContext, and exporting such rules after module evaluation in + // BzlLoadFunction#execAndExport. + PackageContext pkgContext = thread.getThreadLocal(PackageContext.class); + StoredEventHandler handler = new StoredEventHandler(); + starlarkRuleFunction.export( + handler, pkgContext.getLabel(), name + "_test"); // export in BUILD thread + if (handler.hasErrors()) { + StringBuilder errors = + handler.getEvents().stream() + .filter(e -> e.getKind() == EventKind.ERROR) + .reduce( + new StringBuilder(), + (sb, ev) -> sb.append("\n").append(ev.getMessage()), + StringBuilder::append); + throw Starlark.errorf("Errors in exporting %s: %s", name, errors.toString()); + } + + // Instantiate the target + Dict.Builder args = Dict.builder(); + args.put("name", name); + args.putAll(attrValues); + starlarkRuleFunction.call(thread, Tuple.of(), args.buildImmutable()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java index facd72ce53865d..9883f67121e990 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java @@ -397,77 +397,6 @@ StarlarkCallable rule( StarlarkThread thread) throws EvalException; - @StarlarkMethod( - name = "analysis_test", - doc = - "Creates a new analysis test target.

The number of transitive dependencies of the test" - + " are limited. The limit is controlled by" - + " --analysis_testing_deps_limit flag.", - parameters = { - @Param( - name = "name", - named = true, - doc = - "Name of the target. It should be a Starlark identifier, matching pattern" - + " '[A-Za-z_][A-Za-z0-9_]*'."), - @Param( - name = "implementation", - named = true, - doc = - "The Starlark function implementing this analysis test. It must have exactly one" - + " parameter: ctx. The function is called during the" - + " analysis phase. It can access the attributes declared by attrs" - + " and populated via attr_values. The implementation function may" - + " not register actions. Instead, it must register a pass/fail result" - + " via providing AnalysisTestResultInfo."), - @Param( - name = "attrs", - allowedTypes = { - @ParamType(type = Dict.class), - @ParamType(type = NoneType.class), - }, - named = true, - defaultValue = "None", - doc = - "Dictionary declaring the attributes. See the rule call." - + "Attributes are allowed to use configuration transitions defined using analysis_test_transition."), - @Param( - name = "fragments", - allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)}, - named = true, - defaultValue = "[]", - doc = - "List of configuration fragments that are available to the implementation of the" - + " analysis test."), - @Param( - name = TOOLCHAINS_PARAM, - allowedTypes = {@ParamType(type = Sequence.class, generic1 = Object.class)}, - named = true, - defaultValue = "[]", - doc = - "The set of toolchains the test requires. See the rule" - + " call."), - @Param( - name = "attr_values", - allowedTypes = {@ParamType(type = Dict.class, generic1 = String.class)}, - named = true, - defaultValue = "{}", - doc = "Dictionary of attribute values to pass to the implementation."), - }, - useStarlarkThread = true, - enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_ANALYSIS_TEST_CALL) - void analysisTest( - String name, - StarlarkFunction implementation, - Object attrs, - Sequence fragments, - Sequence toolchains, - Object argsValue, - StarlarkThread thread) - throws EvalException, InterruptedException; - @StarlarkMethod( name = "aspect", doc = diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/BUILD b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/BUILD index a295e6e29c4a4a..bb77166db88d4c 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/BUILD +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/BUILD @@ -25,6 +25,7 @@ java_library( # TODO(b/80307387): Remove dependency on Label implementation. "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform", "//src/main/java/net/starlark/java/annot", diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java index dd91fdc7739a1b..9b802eb8c475d1 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java @@ -14,14 +14,19 @@ package com.google.devtools.build.lib.starlarkbuildapi.test; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.starlarkbuildapi.RunEnvironmentInfoApi; +import com.google.devtools.build.lib.starlarkbuildapi.StarlarkRuleFunctionsApi; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.NoneType; import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.StarlarkFunction; +import net.starlark.java.eval.StarlarkThread; import net.starlark.java.eval.StarlarkValue; /** Helper module for accessing test infrastructure. */ @@ -81,4 +86,75 @@ RunEnvironmentInfoApi testEnvironment( Dict environment, // expected Sequence inheritedEnvironment /* expected */) throws EvalException; + + @StarlarkMethod( + name = "analysis_test", + doc = + "Creates a new analysis test target.

The number of transitive dependencies of the test" + + " are limited. The limit is controlled by" + + " --analysis_testing_deps_limit flag.", + parameters = { + @Param( + name = "name", + named = true, + doc = + "Name of the target. It should be a Starlark identifier, matching pattern" + + " '[A-Za-z_][A-Za-z0-9_]*'."), + @Param( + name = "implementation", + named = true, + doc = + "The Starlark function implementing this analysis test. It must have exactly one" + + " parameter: ctx. The function is called during the" + + " analysis phase. It can access the attributes declared by attrs" + + " and populated via attr_values. The implementation function may" + + " not register actions. Instead, it must register a pass/fail result" + + " via providing AnalysisTestResultInfo."), + @Param( + name = "attrs", + allowedTypes = { + @ParamType(type = Dict.class), + @ParamType(type = NoneType.class), + }, + named = true, + defaultValue = "None", + doc = + "Dictionary declaring the attributes. See the rule call." + + "Attributes are allowed to use configuration transitions defined using analysis_test_transition."), + @Param( + name = "fragments", + allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)}, + named = true, + defaultValue = "[]", + doc = + "List of configuration fragments that are available to the implementation of the" + + " analysis test."), + @Param( + name = StarlarkRuleFunctionsApi.TOOLCHAINS_PARAM, + allowedTypes = {@ParamType(type = Sequence.class, generic1 = Object.class)}, + named = true, + defaultValue = "[]", + doc = + "The set of toolchains the test requires. See the rule" + + " call."), + @Param( + name = "attr_values", + allowedTypes = {@ParamType(type = Dict.class, generic1 = String.class)}, + named = true, + defaultValue = "{}", + doc = "Dictionary of attribute values to pass to the implementation."), + }, + useStarlarkThread = true, + enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_ANALYSIS_TEST_CALL) + void analysisTest( + String name, + StarlarkFunction implementation, + Object attrs, + Sequence fragments, + Sequence toolchains, + Object argsValue, + StarlarkThread thread) + throws EvalException, InterruptedException; } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index 8c7d4f71e547d0..ac4413a667f9eb 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java @@ -166,17 +166,6 @@ public StarlarkCallable rule( return functionIdentifier; } - @Override - public void analysisTest( - String name, - StarlarkFunction implementation, - Object attrs, - Sequence fragments, - Sequence toolchains, - Object argsValue, - StarlarkThread thread) - throws EvalException, InterruptedException {} - @Override public Label label(String labelString, StarlarkThread thread) throws EvalException { try { diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index 636ce2b230a251..01a156ea2c400e 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -2402,7 +2402,7 @@ public void testAnalysisTest() throws Exception { " message = ''", " )]", "def my_test_macro(name):", - " analysis_test(name = name, implementation = impl)"); + " testing.analysis_test(name = name, implementation = impl)"); scratch.file( "p/BUILD", // "load(':b.bzl','my_test_macro')", @@ -2425,7 +2425,7 @@ public void testAnalysisTestAttrs() throws Exception { " )]", "def my_test_macro(name):", " native.filegroup(name = 'my_subject', srcs = [])", - " analysis_test(name = name,", + " testing.analysis_test(name = name,", " implementation = impl,", " attrs = {'target_under_test': attr.label_list()},", " attr_values = {'target_under_test': [':my_subject']},", @@ -2451,7 +2451,7 @@ public void testAnalysisTestDuplicateName() throws Exception { " message = ''", " )]", "def my_test_macro1(name):", - " analysis_test(name = name, implementation = impl)"); + " testing.analysis_test(name = name, implementation = impl)"); scratch.file( "p/b.bzl", "def impl(ctx): ", @@ -2460,7 +2460,7 @@ public void testAnalysisTestDuplicateName() throws Exception { " message = ''", " )]", "def my_test_macro2(name):", - " analysis_test(name = name, implementation = impl)"); + " testing.analysis_test(name = name, implementation = impl)"); scratch.file( "p/BUILD", // "load(':a.bzl','my_test_macro1')", @@ -2491,7 +2491,7 @@ public void testAnalysisTestBadName() throws Exception { " message = ''", " )]", "def my_test_macro(name):", - " analysis_test(name = name, implementation = impl)"); + " testing.analysis_test(name = name, implementation = impl)"); scratch.file( "p/BUILD", // "load(':b.bzl','my_test_macro')", @@ -2515,7 +2515,8 @@ public void testAnalysisTestBadArgs() throws Exception { " message = ''", " )]", "def my_test_macro(name):", - " analysis_test(name = name, implementation = impl, attr_values = {'notthere': []})"); + " testing.analysis_test(", + " name = name, implementation = impl, attr_values = {'notthere':[]})"); scratch.file( "p/BUILD", // "load(':b.bzl','my_test_macro')", @@ -2538,7 +2539,8 @@ public void testAnalysisTestErrorOnExport() throws Exception { " message = ''", " )]", "def my_test_macro(name):", - " analysis_test(name = name, implementation = impl, attrs = {'name': attr.string()})"); + " testing.analysis_test(name = name, implementation = impl, attrs = {'name':" + + " attr.string()})"); scratch.file( "p/BUILD", // "load(':b.bzl','my_test_macro')", @@ -2564,7 +2566,8 @@ public void testAnalysisTestErrorOverridingName() throws Exception { " message = ''", " )]", "def my_test_macro(name):", - " analysis_test(name = name, implementation = impl, attr_values = {'name': 'override'})"); + " testing.analysis_test(name = name, implementation = impl, attr_values = {'name':" + + " 'override'})"); scratch.file( "p/BUILD", // "load(':b.bzl','my_test_macro')",