test: Migrate Analytics spec to REST; add TS spec runner and REST helpers - #10483
test: Migrate Analytics spec to REST; add TS spec runner and REST helpers#10483dblythy wants to merge 9 commits into
Conversation
Adds a narrow @babel/register hook (spec/support/tsRegister.js) loaded by Jasmine before helper.js so .ts spec/helper files are transpiled at require time. spec_files glob widened to discover .ts specs. Existing .js specs are unaffected — the hook only intercepts .ts under spec/.
|
🚀 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Babel-based TypeScript support to Jasmine, introduces typed REST, client, analytics, configuration, error, and global helpers, and adds TypeScript analytics controller tests. Lockfiles record ChangesTypeScript test infrastructure
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AnalyticsSpec
participant analyticsHelper
participant restRequest
participant ParseServer
participant analyticsAdapter
AnalyticsSpec->>analyticsHelper: Call track or appOpened
analyticsHelper->>restRequest: POST analytics event
restRequest->>ParseServer: Send event request
ParseServer->>analyticsAdapter: Invoke adapter method
analyticsAdapter-->>ParseServer: Return adapter result
ParseServer-->>restRequest: Return response
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Introduces spec/helpers/*.ts — a typed REST client over fetch (request, client, headers, config, errors, globals) so specs can talk to Parse Server through its public HTTP API without going through the Parse JS SDK. Also lands spec/spec_migration.md, the plan that drives the upcoming REST-first migration of the spec suite (towards parse-community#8787).
Replaces spec/Analytics.spec.js (Parse SDK + done() callbacks) with spec/server/analytics.spec.ts (REST client + async/await), plus spec/helpers/analytics.ts. No SDK import, no done(), no setTimeout. First spec to land under the new REST-first layout.
dblythy
changed the title
…sn't break them CI runs Node 24, which natively strips TypeScript and loads .ts as ESM, bypassing the @babel/register CJS hook. ESM needs explicit extensions, so extensionless relative imports failed. Force Jasmine's require loader (engages the hook on Node 20), add explicit .ts extensions, and mark type-only imports with 'import type' so the suite loads identically on Node 20/22/24.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10483 +/- ##
=======================================
Coverage 92.66% 92.66%
=======================================
Files 193 193
Lines 16981 16981
Branches 248 248
=======================================
Hits 15736 15736
Misses 1224 1224
Partials 21 21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
spec/helpers/request.ts (2)
96-97: 💤 Low valueUnsafe type assertion for error body.
Line 97 assigns
e?.data ?? etobody: ParseErrorBodywithout runtime validation. If the caught error is not aParseRequestErroror lacks the expected shape, accessingbody.codeorbody.errormay fail. Since this is a test helper, the test will fail regardless, but a clearer error message would improve debugging.♻️ Add a type guard for clarity
} catch (e: any) { - const body: ParseErrorBody = e && e.data ? e.data : e; + const raw = e?.data ?? e; + const body: ParseErrorBody = (raw && typeof raw === 'object' && 'code' in raw && 'error' in raw) + ? raw + : { code: 0, error: String(raw) }; if (code !== undefined) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/helpers/request.ts` around lines 96 - 97, The catch block in the helper assigns e?.data ?? e to a ParseErrorBody without validation; add a runtime type guard that verifies the caught value is an object and that either e.data exists and has expected keys (e.g., 'code' or 'error') or that e itself has those keys before treating it as ParseErrorBody in the catch of the request helper; if the shape doesn't match, produce a clear error/throw that includes JSON.stringify(e) so tests fail with a helpful message. Locate the catch handling in spec/helpers/request.ts (the catch around the ParseRequestError handling) and replace the unsafe assertion with a guard that narrows to ParseErrorBody or logs/throws a descriptive error when the shape is unexpected.
78-78: ⚡ Quick winType assertion may not match actual error response shape.
Line 78 casts
datatoParseErrorBody | undefinedwithout validating the shape. If the server returns a non-2xx response with plain text, HTML, or a malformed error body,error.datawill not actually conform toParseErrorBody, causing tests that accesserror.data.codeorerror.data.errorto fail withundefinedproperty errors rather than clear assertion failures.🛡️ Add runtime validation or narrow the cast
- error.data = data as ParseErrorBody | undefined; + error.data = (data && typeof data === 'object' && 'code' in data && 'error' in data) + ? data as ParseErrorBody + : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/helpers/request.ts` at line 78, The test helper currently casts response bodies to ParseErrorBody without validation (error.data = data as ParseErrorBody | undefined), which can break when the server returns text/HTML or malformed JSON; add a runtime type guard (e.g., isParseErrorBody) and only assign error.data when the guard passes, otherwise set error.data = undefined or a safe fallback (or attempt safe JSON parsing first), referencing the existing symbol ParseErrorBody and the local error variable so callers accessing error.data.code or error.data.error won’t hit undefined property errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/helpers/client.ts`:
- Line 114: The current return casts and accesses (res.data as { count: number
}).count which will throw if res.data is undefined; update the code in the
function that uses res to first check that res.data exists and that
res.data.count is a number, then return it, otherwise throw a clear error
(include the actual response payload by serializing res.data) so tests fail with
a descriptive message; reference the same res variable and the count property
when making the check and error.
- Line 99: The return expression currently does an unchecked cast: "return
(res.data as { results: T[] }).results;" which will throw if res.data is
undefined; update the handler in spec/helpers/client.ts to guard res.data (e.g.,
check res.data !== undefined) and either throw a clear Error with context
(include res.status or the raw response) or return a safe fallback like an empty
array; locate and modify the function containing that return and the call site
that relies on restRequest (see the current return line and the restRequest
behavior around lines 60-63) so the code uses res.data?.results or an explicit
null check and provides a descriptive error/fallback instead of letting a
TypeError bubble up.
In `@spec/server/analytics.spec.ts`:
- Line 23: Update the test description string in the it(...) call inside
analytics.spec.ts: change "should track a app opened event" to "should track an
app opened event" so the grammar is correct; locate the it(...) invocation in
the spec where the test name is defined and edit the string accordingly.
---
Nitpick comments:
In `@spec/helpers/request.ts`:
- Around line 96-97: The catch block in the helper assigns e?.data ?? e to a
ParseErrorBody without validation; add a runtime type guard that verifies the
caught value is an object and that either e.data exists and has expected keys
(e.g., 'code' or 'error') or that e itself has those keys before treating it as
ParseErrorBody in the catch of the request helper; if the shape doesn't match,
produce a clear error/throw that includes JSON.stringify(e) so tests fail with a
helpful message. Locate the catch handling in spec/helpers/request.ts (the catch
around the ParseRequestError handling) and replace the unsafe assertion with a
guard that narrows to ParseErrorBody or logs/throws a descriptive error when the
shape is unexpected.
- Line 78: The test helper currently casts response bodies to ParseErrorBody
without validation (error.data = data as ParseErrorBody | undefined), which can
break when the server returns text/HTML or malformed JSON; add a runtime type
guard (e.g., isParseErrorBody) and only assign error.data when the guard passes,
otherwise set error.data = undefined or a safe fallback (or attempt safe JSON
parsing first), referencing the existing symbol ParseErrorBody and the local
error variable so callers accessing error.data.code or error.data.error won’t
hit undefined property errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bf93649-5af9-4f45-b194-baa4c599d15e
📒 Files selected for processing (11)
spec/Analytics.spec.jsspec/helpers/analytics.tsspec/helpers/client.tsspec/helpers/config.tsspec/helpers/errors.tsspec/helpers/globals.d.tsspec/helpers/headers.tsspec/helpers/request.tsspec/server/analytics.spec.tsspec/spec_migration.mdspec/support/jasmine.json
💤 Files with no reviewable changes (1)
- spec/Analytics.spec.js
✅ Files skipped from review due to trivial changes (6)
- spec/helpers/errors.ts
- spec/helpers/config.ts
- spec/helpers/globals.d.ts
- spec/helpers/analytics.ts
- spec/support/jasmine.json
- spec/spec_migration.md
Guard undefined response data in find/count helpers and fix grammar in the analytics spec description.
Use a dedicated maintenance-key value in buildHeaders, assert the Parse-error shape in expectParseError, drop the redundant local reconfigureServer decl (it lives in globals.d.ts), and fix the REST-client description in the plan.
Internal-server-error bodies use { code, message } rather than { code, error },
so a numeric code is the reliable signal that a rejection is a Parse error.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/helpers/client.ts`:
- Line 38: Update the helper methods returning CreateResult or the other typed
results around the affected return statements to validate res.data before
casting or returning it. When the response body is missing, throw a descriptive
helper-level error; preserve the existing typed return behavior for present
response data across all three methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 625c51a1-91bf-4c86-8177-3531d5ee1908
📒 Files selected for processing (14)
.gitignorepackage-lock.jsonpackage.jsonspec/Analytics.spec.jsspec/helpers/analytics.tsspec/helpers/client.tsspec/helpers/config.tsspec/helpers/errors.tsspec/helpers/globals.d.tsspec/helpers/headers.tsspec/helpers/request.tsspec/server/analytics.spec.tsspec/support/jasmine.jsonspec/support/tsRegister.js
💤 Files with no reviewable changes (1)
- spec/Analytics.spec.js
| body: data, | ||
| auth, | ||
| }); | ||
| return res.data as CreateResult; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject missing response bodies before returning typed results.
Lines 38, 65, and 81 cast undefined into required result types, even though restRequest() explicitly permits empty/non-JSON success bodies. Validate res.data and throw a descriptive helper-level error instead of letting an invalid value escape to downstream specs.
Also applies to: 65-65, 81-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/helpers/client.ts` at line 38, Update the helper methods returning
CreateResult or the other typed results around the affected return statements to
validate res.data before casting or returning it. When the response body is
missing, throw a descriptive helper-level error; preserve the existing typed
return behavior for present response data across all three methods.
1 participant
Towards #8787.
First slice of the REST-first spec migration:
@babel/registerhook (spec/support/tsRegister.js) loaded by Jasmine beforehelper.jsso.tsspecs/helpers run at require time. Glob widened to discover.tsspecs. Existing.jsspecs are unaffected — the hook only intercepts.tsunderspec/.spec/helpers/*.tsis a small typedfetchwrapper plus an object CRUD/query client and Parse-header builders. No Parse JS SDK, noparse-serverinternals — true black-box REST.spec/Analytics.spec.js→spec/server/analytics.spec.ts. No SDK import, nodone(), async/await throughout.spec/spec_migration.mddescribes the per-file checklist and phasing used by the upcoming PRs.Next: GeoPoint migration in a follow-up PR using the same helpers.
Summary by CodeRabbit
New Features
Tests
Chores