Skip to content

Replace deprecated addSourceFile calls in AndroidTest - #1839

Merged
msridhar merged 5 commits into
uber:masterfrom
jabrailkhalil:fix-1425-addsource-lines
Sep 15, 2026
Merged

msridhar merged 5 commits into
uber:masterfrom
jabrailkhalil:fix-1425-addsource-lines

Conversation

@jabrailkhalil

@jabrailkhalil jabrailkhalil commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #1425

Replaces all remaining calls to the deprecated CompilationTestHelper#addSourceFile in AndroidTest with addSourceLines, inlining the test sources as Java text blocks and deleting the android testdata source files. The now-unneeded class-level @SuppressWarnings("deprecation") is removed.

Tested locally with JDK 21: :nullaway:test --tests "com.uber.nullaway.AndroidTest" passes, and spotlessApply (ratchet from HEAD) is clean.

Summary by CodeRabbit

  • Tests

    • Consolidated Android lifecycle test fixtures and platform stubs directly into the test suite.
    • Removed standalone test resource files for Android, AndroidX, and support-library activities, fragments, and related stubs.
    • Preserved existing lifecycle initialization scenarios, nullability diagnostics, test coverage, and diagnostic expectations.
  • Refactor

    • Centralized Android test source definitions within the test suite.
    • Reduced reliance on external test resource files.
    • Simplified maintenance of Android-related regression tests.

@CLAassistant

CLAassistant commented Sep 9, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9df49ffd-6258-4518-a193-fc18ede1e1ec

📥 Commits

Reviewing files that changed from the base of the PR and between 77674c2 and 585f7ae.

📒 Files selected for processing (1)
  • nullaway/src/test/java/com/uber/nullaway/JakartaPersistenceTests.java
💤 Files with no reviewable changes (1)
  • nullaway/src/test/java/com/uber/nullaway/JakartaPersistenceTests.java

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

AndroidTest now passes inline Java text blocks to CompilationTestHelper.addSourceLines. The change covers core, AndroidX, and support-library fragment and activity tests. Android stub classes and test fixtures were deleted from test resources. Deprecated suppression annotations were removed.

Suggested reviewers: msridhar

Priority: ⬇️ Low

Change: Refactor

Merge Risk: ⚪ Minimal · up to 585f7

The Android test-source migration preserves the prior stub and fixture definitions with inline sources. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: replacing deprecated addSourceFile calls in AndroidTest.
Description check ✅ Passed The description directly explains the replacement of addSourceFile calls, source inlining, file deletions, suppression removal, and test status.
Linked Issues check ✅ Passed The changes satisfy issue #1425. AndroidTest replaces the deprecated CompilationTestHelper#addSourceFile calls with addSourceLines. The relevant Android stubs and test sources are inline Java te…
Out of Scope Changes check ✅ Passed The changes stay within issue #1425. The added AndroidTest constants and inline sources replace the deleted test stubs and fixtures. The suppression removal is deprecation cleanup related to the sam…
  • Fix all pre-merge checks with AI
✨ 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.

@msridhar

msridhar commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

@jabrailkhalil you'll need to sign the CLA before we can review this

… of the deprecatedcom.google.errorprone.CompilationTestHelper#addSourceFile inAndroidTest to addSourceLines with inlined Java text blocks, andremove the android testdata source files. Also drop the class-level@SuppressWarnings("deprecation") annotation, which is no longerneeded.Fixes #1425Signed-off-by: jabrailkhalil <jabrailkhalil@gmail.com>
@jabrailkhalil
jabrailkhalil force-pushed the fix-1425-addsource-lines branch from 5701389 to e4226a2 Compare September 10, 2026 12:28
@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

Thanks @msridhar! The CLA has been signed. Ready for review whenever you have time.

@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

The CI workflows for the current head e4226a24ff are waiting for contributor workflow approval (action_required): GitHub Actions Security Analysis with zizmor 🌈, Continuous integration. Could a maintainer approve the pending runs when convenient? These runs have not executed their checks yet; I will address any failures once they run.

The CLA is already signed; this request concerns the separate workflow-approval gate.

@msridhar msridhar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

couple minor things

@RunWith(JUnit4.class)
@SuppressWarnings("deprecation")
public class AndroidTest {
private static final String CORE_FRAGMENT_STUB =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe ANDROID_APP_FRAGMENT_STUB? There is a CoreFragment class used in a test later that makes this name confusing

}
""";

private static final String CORE_ACTIVITY_STUB =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similarly, maybe ANDROID_APP_CORE_ACTIVITY_STUB

@msridhar

Copy link
Copy Markdown
Collaborator

@jabrailkhalil does this really fully fix #1425? As in there are no more deprecation suppressions anywhere in tests due to that issue?

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.69%. Comparing base (f866bc9) to head (585f7ae).

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1839   +/-   ##
=========================================
  Coverage     87.69%   87.69%           
  Complexity     3450     3450           
=========================================
  Files           110      110           
  Lines         11475    11475           
  Branches       2355     2355           
=========================================
  Hits          10063    10063           
  Misses          652      652           
  Partials        760      760           

☔ View full report in Codecov by Harness.
📢 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.

@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

Addressed both naming comments in 71a0d05:

  • CORE_FRAGMENT_STUBANDROID_APP_FRAGMENT_STUB
  • CORE_ACTIVITY_STUBANDROID_APP_CORE_ACTIVITY_STUB

I attempted the requested targeted :nullaway:test --tests "com.uber.nullaway.AndroidTest", but this host only has JDK 17 while the project now requires JDK 21+; CI can run the test with the required toolchain.

@msridhar

Copy link
Copy Markdown
Collaborator

@jabrailkhalil does this really fully fix #1425? As in there are no more deprecation suppressions anywhere in tests due to that issue?

@jabrailkhalil still need an answer to this question

@msridhar msridhar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I removed one more @SuppressWarnings so now #1425 I believe is completely fixed. Thanks!

@msridhar
msridhar enabled auto-merge (squash) September 15, 2026 15:26
@msridhar
msridhar merged commit fd8a4dc into uber:master Sep 15, 2026
18 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