Refactor ContractsTests to use Java text blocks by cobayo · Pull Request #1435 · uber/NullAway

cobayo · 2026-01-11T01:28:51Z

Following up on my previous PR and the goal of issue #1425, I have refactored ContractsTests.java.

Migrated test data from external files to inline text blocks using addSourceLines.
Deleted physical test files: CheckContractPositiveCases.java and CheckContractNegativeCases.java.
Removed unnecessary @SuppressWarnings("deprecation") annotations.
Part of the ongoing effort to modernize tests (Replace calls to deprecated com.google.errorprone.CompilationTestHelper#addSourceFile in tests #1425).

Thank you for contributing to NullAway!

Please note that once you click "Create Pull Request" you will be asked to sign our Uber Contributor License Agreement via CLA assistant.

Before pressing the "Create Pull Request" button, please provide the following:

A description about what and why you are contributing, even if it's trivial.
The issue number(s) or PR number(s) in the description if you are contributing in response to those.
If applicable, unit tests.

Summary by CodeRabbit

Tests
- Consolidated external test-data into inline test cases within the test suite for simpler maintenance and clearer test flow.
- Removed separate public test-data classes that previously existed as external resources.
- Dropped a deprecation-suppression annotation from the public test class to reflect current warnings handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-11T01:31:37Z

Warning

Rate limit exceeded

@cobayo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 19 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between af0bb6b and 8ccb264.

📒 Files selected for processing (3)

nullaway/src/test/java/com/uber/nullaway/ContractsTests.java
nullaway/src/test/resources/com/uber/nullaway/testdata/CheckContractNegativeCases.java
nullaway/src/test/resources/com/uber/nullaway/testdata/CheckContractPositiveCases.java

Walkthrough

Removed two external test resource files (CheckContractPositiveCases.java and CheckContractNegativeCases.java) and inlined their contents into the ContractsTests test class via addSourceLines. Also removed the class-level @SuppressWarnings("deprecation") annotation from ContractsTests. The changes relocate multiple @Contract-annotated test cases from src/test/resources into the test code and delete the original public testdata classes.

Possibly related PRs

Convert test data in main NullAway tests to use text blocks #1424: Converts external test sources into inlined multi-line test source strings within the test scaffolding, similar inlining change.
Accept any annotation with simple name Contract, and change reporting of invalid contract annotations #1295: Modifies contract-related tests and testdata handling for @Contract annotations, touching the same test area.
Add a test for a useless @Contract #1346: Changes ContractsTests.java and updates contract-related test cases, affecting the same file.

Suggested reviewers

yuxincs
msridhar

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main refactoring objective: migrating ContractsTests to use Java text blocks instead of external test files.

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

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.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @nullaway/src/test/java/com/uber/nullaway/ContractsTests.java:
- Around line 16-79: The virtual filenames passed to addSourceLines should match
the declared package; update the first argument strings for the test sources so
they use package-qualified paths that match package com.uber.nullaway.testdata
(e.g., change "CheckContractPositiveCases.java" to
"com.uber.nullaway.testdata.CheckContractPositiveCases.java" or the test
framework's expected package-qualified form) for the CheckContractPositiveCases
class (and do the same for CheckContractNegativeCases and any other
addSourceLines entries that declare package com.uber.nullaway.testdata) so
diagnostics/searchability align with the declared package.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b8a577 and 0db2411.

📒 Files selected for processing (1)

nullaway/src/test/java/com/uber/nullaway/ContractsTests.java

🧰 Additional context used

🧠 Learnings (1)

📚 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/ContractsTests.java

🔇 Additional comments (1)

nullaway/src/test/java/com/uber/nullaway/ContractsTests.java (1)

16-79: LGTM: inlining the former testdata sources reads cleanly and should keep behavior stable.
Good simplification by removing external test files and keeping the // BUG: Diagnostic contains: assertions close to the tests.

Also applies to: 119-155

@SuppressWarnings

- Migrated test data from external files to inline text blocks using addSourceLines. - Deleted physical test files: CheckContractPositiveCases.java and CheckContractNegativeCases.java. - Removed unnecessary @SuppressWarnings("deprecation") annotations. - Part of the ongoing effort to modernize tests (uber#1425).

msridhar

LGTM, thank you!

codecov · 2026-01-11T21:10:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.44%. Comparing base (4b4d1e8) to head (8ccb264).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1435   +/-   ##
=========================================
  Coverage     88.44%   88.44%           
  Complexity     2672     2672           
=========================================
  Files            97       97           
  Lines          8852     8852           
  Branches       1772     1772           
=========================================
  Hits           7829     7829           
  Misses          507      507           
  Partials        516      516

☔ 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.

@SuppressWarnings

Following up on my previous PR and the goal of issue uber#1425, I have refactored ContractsTests.java. - Migrated test data from external files to inline text blocks using addSourceLines. - Deleted physical test files: CheckContractPositiveCases.java and CheckContractNegativeCases.java. - Removed unnecessary @SuppressWarnings("deprecation") annotations. - Part of the ongoing effort to modernize tests (uber#1425). Thank you for contributing to NullAway! Please note that once you click "Create Pull Request" you will be asked to sign our [Uber Contributor License Agreement](https://cla-assistant.io/uber/NullAway) via [CLA assistant](https://cla-assistant.io/). Before pressing the "Create Pull Request" button, please provide the following: - [x] A description about what and why you are contributing, even if it's trivial. - [x] The issue number(s) or PR number(s) in the description if you are contributing in response to those. - [x] If applicable, unit tests.  ## Summary by CodeRabbit * **Tests** * Consolidated external test-data into inline test cases within the test suite for simpler maintenance and clearer test flow. * Removed separate public test-data classes that previously existed as external resources. * Dropped a deprecation-suppression annotation from the public test class to reflect current warnings handling. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

coderabbitai Bot reviewed Jan 11, 2026

View reviewed changes

Comment thread nullaway/src/test/java/com/uber/nullaway/ContractsTests.java

cobayo force-pushed the issue-1425-contractstests branch from af0bb6b to 8ccb264 Compare January 11, 2026 01:50

msridhar approved these changes Jan 11, 2026

View reviewed changes

msridhar enabled auto-merge (squash) January 11, 2026 21:03

msridhar merged commit 80fbdc9 into uber:master Jan 11, 2026
11 checks passed

coderabbitai Bot mentioned this pull request Jan 13, 2026

Migrate AutoValue to Records #1415

Merged

coderabbitai Bot mentioned this pull request Jan 21, 2026

Improve verification of !null -> !null contracts #1441

Merged

coderabbitai Bot mentioned this pull request Apr 11, 2026

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ContractsTests to use Java text blocks#1435

Refactor ContractsTests to use Java text blocks#1435
msridhar merged 1 commit intouber:masterfrom
cobayo:issue-1425-contractstests

cobayo commented Jan 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 11, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

msridhar left a comment

Uh oh!

codecov Bot commented Jan 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cobayo commented Jan 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msridhar left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cobayo commented Jan 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 11, 2026 •

edited

Loading

codecov Bot commented Jan 11, 2026 •

edited

Loading