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

PR at the official repository

@darko-marinov
Copy link
Contributor

Please squash the two commits. We don't need two commits to change one line.

…ecode.WrapperTest.est_getMethodNames_ContainExtendsParentMethods
@Hussain-Safwan
Copy link
Contributor Author

Please squash the two commits. We don't need two commits to change one line.

I have squashed the commits.

@darko-marinov
Copy link
Contributor

Great. Thanks.

@darko-marinov darko-marinov merged commit 772f341 into TestingResearchIllinois:main Oct 4, 2025
1 check 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.

2 participants