play-json icon indicating copy to clipboard operation
play-json copied to clipboard

Add test for field ordering

Open levinson opened this issue 9 months ago • 1 comments

Pull Request Checklist

  • [X] Have you read through the contributor guidelines?
  • [X] Have you squashed your commits?
  • [X] Have you added copyright headers to new files? N/A
  • [X] Have you updated the documentation? N/A
  • [X] Have you added tests for any changed functionality? N/A

Purpose

This PR demonstrates #1038. The new test fails due to serialized field order when compiled with Scala 3:

[info] - should preserve field ordering *** FAILED *** (33 milliseconds)
[info]   "{"[a":42,"m1":"foo","b":"baz","c":"bar","z":1.2]}" did not equal "{"[m1":"foo","c":"bar","z":1.2,"a":42,"b":"baz"]}" (JsonSharedSpec.scala:454)

levinson avatar May 11 '24 20:05 levinson

Would it make sense to test the field ordering at the JsObject level in addition to the String level?

I note the comment on the fields method on JsObject: it gives the fields in order. Based on this, I would—naively?—expect the Seq returned by fields to be in declaration order when using the Json.writes macro and friends. It would be a real nicety to have a test for that as well.

https://github.com/playframework/play-json/blob/85a8aa03a506e8dbf57cb8f6afcf580a89e6fdfd/play-json/shared/src/main/scala/play/api/libs/json/JsValue.scala#L116-L123

[On the other hand: I can argue that a JsObject doesn't contain any information about the field declaration order except the ordering of the ImmutableLinkedHashMap in underlying. Ergo: if it's wrong at the JsObject level there's no way Json.stringify can correct it, and thus this test case implicitly also verifies that underlying is ordered correctly. That feels a bit indirect for my tastes—I lean towards an independent test of that—but I figured it deserved to be mentioned.]

jonaskoelker-jypo avatar Aug 01 '24 08:08 jonaskoelker-jypo