Skip to content

MINOR: Replace try/fail/catch with assertThrows in producer tests - #23277

Open
hjtiun852 wants to merge 1 commit into
apache:trunkfrom
hjtiun852:minor-assertthrows-producer-tests
Open

MINOR: Replace try/fail/catch with assertThrows in producer tests#23277
hjtiun852 wants to merge 1 commit into
apache:trunkfrom
hjtiun852:minor-assertthrows-producer-tests

Conversation

@hjtiun852

@hjtiun852 hjtiun852 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Modernize six exception assertions in ProducerRecordTest, RecordSendTest and MockProducerTest from the old try/call/fail/catch idiom to assertThrows.

assertThrows fails automatically when no exception is thrown, so a missing fail() can no longer let a test pass silently, and it pins the exact expected type. Where the original catch block asserted on the wrapped cause, the assertThrows return value preserves that check:

ExecutionException err = assertThrows(ExecutionException.class, md2::get);
assertEquals(e, err.getCause());

Follows the same cleanup as #19975.

Testing: ./gradlew :clients:test for the three test classes plus checkstyle, all green.
Reviewers: Ken Huang s7133700@gmail.com

Modernize six exception assertions in ProducerRecordTest, RecordSendTest
and MockProducerTest from the old try/call/fail/catch idiom to assertThrows.

assertThrows fails automatically when no exception is thrown, so a missing
fail() can no longer let a test pass silently, and it pins the exact
expected type. Where the original catch block asserted on the wrapped
cause, the assertThrows return value preserves that check:

    ExecutionException err = assertThrows(ExecutionException.class, md2::get);
    assertEquals(e, err.getCause());

Follows the same cleanup as apache#19975.

Testing: ./gradlew :clients:test for the three test classes plus checkstyle, all green.
@github-actions github-actions Bot added triage PRs from the community producer tests Test fixes (including flaky tests) clients small Small PRs labels Aug 25, 2026

@m1a2st m1a2st left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you also take a look at the following tests?

  • BufferPoolTest
  • ProducerBatchTest
  • ProducerMetadataTest
  • RecordAccumulatorTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved clients producer small Small PRs tests Test fixes (including flaky tests) triage PRs from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants