approvals / approvals/ApprovalTests.java.StarterProject.gradle

Potential issue with usage of verifyAsJson

Open
#102 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
2
Forks
1
Avg merge
1m
Merged PRs (30d)
2

Description

The Sample test uses verifyAsJson method of Approvals which in turn calls JsonApprovals. This causes the following two strings to be compared:
a) JsonUtils.asJson(object)
b) The data in the .approved file

a) uses Gson.toJson() here. JSON strings are unordered by default. Hence it is possible that the behavior of the test can be flaky depending on order of keys

There were three potential options I see:
a)
Have a comparator passed in the test for ordering and comparing both JSON strings: This would involve having to deal with files and needing to add a test dependency to file utils. It would look something like:

    Person hero = new Person("jayne", "cobb", true, 38);
            Approvals.verifyAsJson(hero, new Options().withComparator((received, approved) -> {
                        // Custom JSON comparison logic that ignores order
                        String receivedContent = FileUtils.readFileToString(received, "UTF-8");
                        String approvedContent = FileUtils.readFileToString(approved, "UTF-8");

                        Map<String, Object> receivedMap = new TreeMap<>(new Gson().fromJson(receivedContent, Map.class));
                        Map<String, Object> approvedMap = new TreeMap<>(new Gson().fromJson(approvedContent, Map.class));
                        return Objects.equals(receivedMap, approvedMap) ? VerifyResult.SUCCESS : VerifyResult.FAILURE;
                    }));

This fix changes only the test however it is a short term one as it does not handle the root cause(unordered JSON string direct comparision) which can arise for other projects which use verifyAsJson without a custom comparator. It is a fast fix only for the client and would defeat the purpose of using Approvals as we are doing heavy lifting in the test

b) Convert the object to a json string using Gson locally in the test and call verifyJson, with reorderJson=true to automatically sort the fields of this string. Also change approved.json, to have fields sorted alphabetically. This will ensure test stability with no source code change.

c) Expose a verifyAsJson() method override with reorder flag which can internally sort the fields using the same utilities that verifyJson uses. This involves changes in the Approval library but can be a good extended functionality.

Please let me know your thoughts and I can help contribute

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with SampleTests.java and compare the verifyAsJson path in JsonApprovals.java with verifyJson's reorderJson behavior, then inspect JsonUtils.java and the approved.json fixture. Reproduce the sample test and determine which proposed approach makes verification independent of JSON key order. Done means the sample test is stable without requiring each client to provide custom comparison logic.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.