refactor: Add a few lint rules to test files in /spec
#9815
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe changes are limited to formatting, whitespace, and code style improvements across several test files and the ESLint configuration. No logic, control flow, or exported/public entities were altered. The ESLint configuration was expanded to enforce more stylistic and code quality rules. All modifications are non-functional. Changes
Sequence Diagram(s)No sequence diagram is generated as the changes are limited to formatting and configuration updates without any control flow or feature modifications. 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.38.1)spec/ParseGraphQLServer.spec.jsspec/ParseQuery.spec.js🔧 ESLint
npm warn EBADENGINE Unsupported engine { 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/CloudCode.spec.js (1)
3932-4050: File-hook tests leak global triggers across specsEach test registers
beforeFind/afterFindhooks onParse.Filewithout tearing them down or spinning up a fresh server.
Because triggers are global, a hook from one test will still be active in the next, creating hidden ordering dependencies and race conditions (CI flakes).Consider either:
for (const hook in hooks) { spyOn(hooks, hook).and.callThrough(); - Parse.Cloud[hook](Parse.File, hooks[hook]); + const handle = Parse.Cloud[hook](Parse.File, hooks[hook]); // assume Cloud.<hook> returns an id } ... // inside afterEach Parse.Cloud._removeHook(handle); // or reconfigureServer()or the simpler/idiomatic pattern already used elsewhere in the suite:
await reconfigureServer();inside an
afterEachto reset the trigger registry.This keeps the tests hermetic and prevents sporadic failures when more hooks are added later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
spec/Adapters/Auth/gcenter.spec.js(1 hunks)spec/CloudCode.spec.js(3 hunks)spec/EmailVerificationToken.spec.js(3 hunks)spec/ParseGraphQLServer.spec.js(1 hunks)spec/ParseLiveQuery.spec.js(1 hunks)spec/ParseQuery.spec.js(4 hunks)spec/Utils.spec.js(1 hunks)spec/eslint.config.js(1 hunks)spec/support/CurrentSpecReporter.js(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: mtrezza
PR: parse-community/parse-server#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: mtrezza
PR: parse-community/parse-server#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: mtrezza
PR: parse-community/parse-server#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.
Learnt from: RahulLanjewar93
PR: parse-community/parse-server#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.
spec/EmailVerificationToken.spec.js (3)
Learnt from: mtrezza
PR: parse-community/parse-server#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: mtrezza
PR: parse-community/parse-server#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.
Learnt from: mtrezza
PR: parse-community/parse-server#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()`.
spec/Adapters/Auth/gcenter.spec.js (3)
Learnt from: mtrezza
PR: parse-community/parse-server#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.
Learnt from: mtrezza
PR: parse-community/parse-server#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: mtrezza
PR: parse-community/parse-server#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.
spec/ParseLiveQuery.spec.js (4)
Learnt from: mtrezza
PR: parse-community/parse-server#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.
Learnt from: RahulLanjewar93
PR: parse-community/parse-server#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
PR: parse-community/parse-server#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: mtrezza
PR: parse-community/parse-server#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()`.
spec/CloudCode.spec.js (4)
undefined
<retrieved_learning>
Learnt from: mtrezza
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().
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: RahulLanjewar93
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.
</retrieved_learning>
spec/Utils.spec.js (1)
Learnt from: mtrezza
PR: parse-community/parse-server#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.
spec/ParseGraphQLServer.spec.js (4)
undefined
<retrieved_learning>
Learnt from: mtrezza
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().
</retrieved_learning>
<retrieved_learning>
Learnt from: RahulLanjewar93
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
spec/ParseQuery.spec.js (4)
undefined
<retrieved_learning>
Learnt from: RahulLanjewar93
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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().
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: mtrezza
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.
</retrieved_learning>
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Node 20
- GitHub Check: Node 18
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
🔇 Additional comments (9)
spec/eslint.config.js (1)
51-65: Excellent ESLint rule additions for code consistency.The comprehensive set of formatting and style rules will significantly improve code consistency across the test files. The rules cover all the essential aspects:
- Consistent indentation and spacing
- Unix line endings and end-of-file newlines
- Proper use of
constoverlet/var- Consistent object and block spacing
The disabled rules (
no-unused-vars,no-useless-escape,require-atomic-updates) are appropriate choices for test files where these rules can be overly restrictive.spec/support/CurrentSpecReporter.js (1)
117-118: LGTM - Formatting compliance with ESLint rules.The newline addition at the end of the file aligns with the
eol-lastESLint rule introduced in the configuration. This is a standard formatting practice.spec/Utils.spec.js (1)
11-11: LGTM - Trailing whitespace cleanup.The trailing whitespace removal aligns with the
no-trailing-spacesESLint rule introduced in the configuration. This formatting cleanup improves code consistency.spec/ParseGraphQLServer.spec.js (1)
459-459: Whitespace cleanup conforms to new lint rules — looks good
This change is purely stylistic and keeps the test file compliant with the stricter ESLint configuration introduced by the PR. No functional impact.spec/ParseLiveQuery.spec.js (1)
1185-1186: Nice catch – stray whitespace removed.No functional impact, but eliminates unnecessary diff noise going forward.
spec/ParseQuery.spec.js (1)
5310-5376: LGTM – only whitespace / stylistic nits, no functional impactThe touched lines are confined to the three new-line blocks inside the
describe_only_db('mongo')('query nested keys', …)section.
No logic was modified, tests still follow the agreed-upon async/await pattern (see learnings), and assertions remain intact.
Happy to approve.spec/Adapters/Auth/gcenter.spec.js (1)
15-36: Formatting-only change looks goodIndentation is now consistent with the surrounding code; no functional impact detected.
spec/EmailVerificationToken.spec.js (2)
682-683: �� Switched toconstfor immutable referencesPromoting
userBeforeEmailResetanduserBeforeRequesttoconstaccurately reflects immutability and prevents accidental reassignment. No other adjustments needed.Also applies to: 749-750
1010-1013: Whitespace cleanup acknowledgedTrailing-space removal has no behavioural effect and improves lint compliance.
| 'X-Parse-Cloud-Context': '{"a":"a"}', | ||
| }, | ||
| body: JSON.stringify({_context: { hello: 'world' }}), | ||
| body: JSON.stringify({ _context: { hello: 'world' } }), | ||
| }); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Avoid double-encoding the request body
lib/request already JSON-serialises plain objects that are passed via the body field (see other tests in this file).
Passing a pre-stringified payload here will therefore lead to a double JSON encoding on the wire:
{"body":"{\"_context\":{\"hello\":\"world\"}}"}which breaks the server-side deserialisation and defeats the purpose of the test.
- body: JSON.stringify({ _context: { hello: 'world' } }),
+ body: { _context: { hello: 'world' } },Run the test suite after applying the patch – this part currently fails if the server expects a structured JSON object.
I’ll verify how lib/request handles the body field and check other tests in spec/CloudCode.spec.js for examples of plain-object bodies.
#!/bin/bash
set -e
echo "1) Sample 'body:' usages in spec/CloudCode.spec.js:"
rg -n "body:" spec/CloudCode.spec.js | head -n 20 || true
echo
echo "2) Locating lib/request implementation:"
REQUEST_FILE=$(fd -t f --hidden --glob "request.js" lib || echo "")
if [[ -z "$REQUEST_FILE" ]]; then
echo "lib/request.js not found under lib/, searching entire repo..."
REQUEST_FILE=$(fd -t f --hidden --glob "request.js" .)
fi
echo "Found: $REQUEST_FILE"
echo
echo "3) Inspecting lib/request.js for JSON serialization logic:"
sed -n '1,200p' "$REQUEST_FILE"I’ll verify the body handling in lib/request and how other tests use plain-object bodies:
#!/usr/bin/env bash
set -e
# 1) Locate the request implementation
REQUEST_FILE=$(fd -t f --glob "request.js" lib | head -n1)
if [[ -z "$REQUEST_FILE" ]]; then
REQUEST_FILE=$(fd -t f --glob "request.js" . | head -n1)
fi
echo "Using: $REQUEST_FILE"
# 2) Show the top of lib/request.js to find body serialization logic
echo "----- lib/request.js (first 200 lines) -----"
sed -n '1,200p' "$REQUEST_FILE"
# 3) Look for JSON.stringify usage in lib/request.js
echo
echo "----- JSON.stringify calls in lib/request.js -----"
rg -n "JSON.stringify" "$REQUEST_FILE" || echo "No JSON.stringify found"
# 4) See how other tests pass `body` in CloudCode.spec.js
echo
echo "----- Examples of body: in spec/CloudCode.spec.js -----"
rg -n "body:" spec/CloudCode.spec.js | head -n20 || trueAvoid double-encoding the request body
lib/request already JSON-serializes plain objects passed via the body field. Wrapping the payload in JSON.stringify here causes a double-encoding on the wire and breaks server-side deserialization.
Locations to update:
- spec/CloudCode.spec.js around line 3503
Required change:
- body: JSON.stringify({ _context: { hello: 'world' } }),
+ body: { _context: { hello: 'world' } },Run the test suite after applying this patch to confirm the fix.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'X-Parse-Cloud-Context': '{"a":"a"}', | |
| }, | |
| body: JSON.stringify({_context: { hello: 'world' }}), | |
| body: JSON.stringify({ _context: { hello: 'world' } }), | |
| }); | |
| 'X-Parse-Cloud-Context': '{"a":"a"}', | |
| }, | |
| body: { _context: { hello: 'world' } }, | |
| }); |
🤖 Prompt for AI Agents
In spec/CloudCode.spec.js around lines 3503 to 3507, the request body is being
double-encoded by calling JSON.stringify on an object that lib/request already
serializes. Remove the JSON.stringify call and pass the plain object directly to
the body field to avoid double-encoding and ensure proper server-side
deserialization.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #9815 +/- ##
=======================================
Coverage 93.00% 93.00%
=======================================
Files 187 187
Lines 15082 15082
Branches 174 174
=======================================
Hits 14027 14027
Misses 1043 1043
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request
Issue
Very few lint rules for spec files.
Approach
Add most lint rules that also apply to /src files.
Summary by CodeRabbit
Style
Chores