incubator-seata icon indicating copy to clipboard operation
incubator-seata copied to clipboard

test: bugfix brittle RPC status test

Open cperkkk opened this issue 3 weeks ago • 2 comments

Ⅰ. Describe what this PR did

This pull request addresses several flaky test, in the apache/incubator-seata project. The test was failing intermittently due to assumptions about the ordering of the test. I've noticed that when running the test in specific ordering will produce a test failure (maven surefire plugin unfortunately does not guarantee test ordering), such as:

mvn test -Dtest=org.apache.seata.common.rpc.RpcStatusTest#CbeginCount,org.apache.seata.common.rpc.RpcStatusTest#AendCount,org.apache.seata.common.rpc.RpcStatusTest#BremoveStatus

The tests will fail with output:

org.opentest4j.AssertionFailedError: expected: <-1> but was: <0>
	at org.apache.seata.common.rpc.RpcStatusTest.AendCount(RpcStatusTest.java:54)

This is due to the necessity of the count being set by beginCount first before the eventual endCount runs, but the test fails to explicitly mention ordering (such as using import org.junit.jupiter.api.Order; etc), making this a flaky and brittle test depending on the version of surefire plugin used / if there's a method rename.

Ⅱ. Does this pull request fix one issue?

It does not fixes a particular github issue.

Ⅲ. Why don't you add test cases (unit test/integration test)?

I've only changed test and not code under test

Ⅳ. Describe how to verify it

We can permute the run order, after fixing the test, any run order by surefire will pass the test

mvn test -Dsurefire.runOrder=alphabetical -Dtest=org.apache.seata.common.rpc.RpcStatusTest#beginCount,org.apache.seata.common.rpc.RpcStatusTest#removeStatus,org.apache.seata.common.rpc.RpcStatusTest#endCount
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

Ⅴ. Special notes for reviews

  • The fix ensures the test is robust to non-deterministic behaviors of ordering, making it compatible with future updates to it or related dependencies.

Let me know if further improvements or clarifications are needed!

cperkkk avatar Nov 29 '25 23:11 cperkkk

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 71.05%. Comparing base (b27e6be) to head (2344adb).

Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #7819      +/-   ##
============================================
+ Coverage     71.03%   71.05%   +0.01%     
  Complexity      797      797              
============================================
  Files          1294     1294              
  Lines         49528    49528              
  Branches       5873     5873              
============================================
+ Hits          35182    35191       +9     
+ Misses        11437    11428       -9     
  Partials       2909     2909              

see 8 files with indirect coverage changes

Impacted file tree graph

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 09 '25 03:12 codecov[bot]

Please apply mvn spotless:apply

lokidundun avatar Dec 09 '25 04:12 lokidundun

Please apply mvn spotless:apply

Okay, spotless:apply applied, thanks

cperkkk avatar Dec 12 '25 13:12 cperkkk