Skip to content

fix flaky tests in RelationalExampleMapperTests #1097

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

Conversation

CharlesZKQ
Copy link
Contributor

In org.springframework.data.relational.repository.query.RelationalExampleMapperTests, the tests queryByExampleEvenHandlesInvisibleFields(), queryByExampleSupportsPropertyTransforms(), queryByExampleWithFirstnameAndLastname(), queryByExampleWithFirstnameOrLastname(), queryByExampleWithNullMatchingFirstnameAndLastname() are all flaky due to the non-deterministic property of getDeclatedFields() (please refer document). it is internally invoked by getMappedExample(). I fixed them by generating all possibilities of actual query strings and store them in a Set, then we only need to check if the actual query string is in the Set.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 22, 2021
Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any flakyness, although I tend to agree, that we shouldn't depend on the order of getDeclaredFields.

I don't like the duplication inherent in listing all the possible variants. We should instead split the string and ensure that the resulting set of conditions is the one we expect.

@CharlesZKQ
Copy link
Contributor Author

split the string and ensure that the resulting set of conditions is the one we expect

Hi, I've updated all PRs regarding flaky tests in the way that compare each part in the string instead of split the string and ensure that the resulting set of conditions is the one we expect. And all changes in #1096 and #1095 have been merged into this PR. Thanks for the advice!

schauder pushed a commit that referenced this pull request Dec 7, 2021
Original pull request #1097
schauder added a commit that referenced this pull request Dec 7, 2021
Original pull request #1097
@schauder
Copy link
Contributor

schauder commented Dec 7, 2021

Thanks, that is polished and merged.

@schauder schauder closed this Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants