diff options
Diffstat (limited to 'src/main/java')
7 files changed, 190 insertions, 194 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java index 2384e31c9a..efbd6ec202 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java @@ -415,8 +415,9 @@ public class BuildRequest implements OptionsClassProvider { private long startTimeMillis = 0; // milliseconds since UNIX epoch. - private boolean runningInEmacs = false; - private boolean runTests = false; + private boolean needsInstrumentationFilter; + private boolean runningInEmacs; + private boolean runTests; private static final ImmutableList<Class<? extends OptionsBase>> MANDATORY_OPTIONS = ImmutableList.of( @@ -576,6 +577,14 @@ public class BuildRequest implements OptionsClassProvider { return startTimeMillis; } + public void setNeedsInstrumentationFilter(boolean needInstrumentationFilter) { + this.needsInstrumentationFilter = needInstrumentationFilter; + } + + public boolean needsInstrumentationFilter() { + return needsInstrumentationFilter; + } + /** * Validates the options for this BuildRequest. * diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 6692a9e5ae..dd5b81d58b 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -70,6 +70,8 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.util.RegexFilter; +import com.google.devtools.common.options.OptionsParsingException; import java.util.Collection; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -168,6 +170,22 @@ public final class BuildTool { executionTool.init(); } + // Compute the heuristic instrumentation filter if needed. + if (request.needsInstrumentationFilter()) { + String instrumentationFilter = + InstrumentationFilterSupport.computeInstrumentationFilter( + env.getReporter(), loadingResult.getTestsToRun()); + try { + // We're modifying the buildOptions in place, which is not ideal, but we also don't want + // to pay the price for making a copy. Maybe reconsider later if this turns out to be a + // problem (and the performance loss may not be a big deal). + buildOptions.get(BuildConfiguration.Options.class).instrumentationFilter = + new RegexFilter.RegexFilterConverter().convert(instrumentationFilter); + } catch (OptionsParsingException e) { + throw new InvalidConfigurationException(e); + } + } + // Exit if there are any pending exceptions from modules. env.throwPendingException(); diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupport.java b/src/main/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupport.java new file mode 100644 index 0000000000..5781e0272b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupport.java @@ -0,0 +1,150 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// 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.devtools.build.lib.buildtool; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Joiner; +import com.google.common.collect.Sets; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.AttributeMap; +import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.packages.TargetUtils; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; +import java.util.SortedSet; + +/** + * Helper class to heuristically compute an instrumentation filter from a list of tests to run. + */ +public final class InstrumentationFilterSupport { + public static final String INSTRUMENTATION_FILTER_FLAG = "instrumentation_filter"; + + /** + * Method implements a heuristic used to set default value of the + * --instrumentation_filter option. Following algorithm is used: + * 1) Identify all test targets on the command line. + * 2) Expand all test suites into the individual test targets + * 3) Calculate list of package names containing all test targets above. + * 4) Replace all "javatests/" substrings in package names with "java/". + * 5) If two packages reside in the same directory, use filter based on + * the parent directory name instead. Doing so significantly simplifies + * instrumentation filter in majority of real-life scenarios (in + * particular when dealing with my/package/... wildcards). + * 6) Set --instrumentation_filter default value to instrument everything + * in those packages. + */ + @VisibleForTesting + public static String computeInstrumentationFilter( + EventHandler eventHandler, Collection<Target> testTargets) { + SortedSet<String> packageFilters = Sets.newTreeSet(); + collectInstrumentedPackages(testTargets, packageFilters); + optimizeFilterSet(packageFilters); + + String instrumentationFilter = "//" + Joiner.on(",//").join(packageFilters); + if (!packageFilters.isEmpty()) { + eventHandler.handle( + Event.info("Using default value for --instrumentation_filter: \"" + + instrumentationFilter + "\".")); + eventHandler.handle(Event.info("Override the above default with --" + + INSTRUMENTATION_FILTER_FLAG)); + } + return instrumentationFilter; + } + + private static void collectInstrumentedPackages( + Collection<Target> targets, Set<String> packageFilters) { + for (Target target : targets) { + // Add package-based filters for every test target. + String prefix = getInstrumentedPrefix(target.getLabel().getPackageName()); + if (!prefix.isEmpty()) { + packageFilters.add(prefix); + } + if (TargetUtils.isTestSuiteRule(target)) { + AttributeMap attributes = NonconfigurableAttributeMapper.of((Rule) target); + // We don't need to handle $implicit_tests attribute since we already added + // test_suite package to the set. + for (Label label : attributes.get("tests", BuildType.LABEL_LIST)) { + // Add package-based filters for all tests in the test suite. + packageFilters.add(getInstrumentedPrefix(label.getPackageName())); + } + } + } + } + + /** + * Returns prefix string that should be instrumented for a given package. Input string should + * be formatted like the output of Label.getPackageName(). + * Generally, package name will be used as such string with two modifications. + * - "javatests/ directories will be substituted with "java/", since we do + * not want to instrument java test code. "java/" directories in "test/" will + * be replaced by the same in "main/". + * - "/internal", "/public", and "tests/" package suffix will be dropped, since usually we would + * want to instrument code in the parent package as well + */ + @VisibleForTesting + public static String getInstrumentedPrefix(String packageName) { + if (packageName.endsWith("/internal")) { + packageName = packageName.substring(0, packageName.length() - "/internal".length()); + } else if (packageName.endsWith("/public")) { + packageName = packageName.substring(0, packageName.length() - "/public".length()); + } else if (packageName.endsWith("/tests")) { + packageName = packageName.substring(0, packageName.length() - "/tests".length()); + } + return packageName + .replaceFirst("(?<=^|/)javatests/", "java/") + .replaceFirst("(?<=^|/)test/java/", "main/java/"); + } + + private static void optimizeFilterSet(SortedSet<String> packageFilters) { + Iterator<String> iterator = packageFilters.iterator(); + if (iterator.hasNext()) { + // Find common parent filters to reduce number of filter expressions. In practice this + // still produces nicely constrained instrumentation filter while making final + // filter value much more user-friendly - especially in case of /my/package/... wildcards. + Set<String> parentFilters = Sets.newTreeSet(); + String filterString = iterator.next(); + PathFragment parent = PathFragment.create(filterString).getParentDirectory(); + while (iterator.hasNext()) { + String current = iterator.next(); + if (parent != null && parent.getPathString().length() > 0 + && !current.startsWith(filterString) && current.startsWith(parent.getPathString())) { + parentFilters.add(parent.getPathString()); + } else { + filterString = current; + parent = PathFragment.create(filterString).getParentDirectory(); + } + } + packageFilters.addAll(parentFilters); + + // Optimize away nested filters. + iterator = packageFilters.iterator(); + String prev = iterator.next(); + while (iterator.hasNext()) { + String current = iterator.next(); + if (current.startsWith(prev)) { + iterator.remove(); + } else { + prev = current; + } + } + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java index 6be7ae1afe..c1f3fd094e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommand.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.runtime; -import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsProvider; @@ -52,8 +51,6 @@ public interface BlazeCommand { * * @param env the command environment of the currently running command * @param optionsParser the options parser for the current command - * - * @throws AbruptExitException if something went wrong */ - void editOptions(CommandEnvironment env, OptionsParser optionsParser) throws AbruptExitException; + void editOptions(CommandEnvironment env, OptionsParser optionsParser); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java index 833e01145d..aa9e703fd4 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java @@ -13,35 +13,13 @@ // limitations under the License. package com.google.devtools.build.lib.runtime.commands; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Sets; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.TargetParsingException; -import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.packages.AttributeMap; -import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; -import com.google.devtools.build.lib.packages.Rule; -import com.google.devtools.build.lib.packages.Target; -import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.TestTimeout; -import com.google.devtools.build.lib.pkgcache.FilteringPolicies; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; -import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.ExitCode; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionPriority; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsParsingException; -import com.google.devtools.common.options.OptionsProvider; -import java.util.Collection; -import java.util.Iterator; -import java.util.Set; -import java.util.SortedSet; /** * Handles the 'coverage' command on the Bazel command line. @@ -136,16 +114,13 @@ import java.util.SortedSet; help = "resource:coverage.txt", allowResidue = true) public class CoverageCommand extends TestCommand { - private boolean wasInterrupted = false; - @Override protected String commandName() { return "coverage"; } @Override - public void editOptions(CommandEnvironment env, OptionsParser optionsParser) - throws AbruptExitException { + public void editOptions(CommandEnvironment env, OptionsParser optionsParser) { super.editOptions(env, optionsParser); try { optionsParser.parse(OptionPriority.SOFTWARE_REQUIREMENT, @@ -154,165 +129,9 @@ public class CoverageCommand extends TestCommand { optionsParser.parse(OptionPriority.COMPUTED_DEFAULT, "Options suggested for the coverage command", ImmutableList.of(TestTimeout.COVERAGE_CMD_TIMEOUT)); - if (!optionsParser.containsExplicitOption("instrumentation_filter")) { - setDefaultInstrumentationFilter(env, optionsParser); - } } catch (OptionsParsingException e) { // Should never happen. throw new IllegalStateException("Unexpected exception", e); } } - - @Override - public ExitCode exec(CommandEnvironment env, OptionsProvider options) { - if (wasInterrupted) { - wasInterrupted = false; - env.getReporter().handle(Event.error("Interrupted")); - return ExitCode.INTERRUPTED; - } - - return super.exec(env, options); - } - - /** - * Method implements a heuristic used to set default value of the - * --instrumentation_filter option. Following algorithm is used: - * 1) Identify all test targets on the command line. - * 2) Expand all test suites into the individual test targets - * 3) Calculate list of package names containing all test targets above. - * 4) Replace all "javatests/" substrings in package names with "java/". - * 5) If two packages reside in the same directory, use filter based on - * the parent directory name instead. Doing so significantly simplifies - * instrumentation filter in majority of real-life scenarios (in - * particular when dealing with my/package/... wildcards). - * 6) Set --instrumentation_filter default value to instrument everything - * in those packages. - */ - private void setDefaultInstrumentationFilter(CommandEnvironment env, - OptionsParser optionsProvider) - throws OptionsParsingException, AbruptExitException { - try { - BlazeRuntime runtime = env.getRuntime(); - // Initialize package cache, since it is used by the TargetPatternEvaluator. - // TODO(bazel-team): Don't allow commands to setup the package cache more than once per build. - // We'll have to move it earlier in the process to allow this. Possibly: Move it to - // the command dispatcher and allow commands to annotate "need-packages". - env.setupPackageCache(optionsProvider, runtime.getDefaultsPackageContent(optionsProvider)); - - // Collect all possible test targets. We don't really care whether there will be parsing - // errors here - they will be reported during actual build. - TargetPatternEvaluator targetPatternEvaluator = env.newTargetPatternEvaluator(); - Set<Target> testTargets = - targetPatternEvaluator.parseTargetPatternList( - env.getReporter(), - optionsProvider.getResidue(), - FilteringPolicies.FILTER_TESTS, - /*keep_going=*/true).getTargets(); - - SortedSet<String> packageFilters = Sets.newTreeSet(); - collectInstrumentedPackages(env, testTargets, packageFilters); - optimizeFilterSet(packageFilters); - - String instrumentationFilter = "//" + Joiner.on(",//").join(packageFilters); - final String instrumentationFilterOptionName = "instrumentation_filter"; - if (!packageFilters.isEmpty()) { - env.getReporter().handle( - Event.info("Using default value for --instrumentation_filter: \"" - + instrumentationFilter + "\".")); - - env.getReporter().handle(Event.info("Override the above default with --" - + instrumentationFilterOptionName)); - optionsProvider.parse(OptionPriority.COMPUTED_DEFAULT, - "Instrumentation filter heuristic", - ImmutableList.of("--" + instrumentationFilterOptionName - + "=" + instrumentationFilter)); - } - } catch (TargetParsingException e) { - // We can't compute heuristic - just use default filter. - } catch (InterruptedException e) { - // We cannot quit now because AbstractCommand does not have the - // infrastructure to do that. Just set a flag and return from exec() as - // early as possible. We can do this because there is always an exec() - // after an editOptions(). - wasInterrupted = true; - } - } - - private void collectInstrumentedPackages(CommandEnvironment env, - Collection<Target> targets, Set<String> packageFilters) throws InterruptedException { - for (Target target : targets) { - // Add package-based filters for every test target. - String prefix = getInstrumentedPrefix(target.getLabel().getPackageName()); - if (!prefix.isEmpty()) { - packageFilters.add(prefix); - } - if (TargetUtils.isTestSuiteRule(target)) { - AttributeMap attributes = NonconfigurableAttributeMapper.of((Rule) target); - // We don't need to handle $implicit_tests attribute since we already added - // test_suite package to the set. - for (Label label : attributes.get("tests", BuildType.LABEL_LIST)) { - // Add package-based filters for all tests in the test suite. - packageFilters.add(getInstrumentedPrefix(label.getPackageName())); - } - } - } - } - - /** - * Returns prefix string that should be instrumented for a given package. Input string should - * be formatted like the output of Label.getPackageName(). - * Generally, package name will be used as such string with two modifications. - * - "javatests/ directories will be substituted with "java/", since we do - * not want to instrument java test code. "java/" directories in "test/" will - * be replaced by the same in "main/". - * - "/internal", "/public", and "tests/" package suffix will be dropped, since usually we would - * want to instrument code in the parent package as well - */ - public static String getInstrumentedPrefix(String packageName) { - if (packageName.endsWith("/internal")) { - packageName = packageName.substring(0, packageName.length() - "/internal".length()); - } else if (packageName.endsWith("/public")) { - packageName = packageName.substring(0, packageName.length() - "/public".length()); - } else if (packageName.endsWith("/tests")) { - packageName = packageName.substring(0, packageName.length() - "/tests".length()); - } - return packageName - .replaceFirst("(?<=^|/)javatests/", "java/") - .replaceFirst("(?<=^|/)test/java/", "main/java/"); - } - - private static void optimizeFilterSet(SortedSet<String> packageFilters) { - Iterator<String> iterator = packageFilters.iterator(); - if (iterator.hasNext()) { - // Find common parent filters to reduce number of filter expressions. In practice this - // still produces nicely constrained instrumentation filter while making final - // filter value much more user-friendly - especially in case of /my/package/... wildcards. - Set<String> parentFilters = Sets.newTreeSet(); - String filterString = iterator.next(); - PathFragment parent = PathFragment.create(filterString).getParentDirectory(); - while (iterator.hasNext()) { - String current = iterator.next(); - if (parent != null && parent.getPathString().length() > 0 - && !current.startsWith(filterString) && current.startsWith(parent.getPathString())) { - parentFilters.add(parent.getPathString()); - } else { - filterString = current; - parent = PathFragment.create(filterString).getParentDirectory(); - } - } - packageFilters.addAll(parentFilters); - - // Optimize away nested filters. - iterator = packageFilters.iterator(); - String prev = iterator.next(); - while (iterator.hasNext()) { - String current = iterator.next(); - if (current.startsWith(prev)) { - iterator.remove(); - } else { - prev = current; - } - } - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java index c1403703d4..1900517604 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java @@ -16,9 +16,11 @@ package com.google.devtools.build.lib.runtime.commands; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.buildtool.BuildResult; import com.google.devtools.build.lib.buildtool.BuildTool; +import com.google.devtools.build.lib.buildtool.InstrumentationFilterSupport; import com.google.devtools.build.lib.buildtool.buildevent.TestingCompleteEvent; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -34,7 +36,6 @@ import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier; import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier.TestSummaryOptions; import com.google.devtools.build.lib.runtime.TestResultAnalyzer; import com.google.devtools.build.lib.runtime.TestResultNotifier; -import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter; import com.google.devtools.common.options.OptionPriority; @@ -65,8 +66,7 @@ public class TestCommand implements BlazeCommand { } @Override - public void editOptions(CommandEnvironment env, OptionsParser optionsParser) - throws AbruptExitException { + public void editOptions(CommandEnvironment env, OptionsParser optionsParser) { TestOutputFormat testOutput = optionsParser.getOptions(ExecutionOptions.class).testOutput; try { @@ -112,6 +112,11 @@ public class TestCommand implements BlazeCommand { runtime.getStartupOptionsProvider(), targets, env.getReporter().getOutErr(), env.getCommandId(), env.getCommandStartTime()); request.setRunTests(); + if (options.getOptions(BuildConfiguration.Options.class).collectCodeCoverage + && !options.containsExplicitOption( + InstrumentationFilterSupport.INSTRUMENTATION_FILTER_FLAG)) { + request.setNeedsInstrumentationFilter(true); + } BuildResult buildResult = new BuildTool(env).processRequest(request, null); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java index 82791a126d..f4cd3a39fc 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.runtime.commands.BuildCommand; import com.google.devtools.build.lib.runtime.commands.ProjectFileSupport; -import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionPriority; @@ -111,8 +110,7 @@ public class MobileInstallCommand implements BlazeCommand { } @Override - public void editOptions(CommandEnvironment env, OptionsParser optionsParser) - throws AbruptExitException { + public void editOptions(CommandEnvironment env, OptionsParser optionsParser) { try { if (optionsParser.getOptions(Options.class).v2) { optionsParser.parse( |