From 91374c30e363601dc9d8046a52439db02de347af Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 29 Sep 2023 12:42:35 -0700 Subject: [PATCH] Ignore code coverage for method executed non-deterministically in tests (#838) Fixes #828 (hopefully) There is some non-determinism in whether `com.uber.nullaway.dataflow.AccessPath.IteratorContentsKey#equals` runs or not during tests (see the code comment). The non-determinism will be hard for us to fix, so instead, add an annotation that tells JaCoCo to ignore coverage of this method (which is low risk as it's an `equals()` method that I don't expect will change). --- .../annotations/JacocoIgnoreGenerated.java | 14 ++++++++++++++ .../com/uber/nullaway/dataflow/AccessPath.java | 8 ++++++++ 2 files changed, 22 insertions(+) create mode 100644 nullaway/src/main/java/com/uber/nullaway/annotations/JacocoIgnoreGenerated.java diff --git a/nullaway/src/main/java/com/uber/nullaway/annotations/JacocoIgnoreGenerated.java b/nullaway/src/main/java/com/uber/nullaway/annotations/JacocoIgnoreGenerated.java new file mode 100644 index 0000000000..ca06d27f3f --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/annotations/JacocoIgnoreGenerated.java @@ -0,0 +1,14 @@ +package com.uber.nullaway.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation to indicate to Jacoco that code coverage of a method or constructor should be ignored. + * Jacoco requires such annotations to have "Generated" in their name. + */ +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) +public @interface JacocoIgnoreGenerated {} diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 056344a53f..ba440cfa38 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -35,6 +35,7 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.uber.nullaway.NullabilityUtil; +import com.uber.nullaway.annotations.JacocoIgnoreGenerated; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.List; @@ -648,7 +649,14 @@ public VariableElement getIteratorVarElement() { return iteratorVarElement; } + /** + * We ignore this method for code coverage since there is non-determinism somewhere deep in a + * Map implementation such that, depending on how AccessPaths get bucketed in the Map (which + * depends on non-deterministic hash codes), sometimes this method is called and sometimes it is + * not. + */ @Override + @JacocoIgnoreGenerated public boolean equals(Object o) { if (this == o) { return true;