Skip to content

[ISSUE #10823] Optimize BrokerShutdownTest execution time - #10824

Draft
fuyou001 wants to merge 1 commit into
apache:developfrom
fuyou001:agent/optimize-broker-shutdown-test
Draft

[ISSUE #10823] Optimize BrokerShutdownTest execution time#10824
fuyou001 wants to merge 1 commit into
apache:developfrom
fuyou001:agent/optimize-broker-shutdown-test

Conversation

@fuyou001

@fuyou001 fuyou001 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Which Issue(s) This PR Fixes

Brief Description

BrokerShutdownTest currently starts and stops a complete broker four times. Each lifecycle takes roughly 15-25 seconds, so the test class exceeds Bazel's 60-second small-test timeout.

This change keeps the default broker configuration and reduces redundant lifecycle setup from four instances to two:

  • combines graceful-shutdown and resource-cleanup assertions in one lifecycle;
  • retains shutdown from another thread and propagates worker failures through Future#get;
  • verifies MessageStore reaches SHUTDOWN_OK after both shutdown paths;
  • retains repeated-shutdown coverage;
  • removes the former ordering test's unused flags and unconditional assertion, which did not actually observe shutdown order;
  • adds teardown cleanup for assertion or timeout failure paths.

Only test code is changed. There is no change to broker APIs, wire protocols, configuration, persisted data, or production shutdown behavior.

How Did You Test This Change?

Environment:

  • OpenJDK 21.0.12 (Homebrew, aarch64)
  • Apache Maven 3.9.9

Command, run three times:

mvn -q -pl broker -am -Dtest=BrokerShutdownTest \
  -DfailIfNoTests=false -DskipITs -Dspotbugs.skip=true \
  -Djacoco.skip=true test

Results:

  • 46.375 seconds: 2 tests, 0 failures, 0 errors
  • 46.055 seconds: 2 tests, 0 failures, 0 errors
  • 39.894 seconds: 2 tests, 0 failures, 0 errors

Also verified with git diff --check.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.25%. Comparing base (2daf0e2) to head (378d4e3).

Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10824      +/-   ##
=============================================
- Coverage      48.32%   48.25%   -0.07%     
+ Complexity     13516    13501      -15     
=============================================
  Files           1380     1380              
  Lines         101138   101138              
  Branches       13120    13120              
=============================================
- Hits           48876    48809      -67     
- Misses         46298    46345      +47     
- Partials        5964     5984      +20     

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

@RockteMQ-AI RockteMQ-AI 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.

Summary

Optimizes BrokerShutdownTest by reducing redundant broker lifecycle setups from 4 to 2, keeping the test within Bazel's 60-second small-test timeout. Changes look solid.

Findings

  • [Info] Good use of System.nanoTime() over System.currentTimeMillis() for elapsed-time measurement — avoids wall-clock skew.
  • [Info] Promoting brokerController to an instance field with @After cleanup is a good pattern for test isolation.
  • [Info] The merged testBrokerGracefulShutdownAndResourceCleanup correctly verifies state-machine transitions (SHUTDOWN_OK) instead of just boolean flags — more precise assertions.
  • [Info] testShutdownFromAnotherThread using ExecutorService/Future is cleaner than raw Thread + CountDownLatch.

Verdict

LGTM — well-structured test optimization with improved assertions and proper cleanup.


Automated review by github-manager-bot

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] Optimize BrokerShutdownTest to complete within the Bazel small timeout

3 participants