Skip to content

Commit

Permalink
Fix failing test due to JDK bug.
Browse files Browse the repository at this point in the history
    The bug: https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8169685

    This came up when diagnosing a mind-numbingly perplexing failure on RawAttributeMapperTest from https://buildkite.com/bazel/google-bazel-presubmit/builds/17716#f428d533-71d6-4483-b138-5c21345b97f2, happening due to change https://bazel.googlesource.com/bazel/+/cbcffa054c50fd28e7c2fe5fe935d1991a322527 which has nothing to do with RawAttributeMapperTest at all.

    The failure was triggered by removing LicensingTests.java. This changed how JUnit
    scheduled analysis_select_test. This caused the ClassCastException checked in RawAttributeMapperTest#testGetAttribute,testVisitLabels to be compiled instead of interpreted. Due to https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8169685, this meant its stack trace was no longer available, so the tests couldn't check its error message.

    I was able to produce a minimal repro by adding back in LicensingTests into the srcs of analysis_select_test, then ripping out all of LicensingTests except for testLicenseCheckingTakesOnlyOneSelectBranch.  When I commented out this line:

    //  ConfiguredTarget eve = getConfiguredTarget("//eden:eve");

    RawAttributeMapperTest failed. When I left it in, the test succeeded.

    See bazelbuild/bazel#7444.

    PiperOrigin-RevId: 241937508
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent fa81a71 commit 87b410e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,6 @@ public ImmutableList<String> getPackageDefaultCopts() {

@Override
public Collection<DepEdge> visitLabels() {
return visitLabels(ruleClass.getAttributes());
}

@Override
public Collection<DepEdge> visitLabels(Attribute attribute) {
return visitLabels(ImmutableList.of(attribute));
}

private Collection<DepEdge> visitLabels(Iterable<Attribute> attributes) {
List<DepEdge> edges = new ArrayList<>();
Type.LabelVisitor<Attribute> visitor =
(label, attribute) -> {
Expand All @@ -186,7 +177,7 @@ private Collection<DepEdge> visitLabels(Iterable<Attribute> attributes) {
edges.add(AttributeMap.DepEdge.create(absoluteLabel, attribute));
}
};
for (Attribute attribute : attributes) {
for (Attribute attribute : ruleClass.getAttributes()) {
Type<?> type = attribute.getType();
// TODO(bazel-team): clean up the typing / visitation interface so we don't have to
// special-case these types.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package com.google.devtools.build.lib.analysis.select;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -61,16 +61,16 @@ public void testGetAttribute() throws Exception {

// Configurable attribute: trying to directly access from a RawAttributeMapper throws a
// type mismatch exception.
IllegalArgumentException e =
assertThrows(
"Expected srcs lookup to fail since the returned type is a SelectorList and not a list",
IllegalArgumentException.class,
() -> rawMapper.get("srcs", BuildType.LABEL_LIST));
assertThat(e)
.hasMessageThat()
.isEqualTo(
"wrong type for attribute \"srcs\" in sh_binary rule //x:myrule: "
+ "expected list(label), is SelectorList");
try {
rawMapper.get("srcs", BuildType.LABEL_LIST);
fail("Expected srcs lookup to fail since the returned type is a SelectorList and not a list");
} catch (IllegalArgumentException e) {
assertThat(e)
.hasMessageThat()
.isEqualTo(
"wrong type for attribute \"srcs\" in sh_binary rule //x:myrule: "
+ "expected list(label), is SelectorList");
}
}

@Override
Expand All @@ -95,16 +95,16 @@ public void testConfigurabilityCheck() throws Exception {
@Test
public void testVisitLabels() throws Exception {
RawAttributeMapper rawMapper = RawAttributeMapper.of(setupGenRule());
IllegalArgumentException e =
assertThrows(
"Expected label visitation to fail since one attribute is configurable",
IllegalArgumentException.class,
() -> rawMapper.visitLabels());
assertThat(e)
.hasMessageThat()
.isEqualTo(
"wrong type for attribute \"srcs\" in sh_binary rule //x:myrule: "
+ "expected list(label), is SelectorList");
try {
rawMapper.visitLabels();
fail("Expected label visitation to fail since one attribute is configurable");
} catch (IllegalArgumentException e) {
assertThat(e)
.hasMessageThat()
.isEqualTo(
"wrong type for attribute \"srcs\" in sh_binary rule //x:myrule: "
+ "expected list(label), is SelectorList");
}
}

@Test
Expand Down

0 comments on commit 87b410e

Please sign in to comment.