IBM / java-sdk-core

Core functionality required by Java code generated by the IBM Cloud OpenAPI SDK Generator (openapi-sdkgen)
https://ibm.github.io/java-sdk-core/
Apache License 2.0
20 stars 21 forks source link

WIP: Fix test failures that occur with "nondex" plugin #159

Closed HildoYe closed 2 years ago

HildoYe commented 2 years ago

This PR fixes several flaky tests:

  1. There are several flaky tests in DynamicModelSerializationTest. The root cause of them is that equals() compares two HashMap<String, T> dynamicProperties in DynamicModel. In this PR, LinkedHashMap is used instead of HashMap to resolve the ordering issue. This solution considers 2 DynamicModels are same if their elements have the same insertion order. However, the equals() function should be modified to completely resolve the ordering issue.
  2. There are several flaky tests in RequestBuilderTest. In these tests multiple pairs of JSON strings are compared with each other. In this PR, JSONAssert library is used to ignore the order difference when comparing 2 JSON strings.
  3. There are a couple of flaky test in RequestUtilsTest. The root cause of them is the inconsistency when comparing 2 maps. In this PR, the comparations are changed to match the style of other tests in the same files while ignoring the ordering issue.
CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

padamstx commented 2 years ago

Hi @HildoYe, could you perhaps provide more info about the environment where you are seeing test failures occur? Which OS are you using and which version of Java, for example? That would help me understand a bit more about how/when these failures might occur.

On our team, we typically use either Mac or Linux for development work along with Java 8, and we have not seen these "flakey" test results in the past.

HildoYe commented 2 years ago

Hi @HildoYe, could you perhaps provide more info about the environment where you are seeing test failures occur? Which OS are you using and which version of Java, for example? That would help me understand a bit more about how/when these failures might occur.

On our team, we typically use either Mac or Linux for development work along with Java 8, and we have not seen these "flakey" test results in the past.

The tests I've done are in Linux and Java 8. It can be reproduced with mvn -pl . edu.illinois:nondex-maven-plugin:1.1.2:nondex -Dtest=TESTNAME List:

DynamicModelSerializationTest#testAddlPropsNull
DynamicModelSerializationTest#testAlternatePropertyNames
DynamicModelSerializationTest#testDynamicModelMethods
DynamicModelSerializationTest#testModelAPFoo
DynamicModelSerializationTest#testModelAPFooNullTypeToken
DynamicModelSerializationTest#testModelAPInteger
DynamicModelSerializationTest#testModelAPObject
DynamicModelSerializationTest#testModelAPProtectedCtor
DynamicModelSerializationTest#testModelAPString
DynamicModelSerializationTest#testNullValues
RequestBuilderTest#testBodyContent1
RequestBuilderTest#testBodyContent3
RequestBuilderTest#testBodyContentList
RequestUtilsTest#testOmitWithNulls
RequestUtilsTest#testPickWithNulls
padamstx commented 2 years ago

@HildoYe did you see the unit test failures only when using your edu.illinois:nondex-maven-plugin:1.1.2:nondex plugin? The reason I ask is that we haven't seen any actual unit test failures on Mac or Linux. Also a bit curious about the reason for your involvement here. Are you using an IBM Cloud SDK that depends on the java core library or are you simply experimenting with the java core to exercise your maven plugin?

Edit: I was able to run a couple of the tests with the "nondex" plugin and see the failures. Since these failures seem to occur only when the plugin is used, I think we'll hold off on merging in this PR for now. Thank you for bringing this to our attention though.

padamstx commented 2 years ago

closing without merging as we don't plan to merge this one in...