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

Fix flaky test with transactions #7187

Merged
merged 6 commits into from
Feb 18, 2021

Conversation

davimacedo
Copy link
Member

New Pull Request Checklist

  • [ X ] I am not disclosing a vulnerability.
  • [ X ] I am creating this PR in reference to an issue.

Issue Description

Flaky test with mongodb transactions due to transient error.

Related issue: #7180

Approach

It is a weird brute force solution but it is actually the MongoDB recommended way:
https://docs.mongodb.com/manual/core/transactions-in-applications/#core-api

Basically it recommends to retry in an infinite loop every time you get a TransientError. I limited it to 5 times. Before the fix, I testes and it failed about 1 every 100 times the test runs. Now I can run the test 10,000 with no errors.

TODOs before merging

  • Add test cases
  • [ X ] Add entry to changelog
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK
  • ...

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

Nice!

@codecov
Copy link

codecov bot commented Feb 13, 2021

Codecov Report

Merging #7187 (3a2fd82) into master (738ba9f) will decrease coverage by 9.80%.
The diff coverage is 49.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7187      +/-   ##
==========================================
- Coverage   94.04%   84.23%   -9.81%     
==========================================
  Files         172      172              
  Lines       12850    12867      +17     
==========================================
- Hits        12085    10839    -1246     
- Misses        765     2028    +1263     
Impacted Files Coverage Δ
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 90.26% <0.00%> (-3.25%) ⬇️
src/ParseServerRESTController.js 82.08% <56.00%> (-16.28%) ⬇️
src/batch.js 75.86% <56.52%> (-16.45%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 2.42% <0.00%> (-93.44%) ⬇️
src/Adapters/Storage/Postgres/PostgresClient.js 6.66% <0.00%> (-80.00%) ⬇️
src/Controllers/UserController.js 95.27% <0.00%> (-2.37%) ⬇️
src/Controllers/FilesController.js 92.00% <0.00%> (-2.00%) ⬇️
src/Controllers/DatabaseController.js 94.29% <0.00%> (-1.17%) ⬇️
src/Routers/UsersRouter.js 93.75% <0.00%> (-0.63%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 738ba9f...3a2fd82. Read the comment docs.

@davimacedo
Copy link
Member Author

It looks we now have another flaky test because of this. I will check it out before merging.

@davimacedo davimacedo marked this pull request as draft February 13, 2021 00:17
@davimacedo davimacedo marked this pull request as ready for review February 16, 2021 05:52
@davimacedo davimacedo requested a review from mtrezza February 16, 2021 05:52
@davimacedo davimacedo merged commit a430d6f into parse-community:master Feb 18, 2021
@davimacedo davimacedo mentioned this pull request Feb 18, 2021
4 tasks
dplewis pushed a commit that referenced this pull request Feb 21, 2021
* Fix flaky test with transactions

* Add CHANGELOG entry

* Fix the other transactions related tests that became flaky because now Parse Server tries to submit the transaction multilpe times in the case of TransientError

* Remove fit from tests
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@mtrezza mtrezza mentioned this pull request Mar 12, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants