Skip to content

test: Tests fail when third-party servers don't respond in time - #10723

Merged
mtrezza merged 2 commits into
parse-community:alphafrom
mtrezza:test/third-party-network-calls
Sep 29, 2026
Merged

mtrezza merged 2 commits into
parse-community:alphafrom
mtrezza:test/third-party-network-calls

Conversation

@mtrezza

@mtrezza mtrezza commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

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 validation responds to the request to the App Store with the status 21002 itself, by spying on request of the https client of follow-redirects, which src/request.js uses. 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 available of the Apple and Facebook adapters stubs getSigningKey to reject with the SigningKeyNotFoundError that jwks-rsa throws for an unknown key ID, instead of downloading the public keys. The other specs of these adapters that reach the key lookup already stub getSigningKey.

A check in spec/helper.js that rejects connections to hosts other than localhost was considered, so that new specs can't depend on third-party servers. It was not added, because it would have to replace net.Socket.prototype.connect for 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:

  • With connections to Apple and Facebook held open without a response, the 3 specs timed out and, as on CI, the next spec that restarts Parse Server failed with socket hang up. With this change, the specs passed without connecting to a host other than localhost.
  • The App Store spec fails if Parse Server sends the receipt to the sandbox URL, sends no receipt, or doesn't return the status of the App Store. The Apple and Facebook specs fail if the adapters don't convert the error of getSigningKey.
  • Full test runs with a trace of all connections and DNS lookups connected to no host other than localhost, except for the lookups of the host names not and doesnotexist that don't exist, as described in Remove third-party network calls from required test jobs #10722. The MongoDB run had 1 failure in does 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 with deadlock detected in denies __proto__ after a sibling nested object, which also occurs without this change with the same seed.

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • Tests
    • Added coverage for Apple and Facebook sign-in when signing-key lookup fails, confirming the matching-key error is reported.
    • Expanded App Store receipt-validation checks to verify that rejected receipts are submitted using the expected request method and destination, with the receipt data included in the request. This helps ensure the validation failure scenario is accurately covered.

@parse-github-assistant

Copy link
Copy Markdown

🚀 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

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

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.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: e35778a4-9335-4404-a451-89e7fb383b74

📥 Commits

Reviewing files that changed from the base of the PR and between 1ca4473 and 01e5468.

📒 Files selected for processing (1)
  • spec/PurchaseValidation.spec.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • spec/PurchaseValidation.spec.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

External-service specs

Layer / File(s) Summary
Authentication signing-key failures
spec/AuthenticationAdapters.spec.js
The Apple and Facebook Limited Login specs reject signing-key lookups with SigningKeyNotFoundError naming key 789.
App Store validation request mock
spec/PurchaseValidation.spec.js
The spec intercepts HTTPS requests, supplies a status 21002 response, and checks that validation sends a POST to buy.itunes.apple.com/verifyReceipt with the encoded receipt in receipt-data.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 01e54

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title starts with the allowed prefix test: and uses a capitalized first word. It accurately describes the test changes that remove failures caused by unresponsive third-party servers.
Description check ✅ Passed The description includes the required Pull Request, Issue, Approach, and Tasks sections. It explains the problem, implementation, validation, linked issues, and completed testing work.
Linked Issues check ✅ Passed Issue #10722 identifies three provider-dependent specs. spec/PurchaseValidation.spec.js replaces the App Store request with a local https.request mock and verifies the POST host, path, and encoded…
Out of Scope Changes check ✅ Passed The whole-PR changes only modify the three test cases named in issue #10722. The request assertions and explicit signing-key errors support the required isolation. No unrelated production behavior or …
Security Check ✅ Passed PASS. The pull request changes only spec/AuthenticationAdapters.spec.js and spec/PurchaseValidation.spec.js; it changes no production code, dependencies, or authentication controls. The added HTTP…
Engage In Review Feedback ✅ Passed The current review produced zero actionable findings and no review feedback comments. Therefore, the pull request had no feedback that required engagement, implementation, or reviewer agreement.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 29, 2026
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.94%. Comparing base (c3b4694) to head (01e5468).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mtrezza
mtrezza merged commit 6145e7f into parse-community:alpha Sep 29, 2026
25 checks passed
@mtrezza
mtrezza deleted the test/third-party-network-calls branch September 29, 2026 13:56
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.

Remove third-party network calls from required test jobs

1 participant