Skip to content

Commit 38a7561

Browse files
msridharmsridhar
authored
Stop passing -processorpath to CompilationTestHelper in NullAway tests (#1326)
This seems to have a global side effect that causes weird test failures in #1323, and it's a bit of a hack. Instead, move the relevant tests to the `test-library-model` module where no such hacks are required. Test-only changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added capability to suggest removal of unnecessary non-null casts from library models * **Tests** * Expanded test coverage for library model scenarios with new test cases * **Chores** * Updated build dependencies and test configuration <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 334479c commit 38a7561

5 files changed

Lines changed: 60 additions & 43 deletions

File tree

‎code-coverage-report/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,5 @@ dependencies {
8585
implementation project(':jdk-javac-plugin')
8686
implementation project(':jdk-annotations:astubx-generator')
8787
implementation project(':jdk-annotations:jdk-integration-test')
88+
implementation project(':test-library-models')
8889
}

‎nullaway/src/test/java/com/uber/nullaway/AutoSuggestTest.java‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import com.google.errorprone.BugCheckerRefactoringTestHelper;
2828
import com.sun.source.tree.Tree;
29-
import com.uber.nullaway.testlibrarymodels.TestLibraryModels;
3029
import java.io.IOException;
3130
import org.junit.Rule;
3231
import org.junit.Test;
@@ -44,8 +43,6 @@ private BugCheckerRefactoringTestHelper makeTestHelper() {
4443
.setArgs(
4544
"-d",
4645
temporaryFolder.getRoot().getAbsolutePath(),
47-
"-processorpath",
48-
TestLibraryModels.class.getProtectionDomain().getCodeSource().getLocation().getPath(),
4946
"-XepOpt:NullAway:AnnotatedPackages=com.uber,com.ubercab,io.reactivex",
5047
"-XepOpt:NullAway:CastToNonNullMethod=com.uber.nullaway.testdata.Util.castToNonNull",
5148
"-XepOpt:NullAway:SuggestSuppressions=true");
@@ -172,32 +169,6 @@ public void removeUnnecessaryCastToNonNull() throws IOException {
172169
.doTest();
173170
}
174171

175-
@Test
176-
public void removeUnnecessaryCastToNonNullFromLibraryModel() throws IOException {
177-
makeTestHelper()
178-
.addInputLines(
179-
"Test.java",
180-
"package com.uber;",
181-
"import javax.annotation.Nullable;",
182-
"import static com.uber.nullaway.testdata.Util.castToNonNull;",
183-
"class Test {",
184-
" Object test1(Object o) {",
185-
" return castToNonNull(\"CAST_REASON\",o,42);",
186-
" }",
187-
"}")
188-
.addOutputLines(
189-
"out/Test.java",
190-
"package com.uber;",
191-
"import javax.annotation.Nullable;",
192-
"import static com.uber.nullaway.testdata.Util.castToNonNull;",
193-
"class Test {",
194-
" Object test1(Object o) {",
195-
" return o;",
196-
" }",
197-
"}")
198-
.doTest();
199-
}
200-
201172
@Test
202173
public void removeUnnecessaryCastToNonNullMultiLine() throws IOException {
203174
makeTestHelper()

‎test-library-models/build.gradle‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import net.ltgt.gradle.errorprone.CheckSeverity
1818

1919
plugins {
2020
id "java-library"
21+
id 'nullaway.java-test-conventions'
2122
}
2223

2324
dependencies {
@@ -26,6 +27,13 @@ dependencies {
2627
compileOnly project(":nullaway")
2728
annotationProcessor project(":nullaway")
2829
compileOnly deps.build.guava
30+
31+
testImplementation deps.test.junit4
32+
testImplementation(deps.build.errorProneTestHelpers) {
33+
exclude group: "junit", module: "junit"
34+
}
35+
testImplementation project(":nullaway")
36+
testImplementation project(":test-java-lib")
2937
}
3038

3139
tasks.withType(JavaCompile) {

‎test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods() {
8989
methodRef(
9090
"com.uber.nullaway.testdata.Util", "<T>castToNonNull(java.lang.String,T,int)"),
9191
1)
92+
.put(methodRef("com.uber.Test", "<T>castToNonNull(java.lang.String,T,int)"), 1)
9293
.build();
9394
}
9495

nullaway/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java renamed to test-library-models/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,23 @@
2222

2323
package com.uber.nullaway;
2424

25+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
2526
import com.google.errorprone.CompilationTestHelper;
2627
import com.uber.nullaway.generics.JSpecifyJavacConfig;
27-
import com.uber.nullaway.testlibrarymodels.TestLibraryModels;
28-
import java.util.ArrayList;
2928
import java.util.Arrays;
3029
import java.util.List;
30+
import org.junit.Rule;
3131
import org.junit.Test;
32+
import org.junit.rules.TemporaryFolder;
3233

33-
public class CustomLibraryModelsTests extends NullAwayTestsBase {
34+
public class CustomLibraryModelsTests {
3435

3536
private CompilationTestHelper makeLibraryModelsTestHelperWithArgs(List<String> args) {
36-
// Adding directly to args will throw an UnsupportedOperationException, since that list is
37-
// created by calling Arrays.asList (for consistency with the rest of NullAway's test cases),
38-
// which produces a list which doesn't support add/addAll. Because of this, before we add our
39-
// additional arguments, we must first copy the list into a mutable ArrayList.
40-
List<String> extendedArguments = new ArrayList<>(args);
41-
extendedArguments.addAll(
42-
0,
43-
Arrays.asList(
44-
"-processorpath",
45-
TestLibraryModels.class.getProtectionDomain().getCodeSource().getLocation().getPath()));
46-
return makeTestHelperWithArgs(extendedArguments);
37+
return CompilationTestHelper.newInstance(NullAway.class, getClass()).setArgs(args);
4738
}
4839

40+
@Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
41+
4942
@Test
5043
public void allowLibraryModelsOverrideAnnotations() {
5144
makeLibraryModelsTestHelperWithArgs(
@@ -290,4 +283,47 @@ public void issue1194() {
290283
"}")
291284
.doTest();
292285
}
286+
287+
@Test
288+
public void suggestRemovingUnnecessaryCastToNonNullFromLibraryModel() {
289+
var testHelper =
290+
BugCheckerRefactoringTestHelper.newInstance(NullAway.class, getClass())
291+
.setArgs(
292+
"-d",
293+
temporaryFolder.getRoot().getAbsolutePath(),
294+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
295+
"-XepOpt:NullAway:SuggestSuppressions=true");
296+
testHelper
297+
.addInputLines(
298+
"Test.java",
299+
"package com.uber;",
300+
"import javax.annotation.Nullable;",
301+
"class Test {",
302+
" public static <T> T castToNonNull(String reason, T value, int line) {",
303+
" if (value == null) {",
304+
" throw new NullPointerException(reason + \" at line \" + line);",
305+
" }",
306+
" return value;",
307+
" }",
308+
" Object test1(Object o) {",
309+
" return Test.castToNonNull(\"CAST_REASON\",o,42);",
310+
" }",
311+
"}")
312+
.addOutputLines(
313+
"out/Test.java",
314+
"package com.uber;",
315+
"import javax.annotation.Nullable;",
316+
"class Test {",
317+
" public static <T> T castToNonNull(String reason, T value, int line) {",
318+
" if (value == null) {",
319+
" throw new NullPointerException(reason + \" at line \" + line);",
320+
" }",
321+
" return value;",
322+
" }",
323+
" Object test1(Object o) {",
324+
" return o;",
325+
" }",
326+
"}")
327+
.doTest();
328+
}
293329
}

0 commit comments

Comments
 (0)