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

Fixed non-idempotent test DeploymentTest#testDeployClass #5190

Closed

Conversation

kaiyaok2
Copy link

@kaiyaok2 kaiyaok2 commented Apr 25, 2024

Motivation:

The test io.vertx.core.DeploymentTest.testDeployClass is not idempotent and fails in the second run in the same JVM, because it pollutes state shared among tests. Specifically, it deploys a vertical but did not clean up ReferenceSavingMyVerticle.myVerticles after the deployment, hence the assertion assertEquals(deploymentId, myVerticle.deploymentID); can fail when myVerticles still contains verticals from the previous run. It shall be good to clean this state pollution so that other tests do not fail in the future due to the shared state polluted by this test.

##Stacktrace of failure in the second run:

org.junit.ComparisonFailure: expected:<3[7b3fe25-12dd-4a61-b70d-734f577c4b0a]> but was:<3[e168fd5-0d9b-40ff-8801-d20b74837de8]>
	at org.junit.Assert.assertEquals(Assert.java:117)
	at org.junit.Assert.assertEquals(Assert.java:146)
	at io.vertx.test.core.AsyncTestBase.assertEquals(AsyncTestBase.java:360)
	at io.vertx.core.DeploymentTest.lambda$testDeployClass$126(DeploymentTest.java:1189)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at io.vertx.core.DeploymentTest.lambda$testDeployClass$127(DeploymentTest.java:1188)

Proposed Fix

Clear ReferenceSavingMyVerticle.myVerticles before starting the individual tests.

Conformance:

ECA Submitted

@tsegismont
Copy link
Contributor

Thanks for sharing your findings. Can you explain in which circumstances the same test would be executed twice in the same JVM ? Afaik, we've never seen this problem in CI.

@kaiyaok2
Copy link
Author

@tsegismont You're right that tests usually don't get re-executed in the same environment in the CI/CD pipeline. Certain mechanisms do support retrying tests that does not pass in the first run due to timeout / race conditions though. However, the major reason of executing a test twice in one environment is to ensure that all unit tests are self-contained.

This would help maintaining test isolation by ensuring that the state of the system under test is consistent at the beginning of each test, regardless of previous test runs. Fixing non-idempotent tests can help proactively avoid state pollution that may result in test order dependency (which could then hurt regression testing under test selection / prioritization / parallelization).

@tsegismont
Copy link
Contributor

tsegismont commented May 21, 2024

Certain mechanisms do support retrying tests that does not pass in the first run due to timeout / race conditions

Which mechanisms are you referring to?

@kaiyaok2
Copy link
Author

@tsegismont examples include the @RepeatedTest annotation introduced since JUnit 5, and the retryAnalyzer attribute of TestNG.

@tsegismont
Copy link
Contributor

Thanks for providing the details. So this test is not marked for repeated execution. Should this change, the test would need to be adapted of course. But for now this is not necessary.

@tsegismont tsegismont closed this May 21, 2024
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.

None yet

2 participants