Convert test data in main NullAway tests to use text blocks#1424
Convert test data in main NullAway tests to use text blocks#1424
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1424 +/- ##
=========================================
Coverage 88.37% 88.37%
Complexity 2667 2667
=========================================
Files 97 97
Lines 8853 8853
Branches 1773 1773
=========================================
Hits 7824 7824
Misses 512 512
Partials 517 517 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughRefactors 30+ test files to replace per-line addSourceLines string arrays with Java text blocks (multi-line string literals) embedding whole test Java sources. Test code content, control flow, and diagnostic expectations are preserved. In the ContractsTests embedded test fixtures, the in-test class Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-08-14T18:50:06.159ZApplied to files:
📚 Learning: 2025-11-25T22:43:06.446ZApplied to files:
📚 Learning: 2025-08-29T18:41:43.584ZApplied to files:
📚 Learning: 2025-08-28T04:54:20.953ZApplied to files:
📚 Learning: 2025-10-29T23:56:18.236ZApplied to files:
📚 Learning: 2025-09-24T23:14:37.777ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java (1)
189-199: Consider converting remaining test sources to text blocks.While Test.java and Test2.java were converted to text blocks, the GestureDetector.java and MotionEvent.java sources (lines 189-199) still use the old string array format. For consistency with the PR's goal and the rest of this file, these could also be converted to text blocks.
🔎 Suggested conversion
defaultCompilationHelper - .addSourceLines( // Dummy android.view.GestureDetector.OnGestureListener interface - "GestureDetector.java", - "package android.view;", - "public class GestureDetector {", - " public static interface OnGestureListener {", - // Ignore other methods for this test, to make code shorter on both files: - " boolean onScroll(MotionEvent me1, MotionEvent me2, float f1, float f2);", - " }", - "}") - .addSourceLines( // Dummy android.view.MotionEvent class - "MotionEvent.java", "package android.view;", "public class MotionEvent { }") + .addSourceLines( + "GestureDetector.java", + """ + package android.view; + public class GestureDetector { + public static interface OnGestureListener { + // Ignore other methods for this test, to make code shorter on both files: + boolean onScroll(MotionEvent me1, MotionEvent me2, float f1, float f2); + } + } + """) + .addSourceLines( + "MotionEvent.java", + """ + package android.view; + public class MotionEvent { } + """)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (42)
nullaway/src/test/java/com/uber/nullaway/AccessPathsTests.javanullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/ArrayTests.javanullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.javanullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.javanullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.javanullaway/src/test/java/com/uber/nullaway/CoreTests.javanullaway/src/test/java/com/uber/nullaway/EnsuresNonNullIfTests.javanullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.javanullaway/src/test/java/com/uber/nullaway/ErrorProneCLIFlagsConfigTest.javanullaway/src/test/java/com/uber/nullaway/FrameworkTests.javanullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.javanullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.javanullaway/src/test/java/com/uber/nullaway/InitializationTests.javanullaway/src/test/java/com/uber/nullaway/Java8Tests.javanullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.javanullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.javanullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.javanullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.javanullaway/src/test/java/com/uber/nullaway/PureExceptLambdaTests.javanullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.javanullaway/src/test/java/com/uber/nullaway/SerializationTest.javanullaway/src/test/java/com/uber/nullaway/SuppressionNameAliasesTests.javanullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.javanullaway/src/test/java/com/uber/nullaway/ThriftTests.javanullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.javanullaway/src/test/java/com/uber/nullaway/UnsoundnessTests.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/BytecodeGenericsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericInheritanceTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodLambdaArgTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyLibraryModelsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.javanullaway/src/test/java/com/uber/nullaway/thirdpartylibs/GrpcTest.javascripts/convert_addSourceLines_textblock.py
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
Learnt from: CR
Repo: uber/NullAway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T22:43:06.446Z
Learning: Run only the tests for the main NullAway module using `./gradlew :nullaway:test` unless specifically asked to run tests in a different module
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/SuppressionNameAliasesTests.javanullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.javanullaway/src/test/java/com/uber/nullaway/CoreTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/BytecodeGenericsTests.javanullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.javanullaway/src/test/java/com/uber/nullaway/FrameworkTests.javanullaway/src/test/java/com/uber/nullaway/PureExceptLambdaTests.javanullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.javanullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.javanullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.javanullaway/src/test/java/com/uber/nullaway/ErrorProneCLIFlagsConfigTest.javanullaway/src/test/java/com/uber/nullaway/EnsuresNonNullIfTests.javanullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.javanullaway/src/test/java/com/uber/nullaway/SerializationTest.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericInheritanceTests.javanullaway/src/test/java/com/uber/nullaway/AccessPathsTests.javanullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.javanullaway/src/test/java/com/uber/nullaway/Java8Tests.javanullaway/src/test/java/com/uber/nullaway/ArrayTests.javanullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.javanullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.javanullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.javanullaway/src/test/java/com/uber/nullaway/InitializationTests.javanullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.javanullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.java
📚 Learning: 2025-10-29T23:56:18.236Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/SuppressionNameAliasesTests.javanullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.javanullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.javanullaway/src/test/java/com/uber/nullaway/SerializationTest.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java
📚 Learning: 2025-08-29T18:41:43.584Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.javanullaway/src/test/java/com/uber/nullaway/CoreTests.javanullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/ErrorProneCLIFlagsConfigTest.javanullaway/src/test/java/com/uber/nullaway/SerializationTest.javanullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.java
📚 Learning: 2025-08-28T04:54:20.953Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/CoreTests.javanullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.javanullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.javanullaway/src/test/java/com/uber/nullaway/SerializationTest.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/jspecify/GenericInheritanceTests.javanullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.javanullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.java
📚 Learning: 2025-11-25T22:43:06.446Z
Learnt from: CR
Repo: uber/NullAway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T22:43:06.446Z
Learning: Use the `--tests` flag with gradle to run specific test classes or methods, e.g., `./gradlew :nullaway:test --tests "com.uber.nullaway.NullAwayTest"`
Applied to files:
nullaway/src/test/java/com/uber/nullaway/CoreTests.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java
📚 Learning: 2025-11-25T22:43:06.446Z
Learnt from: CR
Repo: uber/NullAway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T22:43:06.446Z
Learning: Run only the tests for the main NullAway module using `./gradlew :nullaway:test` unless specifically asked to run tests in a different module
Applied to files:
nullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.javanullaway/src/test/java/com/uber/nullaway/FrameworkTests.javanullaway/src/test/java/com/uber/nullaway/PureExceptLambdaTests.javanullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.javanullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.javanullaway/src/test/java/com/uber/nullaway/VarargsTests.javanullaway/src/test/java/com/uber/nullaway/AccessPathsTests.javanullaway/src/test/java/com/uber/nullaway/ArrayTests.javanullaway/src/test/java/com/uber/nullaway/UnannotatedTests.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.java
🔇 Additional comments (37)
nullaway/src/test/java/com/uber/nullaway/UnsoundnessTests.java (1)
28-42: LGTM! Text block conversion improves readability.The conversion from multiple string lines to a single text block is correctly formatted and preserves the test source content. The triple-quoted string literal is the modern, preferred approach for multi-line test data in Java.
nullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.java (1)
15-23: LGTM! Consistent text block conversions throughout.All test methods have been consistently converted to use text blocks, improving readability while preserving the test semantics, expected diagnostics, and control flow.
Also applies to: 32-45, 54-72, 81-113, 122-141, 150-177, 186-210, 219-254, 263-300
nullaway/src/test/java/com/uber/nullaway/Java8Tests.java (1)
39-57: LGTM! Text block conversions preserve test content.The conversions correctly maintain all test source content, including inline comments, while improving readability through the use of text blocks.
Also applies to: 67-80
nullaway/src/test/java/com/uber/nullaway/SuppressionNameAliasesTests.java (1)
18-36: LGTM! Clean conversion to text block.The text block conversion is properly formatted and maintains the test's suppression configuration and expected diagnostic behavior.
nullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.java (1)
11-20: LGTM! Consistent text block conversions across all tests.All test methods have been successfully converted to use text blocks, maintaining the original test semantics, expected diagnostics, and bytecode interaction scenarios.
Also applies to: 29-40, 49-58, 67-86
nullaway/src/test/java/com/uber/nullaway/PureExceptLambdaTests.java (1)
23-89: LGTM! Text block conversion applied correctly.The conversion of test source data from multiple string literals to Java text blocks is accurate and preserves all semantic content, including package declarations, imports, method bodies, and diagnostic comments.
nullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.java (1)
39-405: LGTM! Consistent text block conversion across all test cases.All test methods have been successfully converted to use Java text blocks for embedded source code. The conversion preserves all type-use annotations, diagnostic comments, and test expectations.
nullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.java (1)
12-172: LGTM! Text block conversion maintains test integrity.All KeySetIterator test cases have been converted to text blocks while preserving map iteration logic, diagnostic comments, and error expectations.
nullaway/src/test/java/com/uber/nullaway/jspecify/BytecodeGenericsTests.java (1)
17-262: LGTM! Generic type tests converted accurately.All JSpecify bytecode generics tests have been converted to text blocks with generic type parameters, annotations, and override expectations correctly preserved.
nullaway/src/test/java/com/uber/nullaway/VarargsTests.java (1)
14-787: LGTM! Comprehensive varargs tests converted to text blocks.The extensive varargs nullability test suite has been successfully converted to use Java text blocks. All type-use annotations, declaration annotations, diagnostic expectations, and complex override scenarios are correctly preserved. The selective retention of the old format in
testVarargsNullArrayUnannotated(lines 535-552) andGenerated.javaintestNonNullVarargsFromHandler(lines 199-211) is appropriate where the old format remains clear and functional.nullaway/src/test/java/com/uber/nullaway/jspecify/GenericInheritanceTests.java (1)
13-272: Text block conversions for complex generic and inheritance patterns are syntactically correct and complete.All test source conversions to text blocks properly preserve:
- Multi-line JavaDoc comments with embedded formatting (e.g., lines 46, 53–59 in
nullableTypeArgInExtendsMulti).- Complex generic type syntax with annotations (
extends @Nullable Object,extends Supplier<@Nullable T2>,extends BiSupplier<@Nullable V2, K2>).- Diagnostic assertion comments embedded in source (
// BUG: Diagnostic contains: returning @Nullable expressionin lines 97, 160, 189).- Inheritance chain structures across multiple test classes including nested generic bounds.
- Special characters (e.g., en-dash in line 54:
Sub‑interface).Text block syntax is well-formed throughout: all closing
"""are properly placed, indentation is consistent, and no escaping issues are present.nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1)
25-105: Text block conversions are syntactically correct and semantically preserved.All five conversions from multi-string
addSourceLines()calls to Java text blocks are properly formatted (lines 29–36, 45–50, 60–65, 74–79, 88–93). Inline comments (e.g.,// BUG: Diagnostic contains:) are correctly embedded within the text blocks. The file appropriately retains the old multi-string format for package-info.java handling (lines 100–103).nullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.java (1)
16-31: Text block conversion is correct with proper comment preservation.The diagnostic and inline comments are correctly embedded within the text blocks. For example, line 102's comment
// inInstanceOf => isNotNull!and the// BUG: Diagnostic contains:markers are all preserved within the multi-line strings.nullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.java (1)
17-53: Text block conversion preserves all diagnostic and inline comments correctly.Comments are properly maintained within the text blocks, including diagnostic markers like
// BUG: Diagnostic contains:and explanatory notes like// no error since a.isPresent() is called.nullaway/src/test/java/com/uber/nullaway/AccessPathsTests.java (1)
11-18: Text block conversion handles complex nested code and comments correctly.The conversion properly preserves intricate code structures including nested classes, lambdas, local classes, and multi-line comments at various nesting levels. For example, the local class at lines 357–364 and the complex shadowing scenario at lines 400–421 are both correctly formatted within their respective text blocks.
nullaway/src/test/java/com/uber/nullaway/ArrayTests.java (1)
38-185: LGTM! Text block conversion improves test readability.The conversion from multiple string lines to Java text blocks is clean and preserves all test logic, BUG diagnostic comments, and assertions. The formatting enhances maintainability without changing test behavior.
nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java (1)
18-560: LGTM! Comprehensive text block conversion.The conversion to text blocks across all test methods is consistent and correct. The test at lines 138-151 appropriately retains the original format for
Generated.javasince it uses a dynamically constructed annotation based on Java version. All test logic and diagnostic expectations are preserved.nullaway/src/test/java/com/uber/nullaway/ThriftTests.java (1)
14-212: LGTM! Text blocks enhance test source clarity.The conversion to text blocks for multi-line test sources is clean and consistent. Single-line source declarations (e.g., TBase.java) appropriately retain the original format. All test expectations and diagnostic comments are preserved.
nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java (1)
16-175: LGTM! Clean conversion to text blocks.The conversion of all test sources to Java text blocks is successful, preserving all test logic, inline comments, and diagnostic expectations. The formatting significantly improves readability.
nullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java (1)
29-257: LGTM! Text block conversions improve test maintainability.The conversion of test sources to Java text blocks is well executed, preserving all test logic, BUG diagnostics, and inline comments. Tests that use dynamic annotations (line 91-95) or external source files appropriately retain their original format.
nullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.java (1)
17-27: LGTM!The text block conversion preserves the test source content correctly. The embedded Java source code, including package declaration, imports, class definition, and method body, is properly formatted within the text block.
nullaway/src/test/java/com/uber/nullaway/InitializationTests.java (1)
39-77: LGTM!Text block conversions for the
externalInitSupporttest are correct. The inline comments (e.g.,// no error here due to external init) are properly preserved within the text blocks, which aligns with the PR's approach of including Java comments inside the text blocks.nullaway/src/test/java/com/uber/nullaway/CoreTests.java (2)
83-108: LGTM!Text block conversions in
testGenericAnonymousInnerare correct. The embedded Java sources forGenericSuper.javaandAnonSub.javaare properly formatted with preserved diagnostic comments.
670-696: LGTM!The
testMapComputeIfAbsenttest correctly embeds the Java comment// Need JSpecify (vs javax) for annotating genericswithin the text block. This aligns with the PR's approach of preserving inline comments inside text blocks.nullaway/src/test/java/com/uber/nullaway/ErrorProneCLIFlagsConfigTest.java (1)
31-39: LGTM!The text block conversion in
onlyNullMarkedOkcorrectly preserves the test source including the@NullMarkedannotation and diagnostic comment.nullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.java (1)
33-38: LGTM!Text block conversions across
AcknowledgeRestrictiveAnnotationsTestsare correct. The embedded test sources, diagnostic comments, and inline explanatory comments are all properly preserved.nullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.java (1)
13-60: LGTM!Text block conversions in
multipleTypeParametersInstantiationare correct. TheNullableFunction.java,Test.java, andTestGuava.javasources are properly formatted with preserved functional interface definitions and diagnostic comments.nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java (3)
51-98: LGTM!Text block conversions in
streamSupportCollectorsToMapare correct. The complex test with filter predicates, Collectors usage, and multiple diagnostic comments is properly preserved.
476-529: LGTM!The Mockito-related test sources use explicit
// language=javacomments with text blocks, which is a valid alternative style that aids IDE language injection. This is consistent with the rest of the file's formatting approach.
752-776: Intentionally unconverted test noted.This test (
mapGetOrDefault) retains the originalString[]format. Per the PR description, additional test conversions outside the main tests will be done in follow-ups. The reuse of the samesourceLinesarray for two separate test helper invocations justifies keeping this format for now.nullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.java (1)
14-39: LGTM! Clean conversion to text blocks.All test methods have been correctly converted from multiple per-line string arguments to Java text blocks. The embedded test source code, including all
// BUG: Diagnostic contains:diagnostic comments, is preserved correctly. This improves readability without changing test semantics.Also applies to: 48-69, 78-87, 96-105, 114-124, 133-143, 152-162, 171-181, 190-210, 219-228, 237-246, 255-271, 280-317, 326-339, 348-361, 370-383, 392-405, 414-444, 453-483, 492-501, 510-519, 528-544, 553-568, 577-592, 601-634, 646-679
nullaway/src/test/java/com/uber/nullaway/SerializationTest.java (3)
1499-1550: Offset adjustments forerrorSerializationTestEnclosedByFieldDeclaration.Multiple offset values (248, 354, 487, 648) have been adjusted to reflect the new byte positions after converting to text blocks. These changes are consistent with the PR's stated approach of fixing offsets where inline comments shifted source positions.
139-150: LGTM! Text block conversions throughout SerializationTest.All test source definitions have been cleanly converted to text blocks while preserving the test semantics and expected outputs. The embedded Java source code content remains identical.
Also applies to: 183-208, 241-254, 287-312, 345-359, 392-413, 446-457, 490-503, 536-567, 613-619, 652-693, 794-806, 852-880, 902-923, 969-984, 1017-1052, 1111-1133, 1166-1202, 1248-1269, 1302-1325, 1359-1370, 1403-1414, 1456-1484, 1571-1580, 1635-1644, 1678-1701, 1774-1788, 1828-1846, 1900-1941, 2028-2037, 2120-2134, 2224-2234, 2268-2278, 2312-2355
151-167: Offset value verified for text block conversion.The offset value of
200at line 157 has been correctly adjusted to account for the text block conversion. When converting from concatenated strings to text block syntax ("""..."""), the byte positions of elements shift because the text block includes all content, including the comment// BUG: Diagnostic contains: returning @Nullable, as part of the source representation. This offset adjustment is necessary to ensure the error is reported at the correct location in the compiled source.nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.java (1)
12-31: LGTM! Clean text block conversion.All test source definitions have been correctly converted to text blocks. The
// BUG: Diagnostic contains:diagnostic markers are properly preserved within the text blocks. Simple one-liner helper sources (Item.java, Bar.java, Box.java) appropriately remain as inline strings.Also applies to: 46-85, 96-154, 162-196, 207-252, 260-338, 347-362, 373-391, 402-422, 433-480
nullaway/src/test/java/com/uber/nullaway/ContractsTests.java (2)
30-45: LGTM! Text block conversions are clean.The remaining test source definitions have been correctly converted to text blocks. All
// BUG: Diagnostic contains:diagnostic markers are preserved. TheCustomContract.javaandPureLibrary.javasources appropriately remainpublicsince they're in different packages (com.example.library) and need cross-package visibility.Also applies to: 85-104, 117-167, 180-214, 227-258, 272-319, 332-352, 364-395, 409-459, 472-511, 524-563, 576-605, 618-644
70-82:NullnessCheckerclasses should be package-private, notpublic.All 9
NullnessCheckerdefinitions in this file were changed topublic, appearing to be a systematic change from the conversion script. However, this is inconsistent with the pattern in other test files (e.g., TypeUseAnnotationsTests.java) where test helper classes remain package-private. Revert toclass NullnessCheckerto match the original code and maintain consistency across the test suite.
- Migrated UnannotatedTests and CoreTests from addSourceFile to addSourceLines. - Used Java text blocks for better readability, following the direction in uber#1424. - Removed physical test data files as they are now fully inlined. - Fixed CI failure by updating all dependent test classes.
These conversions were done by running the included
convert_addSourceLines_textblock.pyscript (fully generated by Codex) on the test source files. Beyond a straightforward conversion, any Java comments appearing between the String lines of the test data beforehand are now included in the text block so we don't lose them. See, e.g., how this test is transformed inFrameworkTests.I've reviewed by reading over the script and checking many converted source files manually. Also, the test coverage is exactly identical before and after this change. In a couple of tests in
SerializationTestthat rely on source positions, we had to fix some offsets, which changed since inline Java comments are now part of the test data.There are more tests to convert, outside the main NullAway tests and also tests of our auto-fixes; those will be done in follow-ups.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.