Skip to content

Conversation

@mtrezza
Copy link
Member

@mtrezza mtrezza commented Nov 18, 2025

Pull Request

Issue

Missing tests for circular and self-referencing pointers when using Parse.Query.includeAll.

Approach

Add tests.

Summary by CodeRabbit

  • Tests
    • Added tests validating includeAll behavior with circular and self-referencing pointers.
    • Tests verify proper handling of nested reference population depth.

@mtrezza mtrezza changed the title test: Add tests for Parse.Query.includeAll test: Add tests for Parse.Query.includeAll for circular and self-referencing pointers Nov 18, 2025
@parse-github-assistant
Copy link

🚀 Thanks for opening this pull request!

1 similar comment
@parse-github-assistant
Copy link

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

📝 Walkthrough

Walkthrough

Two new test cases were added to validate includeAll query behavior with circular object references and self-referencing pointers, ensuring nested objects are populated only one level deep rather than fully traversed.

Changes

Cohort / File(s) Summary
Test coverage for includeAll with circular references
spec/ParseQuery.spec.js
Added two async test cases validating includeAll behavior: one testing circular reference handling (where object A references object B which references object A), and another testing self-referencing pointers (where an object references itself). Both tests verify partial population depth and pointer data structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify test logic: Confirm that the test assertions correctly validate one-level-deep population for both circular and self-referencing scenarios
  • Edge case coverage: Check whether the test cases cover the intended boundary conditions and that the expected population depth (one level) aligns with the intended behavior

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description follows the template structure with Issue and Approach sections, though the Approach section is minimal. The required Issue link and basic structure are present but incomplete. While the template structure is present, the Approach section is too brief ('Add tests') and lacks a 'Closes: ' link to a specific issue as required by the template. Add the issue number in the format 'Closes: #ISSUE_NUMBER' and provide more detail on what tests were added.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding tests for includeAll behavior with circular and self-referencing pointers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50650a3 and b12bb1e.

📒 Files selected for processing (1)
  • spec/ParseQuery.spec.js (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • spec/ParseQuery.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Applied to files:

  • spec/ParseQuery.spec.js
🧬 Code graph analysis (1)
spec/ParseQuery.spec.js (2)
spec/helper.js (2)
  • TestObject (279-281)
  • Parse (4-4)
src/RestQuery.js (4)
  • query (328-337)
  • query (338-338)
  • results (756-756)
  • result (55-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: Node 20
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Redis Cache
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Node 18
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: Node 22
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: Benchmarks
  • GitHub Check: Docker Build
🔇 Additional comments (3)
spec/ParseQuery.spec.js (3)

4673-4723: LGTM! Well-structured test for circular references.

The test correctly validates that includeAll properly handles circular pointer references by including only one level deep. The verification includes both field population checks and JSON format validation, which provides comprehensive coverage.

Based on learnings


4673-4756: Suggested PR title for changelog:

Based on the Angular commit convention, consider updating the PR title to:

test(query): add includeAll tests for circular and self-referencing pointers

This format helps generate meaningful changelog entries by clearly indicating:

  • Type: test (adding test coverage)
  • Scope: query (Parse.Query functionality)
  • Description: what specific behavior is being tested

Based on learnings


4725-4756: Let me check if there are any implementation issues or related failing tests:

Now let me check the git context to see if this is a new test or modified code:

Based on my verification of the codebase and test patterns, here's my analysis:

Test expectations are logically correct. The expected behavior of includeAll is to include only the first layer of nested children objects. Since the self-reference (selfRef.ref) is at the first level, it should be fully populated, making the test expectation at line 4755 (expect(ref.get('name')).toBe('Self-Referencing')) correct.

The test pattern is consistent with the preceding circular reference test, which verifies that level 2 references are NOT fully populated. The self-reference test correctly expects the opposite for level 1 includes.

However, the original review's suggestion to add JSON format verification (like the circular test does on lines 4717-4722) is a minor consistency improvement, not a fix for a critical issue. The current test sufficiently verifies the core behavior through .get() calls alone. Adding JSON verification would follow the same pattern as the circular reference test but is optional.

No issues were found with the test logic or expectations. The behavior being tested aligns with Parse Server's documented includeAll semantics.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.0)
spec/ParseQuery.spec.js

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.08%. Comparing base (c22cb0a) to head (b12bb1e).
⚠️ Report is 1 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9936      +/-   ##
==========================================
+ Coverage   93.06%   93.08%   +0.02%     
==========================================
  Files         187      187              
  Lines       15253    15253              
  Branches      177      177              
==========================================
+ Hits        14195    14199       +4     
+ Misses       1046     1042       -4     
  Partials       12       12              

☔ View full report in Codecov by Sentry.
📢 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.

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.

2 participants