Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Non-idempotent unit tests (follow-up of #14134) #14171

Closed
4 tasks done
kaiyaok2 opened this issue May 10, 2024 · 0 comments · Fixed by #14172
Closed
4 tasks done

[Bug] Non-idempotent unit tests (follow-up of #14134) #14171

kaiyaok2 opened this issue May 10, 2024 · 0 comments · Fixed by #14172
Labels
component/need-triage Need maintainers to triage type/need-triage Need maintainers to triage

Comments

@kaiyaok2
Copy link
Contributor

kaiyaok2 commented May 10, 2024

Pre-check

  • I am sure that all the content I provide is in English.

Search before asking

  • I had searched in the issues 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 #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?

  • Yes I am willing to submit a pull request on my own!

Code of Conduct

@kaiyaok2 kaiyaok2 added component/need-triage Need maintainers to triage type/need-triage Need maintainers to triage labels May 10, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Dubbo Board May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/need-triage Need maintainers to triage type/need-triage Need maintainers to triage
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant