Skip to content

Ordering fix comparejson#23

Merged
slayerjain merged 4 commits intokeploy:mainfrom
harshitashankar:ordering-fix-comparejson
Jun 9, 2025
Merged

Ordering fix comparejson#23
slayerjain merged 4 commits intokeploy:mainfrom
harshitashankar:ordering-fix-comparejson

Conversation

@harshitashankar
Copy link
Contributor

@harshitashankar harshitashankar commented May 9, 2025

Tests were failing intermittently because Go's map iteration order is random by design, which meant that the order of nested objects could vary between runs. By sorting the keys before iteration, we can ensure consistent order that matches the test's expectations.

Fixes #17, #16

Signed-off-by: Harshita Shankar <harshitashankar@yahoo.com>
Signed-off-by: Harshita Shankar <harshitashankar@yahoo.com>
Signed-off-by: Harshita Shankar <harshitashankar@yahoo.com>
@keploy
Copy link

keploy bot commented May 9, 2025

Keploy failed to create test cases for this PR. For retrying, please click here

Signed-off-by: Harshita Shankar <harshitashankar@yahoo.com>
@harshitashankar harshitashankar force-pushed the ordering-fix-comparejson branch from 49bbf32 to e5ff27c Compare June 5, 2025 09:32
@Hermione2408
Copy link
Member

@harshitashankar, can you please send me the sample app with which you tested this change.
Also, we do have IgnoreOrdering flag in keploy open source which would ignore the ordering of the json map

@harshitashankar
Copy link
Contributor Author

@harshitashankar, can you please send me the sample app with which you tested this change. Also, we do have IgnoreOrdering flag in keploy open source which would ignore the ordering of the json map

Verified using this UT- https://github.com/keploy/jsondiff/blob/main/jsonDiff_test.go#L677-L691
Reg ignoreOrdering flag - I checked this. Looks like ignore-ordering flag is not used in the jsondiff comparision.
I could find only one usage of this flag which was here
https://github.com/keploy/keploy/blob/main/pkg/matcher/utils.go#L943
its not being passed to interactions with jsondiff

Let me know if you think something else also needs to be tested.

@Hermione2408
Copy link
Member

It looks good to me!

@slayerjain slayerjain merged commit 7b7a28f into keploy:main Jun 9, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The tests for this package are flaky

3 participants