kaiyaok2 opened a new issue, #14171:
URL: https://github.com/apache/dubbo/issues/14171

   ### Pre-check
   
   - [X] I am sure that all the content I provide is in English.
   
   
   ### Search before asking
   
   - [X] I had searched in the 
[issues](https://github.com/apache/dubbo/issues?q=is%3Aissue) and found no 
similar issues.
   
   
   ### Apache Dubbo Component
   
   Java SDK (apache/dubbo)
   
   ### Dubbo Version
   
   Dubbo 3.2, OpenJDK 17, Ubuntu 22.04
   
   ### Steps to reproduce this issue
   
   Run the test twice in the same JVM. For example, for 
`org.apache.dubbo.common.bytecode.ClassGeneratorTest#test` (using the 
`NIOInspector` plugin):
   ```
   cd dubbo-common
   mvn clean install
   mvn edu.illinois:NIOInspector:rerun 
-Dtest=org.apache.dubbo.common.bytecode.ClassGeneratorTest#test
   ```
   
   ### What you expected to happen
   
   The unit test should be idempotent and pass deterministically in repeated 
runs. However, the following tests are non-idempotent:
   ### `ClassGeneratorTest#test` and `ClassGeneratorTest#testMain0`
   Reason: Similar as described in https://github.com/apache/dubbo/pull/14135, 
these tests employs the ClassGenerator to configure a custom class with 
**hard-coded names**, and then invokes `ClassGenerator.toClass()` to 
instantiate the desired `Class<?>` object. However, within 
`ClassGenerator.toClass()`, the invoked method 
`javassist.ClassPool.makeClass()` is called to create the class. Given that 
Javassist freezes a class upon its initial loading (ensuring classes cannot be 
altered or removed at runtime), repeated invocations of the two unit tests fail 
as the class names remain unchanged, leading to class generation errors.
   
   Error message of one of the tests in the repeated run:
   ```
   java.lang.RuntimeException: org.apache.dubbo.common.bytecode.Bean$Builder: 
frozen class (cannot edit)
   ```
   Fix: Assign a unique ID to each class to be generated in the same 
classloader.
   
   ### `ChannelHandlerDispatcherTest#test`
   Reason: The `MockChannelHandler` class has stores metrics like `sentCount` 
and `connectedCount` as static variables. However, there're no reset methods to 
clear these metrics after a round of test execution. Therefore, in the repeated 
runs, metrics like `sentCount` will be an accumulative count across multiple 
test runs. This will make the assertions fail.
   
   Error message in the repeated run:
   ```
   org.opentest4j.AssertionFailedError: expected: <4> but was: <2>
   ```
   Fix: Add a `reset()` methods to clear all metrics, and call this `reset()` 
method after each test execution.
   
   ### `RpcStatusTest#testStatistics`, `RpcStatusTest#testBeginCountEndCount` 
and `RpcStatusTest#testBeginCountEndCountInMultiThread`
   Reason: For each test method, `RpcStatus` records metrics retrieved by 
methods like `getTotal()`. However, the metrics are not reset before the 
repeated runs of the same test method, so the values returned by methods like 
`getTotal()` would be an accumulative count of multiple test runs, causing the 
assertions fail.
   
   Error message of one of the tests in the repeated run:
   ```
   org.opentest4j.AssertionFailedError: expected: <8> but was: <4>
   ```
   Fix: Call `RpcStatus.removeStatus()` to reset the metrics corresponding to 
the url and the method name for each test at the start of test execution.
   
   ### `DubboMonitorTest#testMonitorFactory`
   Reason: `monitorFactory.getMonitor()` uses the hard-coded port number 
`17979`. However, the port will be occupied by the first test execution, so 
repeated run will fail to start the server since the address has already been 
used.
   
   Error message:
   ```
   org.apache.dubbo.rpc.RpcException: Fail to start server(url: 
dubbo://127.0.0.1:17979/org.apache.dubbo.monitor.MonitorService?channel.readonly.sent=true&codec=dubbo&heartbeat=60000)
 Failed to bind NettyServer on /0.0.0.0:17979, cause: Address already in use
   ```
   
   Fix: Find a free port using `java.net.ServerSocket` and use it.
   
   ### `PrometheusMetricsThreadPoolTest#testExporterThreadpoolName` and 
`PrometheusMetricsReporterTest#testExporter`
   Reason: The tests attempt to create a `prometheusExporterHttpServer` in each 
execution. However, in the first execution, the server is created but not shut 
down after execution. Therefore, repeated test executions will throw a runtime 
exception since the address corresponding to the server is already in use.
   
   Error Message:
   ```
   java.lang.RuntimeException: java.net.BindException: Address already in use
   ```
   
   Fix: Extract `prometheusExporterHttpServer` as a field and stop it in the 
`tearDown()` method to ensure each test execution starts with a fresh state.
   
   ### Register Center Integration Tests
   - `MultipleRegistryCenterInjvmIntegrationTest#integrate`
   - `SingleRegistryCenterInjvmIntegrationTest#integrate`
   - `MultipleRegistryCenterExportMetadataIntegrationTest#integrate`
   - `SingleRegistryCenterExportMetadataIntegrationTest#integrate`
   - `MultipleRegistryCenterServiceDiscoveryRegistryIntegrationTest#integrate`
   - `MultipleRegistryCenterExportProviderIntegrationTest#integrate`
   - `SingleRegistryCenterExportProviderIntegrationTest#integrate`
   Reason: These integration tests set the static string 
`PROVIDER_APPLICATION_NAME` to `null` in its `tearDown()` method. This causes 
repeated runs to fail to find the corresponding configuration. There's really 
no need to clear `PROVIDER_APPLICATION_NAME` in the reset method since each 
integration test class has its globally unique provide application name and 
they're not interdependent on each other.
   
   Error Message:
   ```
   java.lang.IllegalStateException: No application config found or it's not a 
valid config! Please add <dubbo:application name="..." /> to your spring config.
   ```
   
   Fix: Do not set `PROVIDER_APPLICATION_NAME` to `null` when tearing down.
   
   ### `ReferenceCacheTest#testGetCacheDiffReference`
   Reason: Though the `XxxMockReferenceConfig` instance `configCopy` is 
re-constructed in each test execution, the `configCopy.getCounter()` will 
return accumulative counts across different runs since `counter` is static (of 
global scope). Therefore, the assertion `assertEquals(0L, 
configCopy.getCounter());` will fail in repeated runs.
   
   Error Message:
   ```
   org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
   ```
   
   Fix: Call `XxxMockReferenceConfig.setCounter(0)` in the `setUp()` method.
   
   ### `LiveTest#testExecute`
   Reason: The test sets the check return value of `MockLivenessProbe` to 
`true` during the test execution, but does not reset it back to `false` when 
the test finishes. Therefore, `Assertions.assertEquals(result, "false");` will 
fail in the repeated run.
   
   Error Message:
   ```
   org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
   ```
   
   Fix: Ensure the return value of `MockLivenessProbe` is set to the default 
value (`false`) in the `tearDown()` method.
   
   ### Anything else
   
   _No response_
   
   ### Are you willing to submit a pull request to fix on your own?
   
   - [X] Yes I am willing to submit a pull request on my own!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of 
Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to