Fix testDedicatedHostPrototypeDedicatedHostByZone and 9 other tests

[pengyuejiang]

The Problem

The tests

com.ibm.cloud.is.vpc.v1.model.DedicatedHostPrototypeDedicatedHostByZoneTest.testDedicatedHostPrototypeDedicatedHostByZone
com.ibm.cloud.is.vpc.v1.model.InstanceGroupManagerActionPrototypeScheduledActionPrototypeByCronSpecByManagerTest.testInstanceGroupManagerActionPrototypeScheduledActionPrototypeByCronSpecByManager
com.ibm.cloud.is.vpc.v1.model.InstanceGroupManagerActionPrototypeScheduledActionPrototypeByRunAtByManagerTest.testInstanceGroupManagerActionPrototypeScheduledActionPrototypeByRunAtByManager
com.ibm.cloud.is.vpc.v1.model.InstancePrototypeInstanceByImageTest.testInstancePrototypeInstanceByImage
com.ibm.cloud.is.vpc.v1.model.InstancePrototypeInstanceBySourceTemplateTest.testInstancePrototypeInstanceBySourceTemplate
com.ibm.cloud.is.vpc.v1.model.InstancePrototypeInstanceByVolumeTest.testInstancePrototypeInstanceByVolume
com.ibm.cloud.is.vpc.v1.model.InstanceTemplatePrototypeInstanceByImageTest.testInstanceTemplatePrototypeInstanceByImage
com.ibm.cloud.is.vpc.v1.model.InstanceTemplatePrototypeInstanceBySourceTemplateTest.testInstanceTemplatePrototypeInstanceBySourceTemplate
com.ibm.cloud.is.vpc.v1.model.VolumeAttachmentPrototypeInstanceByImageContextTest.testVolumeAttachmentPrototypeInstanceByImageContext
com.ibm.cloud.is.vpc.v1.model.VolumeAttachmentPrototypeInstanceByVolumeContextTest.testVolumeAttachmentPrototypeInstanceByVolumeContext

are flaky as was discovered through the plugin NonDex. Here are the full steps to reproduce this issue:

git clone https://github.com/IBM/vpc-java-sdk && cd vpc-java-sdk
export JAVA_HOME=$(/usr/libexec/java_home -v1.8)
mvn clean install -pl modules/vpc -am -DskipTests
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=DedicatedHostPrototypeDedicatedHostByZoneTest#testDedicatedHostPrototypeDedicatedHostByZone
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstanceGroupManagerActionPrototypeScheduledActionPrototypeByCronSpecByManagerTest#testInstanceGroupManagerActionPrototypeScheduledActionPrototypeByCronSpecByManager
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstanceGroupManagerActionPrototypeScheduledActionPrototypeByRunAtByManagerTest#testInstanceGroupManagerActionPrototypeScheduledActionPrototypeByRunAtByManager
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstancePrototypeInstanceByImageTest#testInstancePrototypeInstanceByImage
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstancePrototypeInstanceBySourceTemplateTest#testInstancePrototypeInstanceBySourceTemplate
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstancePrototypeInstanceByVolumeTest#testInstancePrototypeInstanceByVolume
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstanceTemplatePrototypeInstanceByImageTest#testInstanceTemplatePrototypeInstanceByImage
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=InstanceTemplatePrototypeInstanceBySourceTemplateTest#testInstanceTemplatePrototypeInstanceBySourceTemplate
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=VolumeAttachmentPrototypeInstanceByImageContextTest#testVolumeAttachmentPrototypeInstanceByImageContext
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -pl modules/vpc -Dtest=VolumeAttachmentPrototypeInstanceByVolumeContextTest#testVolumeAttachmentPrototypeInstanceByVolumeContext

The build will fail.

The current implementation compares two serializations with each other in each one of these tests. However, it would be safer to directly compare the objects themselves to avoid any issues where serialization strings are different but representing the same object. NonDex shuffles the ordering of fields so that the serialization strings differ, causing the tests to fail without having any real issues.

The Fix

It's a very simple fix, instead of comparing serializations, JsonParser from GSON is used to compare their deserialized forms to make sure that the test will not fail with the plugin because a field in the serialization string is in a different ordering from another that is constructed from the same object.

After the fix, running the previous series of commands will no longer make the build fail. It will make sure that no false alarm goes off because of such reasons.

Please do give some feedbacks on whether you think this way of fixing it is feasible. Thanks!

[uibm]

We are comparing two strings(.toString()), both extending same GenericModel class.

Is this needed ? When I ran the tests, it ran fine.

[pengyuejiang]

You are right that currently it won't cause an issue. However, the method toString() will implicitly call HashMap's iterator. On Java's documentation, it said

Hash table based implementation of the Map interface. This implementation provides all of the optional map operations, and permits null values and the null key. (The HashMap class is roughly equivalent to Hashtable, except that it is unsynchronized and permits nulls.) This class makes no guarantees as to the order of the map; in particular, it does not guarantee that the order will remain constant over time.

The behavior of that method is thus non-deterministic over time, so if some fields are in a different order in the string, the comparison will fail without having any real problems.

The change is just a preventive one, in case false alarms arise in future Java versions. It's done with relatively small changes to add in an extra bit of robusticity without affecting the original intentions of the test.