Skip to content

Refactor ThriftTests and Java8Tests to use addSourceLines instead of deprecated addSourceFile#1517

Merged
msridhar merged 4 commits intouber:masterfrom
nanookclaw:fix/replace-deprecated-addSourceFile
Apr 15, 2026
Merged

Refactor ThriftTests and Java8Tests to use addSourceLines instead of deprecated addSourceFile#1517
msridhar merged 4 commits intouber:masterfrom
nanookclaw:fix/replace-deprecated-addSourceFile

Conversation

@nanookclaw
Copy link
Copy Markdown
Contributor

@nanookclaw nanookclaw commented Apr 11, 2026

Summary

Follow-up to #1426, continuing the migration from deprecated CompilationTestHelper#addSourceFile to addSourceLines as described in #1425.

Changes

ThriftTests.java:

  • Replace 1 addSourceFile("testdata/Util.java") call with addSourceLines, inlining the Util.java source as a text block
  • Remove @SuppressWarnings("deprecation") class annotation

Java8Tests.java:

  • Replace 4 addSourceFile calls with addSourceLines:
    • NullAwayJava8PositiveCases.java (inlined)
    • NullAwayJava8NegativeCases.java (inlined)
    • NullAwaySuperFunctionalInterface.java (inlined)
    • NullAwayOverrideFunctionalInterfaces.java (inlined)
  • Remove @SuppressWarnings("deprecation") class and method annotations
  • Delete 4 now-unused testdata resource files

Notes

  • Copyright/license headers are stripped when inlining (consistent with PR Refactor CoreTests and UnannotatedTests to use addSourceLines and inline test data #1426 pattern)
  • Util.java resource file is not deleted — it is still referenced by -XepOpt:NullAway:CastToNonNullMethod=com.uber.nullaway.testdata.Util.castToNonNull in other test files
  • Remaining test files with addSourceFile calls (InitializationTests, AndroidTest, FrameworkTests, CoreTests) can be addressed in follow-up PRs to keep reviews manageable

Fixes #1425 (partial)

Summary by CodeRabbit

Release Notes

  • Tests
    • Modernized test infrastructure to reduce deprecation warnings and improve code organization.

…deprecated addSourceFile

Replaces all calls to deprecated CompilationTestHelper#addSourceFile with
addSourceLines, inlining the source code as text blocks. This follows the
same pattern as PR uber#1426 which handled CoreTests and UnannotatedTests.

Changes:
- ThriftTests: Replace 1 addSourceFile call (Util.java inlined)
- Java8Tests: Replace 4 addSourceFile calls (NullAwayJava8PositiveCases,
  NullAwayJava8NegativeCases, NullAwaySuperFunctionalInterface,
  NullAwayOverrideFunctionalInterfaces all inlined)
- Remove @SuppressWarnings("deprecation") annotations from both classes
- Delete 4 now-unused testdata files for Java8Tests

Fixes uber#1425 (partial - remaining test files can be addressed in follow-up PRs)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5ce9f18f-7979-49d6-a141-e14572af748c

📥 Commits

Reviewing files that changed from the base of the PR and between 596dc40 and b7bc81e.

📒 Files selected for processing (6)
  • nullaway/src/test/java/com/uber/nullaway/Java8Tests.java
  • nullaway/src/test/java/com/uber/nullaway/ThriftTests.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayJava8NegativeCases.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayJava8PositiveCases.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayOverrideFunctionalInterfaces.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwaySuperFunctionalInterface.java
💤 Files with no reviewable changes (4)
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwaySuperFunctionalInterface.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayJava8PositiveCases.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayOverrideFunctionalInterfaces.java
  • nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayJava8NegativeCases.java

Walkthrough

This pull request refactors Java 8 and Thrift test cases by replacing deprecated CompilationTestHelper#addSourceFile() calls with inline addSourceLines(). The changes embed test source code directly in test classes using text blocks, removing the @SuppressWarnings("deprecation") annotations. Four testdata files (NullAwayJava8NegativeCases.java, NullAwayJava8PositiveCases.java, NullAwayOverrideFunctionalInterfaces.java, and NullAwaySuperFunctionalInterface.java) are deleted, with their content migrated inline into Java8Tests.java and ThriftTests.java. The test behavior and coverage remain unchanged.

Possibly related PRs

Suggested reviewers

  • msridhar
  • yuxincs
  • lazaroclapp
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring effort: replacing deprecated addSourceFile calls with addSourceLines in ThriftTests and Java8Tests.
Linked Issues check ✅ Passed The PR fully addresses #1425 objectives: replaces addSourceFile with addSourceLines, inlines source code, and removes @SuppressWarnings("deprecation") annotations.
Out of Scope Changes check ✅ Passed All changes directly support the stated objective of migrating from deprecated addSourceFile to addSourceLines; no extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator

@msridhar msridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.48%. Comparing base (480c8d3) to head (77daf65).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1517   +/-   ##
=========================================
  Coverage     88.48%   88.48%           
  Complexity     2843     2843           
=========================================
  Files           103      103           
  Lines          9489     9489           
  Branches       1905     1905           
=========================================
  Hits           8396     8396           
  Misses          531      531           
  Partials        562      562           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 11, 2026

CLA assistant check
All committers have signed the CLA.

@msridhar msridhar enabled auto-merge (squash) April 11, 2026 14:41
@msridhar msridhar disabled auto-merge April 11, 2026 14:42
@msridhar
Copy link
Copy Markdown
Collaborator

@nanookclaw we'll need you to sign the CLA to accept this change

@nanookclaw
Copy link
Copy Markdown
Contributor Author

Hi @msridhar, just wanted to let you know that the CLA has been signed and the check is now showing as passed. All CI checks are green. Thanks for the review and the merge when you get a chance!

@msridhar msridhar enabled auto-merge (squash) April 15, 2026 19:28
@msridhar msridhar merged commit 331a920 into uber:master Apr 15, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace calls to deprecated com.google.errorprone.CompilationTestHelper#addSourceFile in tests

3 participants