katharsis-project / katharsis-framework

Katharsis adds powerful layer for RESTful endpoints providing implementenation of JSON:API standard
http://katharsis.io
Apache License 2.0
135 stars 65 forks source link

Fixed Flaky test. Changes in JsonApiUrlBuilder.java. #461

Open njain2208 opened 1 year ago

njain2208 commented 1 year ago

PR Overview:


This PR fixes the flaky/non-deterministic behavior of the following test because it assumes the ordering.

io.katharsis.resource.paging.PagedLinksInformationQuerySpecTest#testPagingLast

io.katharsis.resource.paging.PagedLinksInformationQuerySpecTest#testPagingFirst

io.katharsis.resource.paging.PagedLinksInformationQuerySpecTest#testPaging

io.katharsis.queryspec.DefaultQuerySpecSerializerTest#testPagingOnRelation

io.katharsis.queryspec.DefaultQuerySpecSerializerTest#testPaging

Test Overview:


In all of the above tests, a urlBuilder is employed to construct URLs by incorporating various query parameters. However, due to the storage of parameters in a non-deterministic map, the order in which keys and values are stored becomes unpredictable. This lack of predictability is evident in the test failure below, where the urlBuilder exhibits a different order for the 'offset' and 'limit' parameters in the URL compared to the expected order.

This flakiness was identified by the nondex tool created by the researchers of UIUC.

[ERROR]   DefaultQuerySpecSerializerTest.testPaging:136->check:167 expected:<...7.0.0.1/tasks/?page[[limit]=2&page[offset]=1]> but was:<...7.0.0.1/tasks/?page[[offset]=1&page[limit]=2]>
[ERROR]   DefaultQuerySpecSerializerTest.testPagingOnRelation:147 expected:<...hips/projects/?page[[limit]=2&page[offset]=1]> but was:<...hips/projects/?page[[offset]=1&page[limit]=2]>
[ERROR]   PagedLinksInformationQuerySpecTest.testPaging:52 expected:<...7.0.0.1/tasks/?page[[limit]=2&page[offset]=4]> but was:<...7.0.0.1/tasks/?page[[offset]=4&page[limit]=2]>
[ERROR]   PagedLinksInformationQuerySpecTest.testPagingFirst:80 expected:<...7.0.0.1/tasks/?page[[limit]=3&page[offse]t]=3> but was:<...7.0.0.1/tasks/?page[[offset]=3&page[limi]t]=3>
[ERROR]   PagedLinksInformationQuerySpecTest.testPagingLast:93 expected:<...7.0.0.1/tasks/?page[[limit]=4&page[offse]t]=4> but was:<...7.0.0.1/tasks/?page[[offset]=4&page[limi]t]=4>

You can reproduce the issue by running the following commands (the below command is for PagedLinksInformationQuerySpecTest, please change the tests to target other tests) -

mvn install -pl katharsis-core -am -DskipTests
mvn test -pl katharsis-core  -Dtest=-Dtest=io.katharsis.resource.paging.PagedLinksInformationQuerySpecTest
mvn -pl katharsis-core edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=io.katharsis.resource.paging.PagedLinksInformationQuerySpecTest

Fix:


To fix the issue I decided to store all of the query parameters in the urlBuilder in a TreeMap which is deterministic in nature.

https://github.com/njain2208/katharsis-framework/blob/795f84873a75dd7de0781826bdff9cad08a1b2eb/katharsis-core/src/main/java/io/katharsis/core/internal/utils/JsonApiUrlBuilder.java#L86