Skip to content

test: build the cache provider context with a type assertion - #20

Merged
TorstenDittmann merged 1 commit into
appwrite:mainfrom
ChiragAgg5k:fix/ci
Nov 4, 2026
Merged

TorstenDittmann merged 1 commit into
appwrite:mainfrom
ChiragAgg5k:fix/ci

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

What

Renovate's Astro 7.3.3 security update (#18) fails Type check: test/cache-provider.test.js(31,28): Property 'logger' is missing. Main is green on Astro 7.2; the failure comes from the upgrade itself.

Why

Astro 7.3.3 makes logger a required member of the context passed to a cache provider's onRequest. The test helper builds that context as an object literal without it. The provider never reads logger, so only the type changed.

Fix

Assert the helper's context to Parameters<typeof instance.onRequest>[0]. That keeps the test valid against both Astro 7.2 (current lock) and 7.3.3, so this can merge first and #18 passes after a rebase.

Verified locally: npm run typecheck and npm test (67 pass) on the current lock and with astro@7.3.3 installed; biome format and biome lint are clean. Without the change, astro@7.3.3 reproduces the TS2345 error.

Opened from a fork because I don't have push access to this repository.

Astro 7.3.3 adds a required logger to the context onRequest receives, so the test helper object fails tsc on the Renovate upgrade. The provider never reads it; asserting the context type keeps the helper valid on 7.2 and 7.3.
@hansi-codes

hansi-codes Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

🟢 Tier S · Ready to merge

The change is limited to a type assertion in a test helper and introduces no runtime behavior change.

The cache-provider test helper now creates the request context separately and casts it to the parameter type of onRequest. This allows the test context to type-check against Astro versions with differing context requirements.

Verdict New comments Fixed Still open
✅ Approved 0 0 0
📂 Walkthrough · 1
File Change
test/cache-provider.test.js Builds the helper context and types it from the provider's onRequest parameter.

Reviewed 8eb967d · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Tier S · Looks good to merge. Summary

@TorstenDittmann
TorstenDittmann merged commit 47819b2 into appwrite:main Nov 4, 2026
9 checks passed
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