From a9419f38d5f29af31a6c8ebda09a6e0303a6ba54 Mon Sep 17 00:00:00 2001 From: Xavier Bonaventura Date: Mon, 7 Dec 2020 08:23:02 -0800 Subject: [PATCH] Fix common prefix for instrumentation filter In case that two path share a prefix, the instrumentation filter is not computed proper. https://github.com/bazelbuild/bazel/issues/10949 This PR takes as a base the one that was created by @caiyulun and adds some tests. Original PR: https://github.com/bazelbuild/bazel/issues/10949 Closes #12607. PiperOrigin-RevId: 346097533 --- .../InstrumentationFilterSupport.java | 5 +- .../google/devtools/build/lib/buildtool/BUILD | 15 ++++++ .../InstrumentationFilterSupportTest.java | 47 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupportTest.java 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 index aa6c1062e3edc3..104a8f40bc7b53 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupport.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupport.java @@ -25,6 +25,7 @@ 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; @@ -124,9 +125,9 @@ private static void optimizeFilterSet(SortedSet packageFilters) { if (iterator.hasNext()) { // Optimize away nested filters. iterator = packageFilters.iterator(); - String prev = iterator.next(); + PathFragment prev = PathFragment.create(iterator.next()); while (iterator.hasNext()) { - String current = iterator.next(); + PathFragment current = PathFragment.create(iterator.next()); if (current.startsWith(prev)) { iterator.remove(); } else { diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD index 871c83d4d840f6..4d165b1b6b6186 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD @@ -311,6 +311,21 @@ java_test( ], ) +java_test( + name = "InstrumentationFilterSupportTest", + srcs = [ + "InstrumentationFilterSupportTest.java", + ], + deps = [ + "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/packages", + "//src/test/java/com/google/devtools/build/lib/analysis/util", + "//third_party:junit4", + "//third_party:truth", + ], +) + java_test( name = "LabelCrossesPackageBoundaryTest", srcs = ["LabelCrossesPackageBoundaryTest.java"], diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupportTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupportTest.java new file mode 100644 index 00000000000000..386c27a0d4efe4 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/buildtool/InstrumentationFilterSupportTest.java @@ -0,0 +1,47 @@ +// Copyright 2020 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 static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.events.EventCollector; +import com.google.devtools.build.lib.events.EventKind; +import com.google.devtools.build.lib.packages.Target; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests {@link com.google.devtools.build.lib.buildtool.InstrumentationFilterSupport}. */ +@RunWith(JUnit4.class) +public class InstrumentationFilterSupportTest extends BuildViewTestCase { + + @Test + public void testComputeInstrumentationFilter() throws Exception { + EventCollector events = new EventCollector(EventKind.INFO); + scratch.file("foo/BUILD", "sh_test(name='t', srcs=['t.sh'])"); + scratch.file("foobar/BUILD", "sh_test(name='t', srcs=['t.sh'])"); + List listOfTargets = new ArrayList<>(); + listOfTargets.add(getTarget("//foo:t")); + listOfTargets.add(getTarget("//foobar:t")); + Collection targets = Collections.unmodifiableCollection(listOfTargets); + String expectedFilter = "^//foo[/:],^//foobar[/:]"; + assertThat(InstrumentationFilterSupport.computeInstrumentationFilter(events, targets)) + .isEqualTo(expectedFilter); + } +}