Fix more non-idempotent unit tests #14172
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 #14135, these tests employs the ClassGenerator to configure a custom class with hard-coded names, and then invokes
ClassGenerator.toClass()
to instantiate the desiredClass<?>
object. However, withinClassGenerator.toClass()
, the invoked methodjavassist.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:
Fix: Assign a unique ID to each class to be generated in the same classloader.
ChannelHandlerDispatcherTest#test
Reason: The
MockChannelHandler
class has stores metrics likesentCount
andconnectedCount
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 likesentCount
will be an accumulative count across multiple test runs. This will make the assertions fail.Error message in the repeated run:
Fix: Add a
reset()
methods to clear all metrics, and call thisreset()
method after each test execution.RpcStatusTest#testStatistics, RpcStatusTest#testBeginCountEndCount and RpcStatusTest#testBeginCountEndCountInMultiThread
Reason: For each test method,
RpcStatus
records metrics retrieved by methods likegetTotal()
. However, the metrics are not reset before the repeated runs of the same test method, so the values returned by methods likegetTotal()
would be an accumulative count of multiple test runs, causing the assertions fail.Error message of one of the tests in the repeated run:
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 number17979
. 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:
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:
Fix: Extract
prometheusExporterHttpServer
as a field and stop it in thetearDown()
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
tonull
in itstearDown()
method. This causes repeated runs to fail to find the corresponding configuration. There's really no need to clearPROVIDER_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:
Fix: Do not set
PROVIDER_APPLICATION_NAME
tonull
when tearing down.ReferenceCacheTest#testGetCacheDiffReference
Reason: Though the
XxxMockReferenceConfig
instanceconfigCopy
is re-constructed in each test execution, theconfigCopy.getCounter()
will return accumulative counts across different runs sincecounter
is static (of global scope). Therefore, the assertionassertEquals(0L, configCopy.getCounter());
will fail in repeated runs.Error Message:
Fix: Call
XxxMockReferenceConfig.setCounter(0)
in thesetUp()
method.LiveTest#testExecute
Reason: The test sets the check return value of
MockLivenessProbe
totrue
during the test execution, but does not reset it back tofalse
when the test finishes. Therefore,Assertions.assertEquals(result, "false");
will fail in the repeated run.Error Message:
Fix: Ensure the return value of
MockLivenessProbe
is set to the default value (false
) in thetearDown()
method.Verifying this change
After the patch, running the tests repeatedly in the same environment will not lead to failures.
Checklist