Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

njain2208
Copy link

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

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.

None yet

1 participant