Repository navigation
test: Coverage of database update error handling varies between CI runs - #10727
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdded MongoDB and PostgreSQL regression tests that check ChangesAdapter Error Propagation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Merge Risk: ⚪ Minimal · up to This change adds regression coverage for database error propagation without changing adapter behavior. No actionable merge-blocking risk was identified. 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10727 +/- ##
==========================================
+ Coverage 93.93% 93.95% +0.01%
==========================================
Files 193 193
Lines 17071 17071
Branches 257 257
==========================================
+ Hits 16036 16039 +3
+ Misses 1013 1010 -3
Partials 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🎉 This change has been released in version 9.10.2-alpha.6 |
|
🎉 This change has been released in version 9.10.2 |
Pull Request
Issue
codecov/projectfails on many pull requests because 3 lines in the error handling of the database adapters are covered in some CI runs but not in others: lines 1868 and 1887 inupdateObjectsByQueryofsrc/Adapters/Storage/Postgres/PostgresStorageAdapter.js, and line 690 infindOneAndUpdateofsrc/Adapters/Storage/Mongo/MongoStorageAdapter.js. No spec causes an update error other than a duplicate value error, so these lines are only reached by chance. On Postgres, this happens when a background_PushStatusupdate of a push spec runs afterafterEachhas dropped the tables; the cause for the MongoDB line wasn't reproduced. Since April, these were the only lines that varied between CI runs with all coverage uploads, and they caused 19 of the 23codecov/projectfailures on pull requests created from July 1.Closes #10726, part of #10681.
Approach
findOneAndUpdate rethrows errors other than unique violation errorsinspec/PostgresStorageAdapter.spec.jsupdates a class that has no table, and expects the Postgres error42P01(relation does not exist) instead of aParse.Error.findOneAndUpdate rethrows errors other than duplicate key errorsinspec/MongoStorageAdapter.spec.jslets thefindOneAndUpdateof the MongoDB driver reject with an error other than a duplicate key error, and expects the adapter to reject with the same error.PostgreSQLandMongoDBtest jobs. The push specs are not changed; their late_PushStatusupdates can still fail in some runs, but no longer change the coverage.As decided in #10726,
codecov/projectkeeps reporting on pull requests and failing if the coverage decreases, andcodecov/patchstays as it is. Makingcodecov/projectinformational, turning it off, or adding a threshold was considered, but any threshold that tolerates the variation of 1 to 3 lines would also have tolerated the coverage losses that onlycodecov/projectreported, such as in #10498 and #10543. If another line starts varying between CI runs, it can be covered the same way. Oncealphahas shown no varying lines for a while,codecov/projectcan become a required check onalpha, as it already is onreleaseandrelease-8.x.x. A decrease caused by a failed or cancelled test job, which doesn't upload its coverage, remains until the job is re-run.Verified locally with MongoDB 8.0 and PostgreSQL 16:
Parse.Error, also if it keeps the error message, or if it handles the error as a duplicate value error.spec/PostgresStorageAdapter.spec.jsandspec/MongoStorageAdapter.spec.jspass.Tasks
Summary by CodeRabbit
findOneAndUpdatefails.