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 in testIncludeRelationsMultipleSame Test #462

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

Conversation

njain2208
Copy link

@njain2208 njain2208 commented Nov 29, 2023

PR Overview:


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

io.katharsis.legacy.queryParams.DefaultQueryParamsConverterTest#testIncludeRelationsMultipleSame

Test Overview:


In the above test, the addParams method utilizes a non-deterministic HashMap to store parameters. This lack of determinism results in a change in the order of 'project' and 'projects' within the includedRelations array, consequently causing the test to fail.

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

[ERROR]   DefaultQueryParamsConverterTest.testIncludeRelationsMultipleSame:367->transitivityCheckTask:442 transitivity check expected:<QuerySpec{resourceClass=class io.katharsis.resource.mock.models.Task, limit=null, offset=0, filters=[], sort=[], includedFields=[], includedRelations=[project, projects], relatedSpecs={}}> but was:<QuerySpec{resourceClass=class io.katharsis.resource.mock.models.Task, limit=null, offset=0, filters=[], sort=[], includedFields=[], includedRelations=[projects, project], relatedSpecs={}}>

You can reproduce the issue by running the following commands:

mvn install -pl katharsis-core -am -DskipTests
mvn test -pl katharsis-core  -Dtest=io.katharsis.legacy.queryParams.DefaultQueryParamsConverterTest#testIncludeRelationsMultipleSame
mvn -pl katharsis-core edu.illinois:index-maven-plugin:2.1.1:nondex -Dtest=io.katharsis.legacy.queryParams.DefaultQueryParamsConverterTest#testIncludeRelationsMultipleSame

Fix:


To fix the issue I decided to store all of the parameters in a LinkedHashSet so it returns the element in which it is stored.

https://github.com/njain2208/katharsis-framework/blob/36c7411dcf9ae6515c539ec7f38976749939614b/katharsis-core/src/test/java/io/katharsis/legacy/queryParams/AbstractQueryParamsTest.java#L49-L60

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.

1 participant