Skip to content

fix: enforce exception assertion in HeartbeatListenerTest (#6594) - #6947

Open
yykaue wants to merge 2 commits into
apache:masterfrom
yykaue:fix-6594-heartbeat-assert-throws
Open

fix: enforce exception assertion in HeartbeatListenerTest (#6594)#6947
yykaue wants to merge 2 commits into
apache:masterfrom
yykaue:fix-6594-heartbeat-assert-throws

Conversation

@yykaue

@yykaue yykaue commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

Fixes #6594.

This PR replaces the catch-block-only assertion in HeartbeatListenerTest#testSendHeartbeatWithLoginFailure with assertThrows.

The test now:

  • Requires Method.invoke to throw an InvocationTargetException.
  • Verifies that the underlying cause is a RuntimeException.
  • Prevents the test from passing vacuously when no exception is thrown.

Verification

  • HeartbeatListenerTest: 8 tests passed.
  • Checkstyle passed with no violations.

@Aias00 Could you please help review this PR? Thank you!

@Aias00 Aias00 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.

PR #6947 Review — fix: enforce exception assertion in HeartbeatListenerTest (#6594)

Verdict: APPROVE

Scope

shenyu-register-client-beat HeartbeatListenerTest#testSendHeartbeatWithLoginFailure: replace
a catch-block-only assertion with assertThrows.

Verification

  • Old code wrapped sendHeartbeatMethod.invoke(...) in try/catch and only asserted inside catch,
    so the test could pass vacuously when no exception was thrown. New code uses
    assertThrows(InvocationTargetException.class, ...) then
    assertInstanceOf(RuntimeException.class, exception.getCause()), enforcing that the failure
    actually occurs and has the right cause.
  • Imports java.lang.reflect.InvocationTargetException, assertInstanceOf, assertThrows added
    and used; no production code touched.

Notes

  • Pure test-strengthening change; reduces false-green risk. Approving.

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.

[BUG] Catch-block-only assertion (passes vacuously if no exception)

2 participants