Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Improve EventTest#serialization #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pengyuejiang
Copy link

The test method serialization() tests the serialization of Json objects by comparing their string representations. Under most circumstances it won't be an issue, because it is usually expected that the fields have a fixed order.

However, in terms of specification, Gson implicitly relies on calling the Java method getDeclaredFields(), which its documentation said:

The elements in the returned array are not sorted and are not in any particular order.

A plugin that can be useful to detect such potential problems NonDex can be used like this:

git clone https://github.com/square/pagerduty-incidents && cd pagerduty-incidents
mvn clean install -DskipTests
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -Dtest=com.squareup.pagerduty.incidents.EventTest#serialization

Thus, it would be safer to compare objects directly, so that this non-determinism will never generate a false alarm. By serialize and then deserialize, if the test case passes, then one can still say that the serialization is valid, because they are deserialized to the same object.

Please do reply if there are anything that can be made better for this pull request :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant