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

[SCB-2718] Fix flaky test failure in TestOperationGenerator#apiOperationThenResponse #3450

Merged

Conversation

MohsenDehghankar
Copy link
Contributor

What is the purpose of this PR

  • This PR fixes the flaky test org.apache.servicecomb.swagger.generator.core.TestOperationGenerator.apiOperationThenResponse
  • The mentioned test may fail or pass without changes made to the source code when it is run in different JVMs due to the non-deterministic order of the returned array from getDeclaredAnnotations. Because there is no assumption on the order of the returned array from this method in Java documentation.

Why the test fails

  • When processing the annotations of a method in AbstractOperationGenerator#scanMethodAnnotations(), the result of method.getAnnotations() is used which uses AccessibleObject.getDeclaredAnnotations() which does not guarantee a deterministic result order based on Java specifications.
  • This results in processing annotations in arbitrary order, so the expected assertions in both TestOperationGenerator#apiOperationThenResponse and TestOperationGenerator#responseThenApiOperation might fail due to this change in orders of annotations.

Reproduce the test failure

  • Run the test with 'NonDex' maven plugin. The command to recreate the flaky test failure is mvn edu.illinois:nondex-maven-plugin:2.1-SNAPSHOT:nondex -pl swagger/swagger-generator/generator-core -Dtest=TestOperationGenerator#apiOperationThenResponse
  • Fixing the flaky test now may prevent flaky test failures in future Java versions.

Expected Result

  • The tests should run successfully when run with 'NonDex' maven with the same command for recreating the flaky test.

Actual Result

  • We get the following error:
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.258 s <<< FAILURE! - in org.apache.servicecomb.swagger.generator.core.TestOperationGenerator
[ERROR] org.apache.servicecomb.swagger.generator.core.TestOperationGenerator.apiOperationThenResponse  Time elapsed: 0.25 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <null> but was: <io.swagger.models.properties.StringProperty@8f5d1e11>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertNull.failNotNull(AssertNull.java:50)
        at org.junit.jupiter.api.AssertNull.assertNull(AssertNull.java:35)
        at org.junit.jupiter.api.AssertNull.assertNull(AssertNull.java:30)
        at org.junit.jupiter.api.Assertions.assertNull(Assertions.java:276)
        at org.apache.servicecomb.swagger.generator.core.TestOperationGenerator.apiOperationThenResponse(TestOperationGenerator.java:114)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
        at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
        at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
        at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
        at org.junit.vintage.engine.execution.RunnerExecutor.execute(RunnerExecutor.java:42)
        at org.junit.vintage.engine.VintageTestEngine.executeAllChildren(VintageTestEngine.java:80)
        at org.junit.vintage.engine.VintageTestEngine.execute(VintageTestEngine.java:72)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
        at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
        at org.apache.maven.surefire.junitplatform.LazyLauncher.execute(LazyLauncher.java:55)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:223)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:175)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:139)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
        at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
        at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581)

Fix

  • First, we should sort the method.getAnnotations() result deterministically.
  • Secondly, there should be no different in the assertEqual expressions between these two tests: apiOperationThenResponse and responseThenApiOperation

@liubao68
Copy link
Contributor

liubao68 commented Nov 3, 2022

Thanks for your PR.

@omkrpt
Copy link

omkrpt commented Feb 6, 2024

This PR also fixes the flaky test org.apache.servicecomb.swagger.generator.core.TestOperationGenerator.responseThenApiOperation

To test this fix:
In swagger/swagger-generator/generator-core/pom.xml set parent artifact swagger-generator version from 2.9.0-SNAPSHOT to 2.8.1

Checkout the last commit before fix (5f02b0a33b51dcc93fe96c833311cc0fc8d444e7) and run the test with:
mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -pl swagger/swagger-generator/generator-core -Dtest=TestOperationGenerator#responseThenApiOperation

You will get failures.

Then checkout the commit with the fix (b780639a167ca09daaae28535c91c3b022f5d27c) and run the same. The test should no longer fail.

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

3 participants