kaiyaok2 opened a new pull request, #14172: URL: https://github.com/apache/dubbo/pull/14172
Fixes #14171. # The Problem Followup of #14135 - fixing more non-idempotent unit tests (tests that pass in the first run but fails in repeated runs in the same environment in isolation) # All Non-Idempotent Tests & Proposed Fix ## 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. # Verifying this change After the patch, running the tests repeatedly in the same environment will not lead to failures. <!-- Follow this checklist to help us incorporate your contribution quickly and easily: --> ## Checklist - [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue. - [x] Each commit in the pull request should have a meaningful subject line and body. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] GitHub Actions works fine on your own branch. -- 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]
