Skip to content

Conversation

@Hussain-Safwan
Copy link
Contributor

What is the purpose of this PR

  • The purpose of this PR is to resolve the unpredictability caused by the flaky test: org.apache.dubbo.common.bytecode.WrapperTest.est_getMethodNames_ContainExtendsParentMethods.
  • The test passes/fails based on the ordering of the names of methods returned bygetMethodNames methods from Wrapper and ClassUtils classes.

Why the test fail

  • The test asserts whether a certain array containing the names of the methods is available in the aforementioned classes.
  • The problem, however, arises because the order in which the names would be returned is not guaranteed by the method used to retrieve the names.
  • But assertArrayEquals expects even the order of the array elements to match - hence sometimes causing the failure.

How to reproduce the test failure

  • The scenario may be reproduced by running the tests with Nondex tool:

  • `mvn -pl dubbo-common edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=org.apache.dubbo.common.bytecode.WrapperTest#test_getMethodNames_ContainExtendsParentMethods`
    
  • Instructions on Nondex installation may be found here.

Expected results

The test should run successfully with Nondex instrumentation.

Actual Results

  • Following error result is obtained upon running the original code with Nondex:
  • [ERROR] Failed to execute goal edu.illinois:nondex-maven-plugin:2.1.7:nondex (default-cli) on project dubbo-common: Unable to execute mojo: There are test failures.
  • Evidence that the test is indeed failing due to the difference in ordering can be observed in the following line:
  • [ERROR] WrapperTest.test_getMethodNames_ContainExtendsParentMethods:184 array contents differ at index [0], expected: <hello> but was: <world>

Description of fix

  • The idea is to sort the array containing the names of methods before asserting.
  • This can be performed using the sort method of java.util as in Arrays.sort(methodNames).

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.00%. Comparing base (d45ce97) to head (77bc1c3).

Additional details and impacted files
@@             Coverage Diff              @@
##                3.3   #15683      +/-   ##
============================================
- Coverage     61.01%   61.00%   -0.01%     
+ Complexity    11698    11692       -6     
============================================
  Files          1923     1923              
  Lines         87069    87069              
  Branches      13112    13112              
============================================
- Hits          53124    53116       -8     
- Misses        28495    28500       +5     
- Partials       5450     5453       +3     
Flag Coverage Δ
integration-tests-java21 32.90% <ø> (ø)
integration-tests-java8 32.96% <ø> (-0.01%) ⬇️
samples-tests-java21 32.63% <ø> (-0.02%) ⬇️
samples-tests-java8 30.33% <ø> (+0.03%) ⬆️
unit-tests-java11 58.98% <ø> (-0.04%) ⬇️
unit-tests-java17 58.74% <ø> (-0.02%) ⬇️
unit-tests-java21 58.74% <ø> (-0.01%) ⬇️
unit-tests-java8 58.99% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@zrlw zrlw left a comment

Choose a reason for hiding this comment

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

LGTM

@zrlw zrlw requested a review from RainYuY September 10, 2025 04:00
@zrlw zrlw requested a review from oxsean September 18, 2025 06:05
Copy link
Contributor

@oxsean oxsean left a comment

Choose a reason for hiding this comment

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

LGTM.

@zrlw zrlw merged commit 956a47b into apache:3.3 Sep 18, 2025
32 checks passed
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.

4 participants