test: Tests fail when third-party servers don't respond in time - #10723
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe authentication adapter specs now use a specific missing-signing-key error. The App Store validation spec mocks the response and checks the outgoing request method, destination, and receipt data. ChangesExternal-service specs
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~8 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The three affected specs no longer depend on responses from the named third-party hosts and are ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 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 #10723 +/- ##
=======================================
Coverage 93.94% 93.94%
=======================================
Files 193 193
Lines 17071 17071
Branches 257 257
=======================================
+ Hits 16037 16038 +1
+ Misses 1012 1011 -1
Partials 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Pull Request
Issue
Three specs send requests to servers of Apple and Facebook, and fail if a server doesn't respond within the spec timeout of 20 s. The App Store spec failed 5 test jobs between September 22 and 24 this way, and in each of them the next spec that restarted Parse Server failed as well, with
socket hang up.Closes #10722, part of #10681.
Approach
should fail at appstore validationresponds to the request to the App Store with the status21002itself, by spying onrequestof thehttpsclient offollow-redirects, whichsrc/request.jsuses. The spec also checks the request that Parse Server sends to the App Store.should throw error if public key used to encode token is not availableof the Apple and Facebook adapters stubsgetSigningKeyto reject with theSigningKeyNotFoundErrorthatjwks-rsathrows for an unknown key ID, instead of downloading the public keys. The other specs of these adapters that reach the key lookup already stubgetSigningKey.A check in
spec/helper.jsthat rejects connections to hosts other thanlocalhostwas considered, so that new specs can't depend on third-party servers. It was not added, because it would have to replacenet.Socket.prototype.connectfor all specs, and full test runs with MongoDB and PostgreSQL found only these 3 specs sending requests to third-party servers.Verified locally with MongoDB and PostgreSQL:
socket hang up. With this change, the specs passed without connecting to a host other thanlocalhost.getSigningKey.localhost, except for the lookups of the host namesnotanddoesnotexistthat don't exist, as described in Remove third-party network calls from required test jobs #10722. The MongoDB run had 1 failure indoes not allow signup with preventSignupWithUnverified, which doesn't wait for the verification email to be sent; it passed when the run was repeated with the same seed, and in 8 of 8 runs of the spec alone. The PostgreSQL run had 1 failure withdeadlock detectedindenies __proto__ after a sibling nested object, which also occurs without this change with the same seed.Tasks
Summary by CodeRabbit