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 7 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds ChangesTypeScript Support for Jasmine Specs
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): [00.18][ERROR]: Error: exception Unix_error: No such file or directory stat spec/helpers/config.ts 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.60% 92.58% -0.02%
==========================================
Files 192 193 +1
Lines 16324 16893 +569
Branches 200 234 +34
==========================================
+ Hits 15117 15641 +524
- Misses 1190 1229 +39
- Partials 17 23 +6 ☔ View full report in Codecov by Sentry. 🚀 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.
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
Chores
Tests
Documentation