test: mock adobe api responses#819
Merged
Merged
Conversation
commit: |
Deploying fonts with
|
| Latest commit: |
65bf33a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://48e3afb8.fonts-dsw.pages.dev |
| Branch Preview URL: | https://test-adobe-provider-flake.fonts-dsw.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
=======================================
Coverage 68.47% 68.47%
=======================================
Files 9 9
Lines 295 295
Branches 67 67
=======================================
Hits 202 202
Misses 75 75
Partials 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis pull request introduces test infrastructure for mocking Adobe Typekit font API interactions. A new fixture module exports Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/fixtures/adobe/index.ts (2)
4-8: Nit: preferpath.joinover template-literal path concat.
fileURLToPath(new URL('.', import.meta.url))returns a trailing-slash path, so the current${fixtureDir}/${name}produces…/adobe//kit-…json. Linux/macOS tolerate it, butpath.joinis cleaner and portable.♻️ Proposed fix
import { promises as fsp } from 'node:fs' +import { join } from 'node:path' import { fileURLToPath } from 'node:url' const fixtureDir = fileURLToPath(new URL('.', import.meta.url)) async function readFixture(name: string) { - return fsp.readFile(`${fixtureDir}/${name}`, 'utf8') + return fsp.readFile(join(fixtureDir, name), 'utf8') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/adobe/index.ts` around lines 4 - 8, The readFixture helper currently concatenates paths using a template literal which can produce duplicate slashes; update readFixture to use path.join to build the file path (use the existing fixtureDir from fileURLToPath(new URL('.', import.meta.url)) and join it with name), and add the required import for path (or import { join } from 'path') so the function becomes fsp.readFile(path.join(fixtureDir, name), 'utf8') — update references to fixtureDir, readFixture, and fileURLToPath accordingly.
11-30: MakemockAdobeFetch()idempotent and restorable.Two concerns:
- Re-invocation hazard. If
mockAdobeFetch()is ever called more than once (e.g. another test file imports it, or a future setup hook), the second call will capture the already-patchedglobalThis.fetchasrealFetch. Non-Typekit fall-through requests then route through the previous mock unnecessarily — and a third call would set up a self-referential chain. Cheap to guard against now.- No teardown. Because the patch is permanent for the worker, any later test that legitimately needs real
fetch(or a different mock) inherits this one. Returning a restore handle (or registeringafterAll) keeps the surface honest.♻️ Proposed fix
-/** Replace adobe HTTP calls with shape-only stubs so the suite doesn't depend on `typekit.com`. */ -export function mockAdobeFetch() { - const realFetch = globalThis.fetch - globalThis.fetch = async function mockedFetch(input, init) { +/** Replace adobe HTTP calls with shape-only stubs so the suite doesn't depend on `typekit.com`. */ +export function mockAdobeFetch() { + const mocked = globalThis.fetch as typeof fetch & { __adobeMock?: true } + if (mocked.__adobeMock) return () => {} + const realFetch = globalThis.fetch + const mockedFetch = async function mockedFetch(input: RequestInfo | URL, init?: RequestInit) { const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url const kitMeta = url.match(/^https:\/\/typekit\.com\/api\/v1\/json\/kits\/([^/]+)\/published/) if (kitMeta) { const body = await readFixture(`kit-${kitMeta[1]}.json`) return new Response(body, { status: 200, headers: { 'content-type': 'application/json' } }) } const kitCss = url.match(/^https:\/\/use\.typekit\.net\/([^/]+)\.css/) if (kitCss) { const body = await readFixture(`${kitCss[1]}.css`) return new Response(body, { status: 200, headers: { 'content-type': 'text/css' } }) } if (/^https:\/\/use\.typekit\.net\/stub\//.test(url)) { return new Response(new Uint8Array(), { status: 200, headers: { 'content-type': 'font/woff2' } }) } - return realFetch(input as RequestInfo, init) - } as typeof fetch -} + return realFetch(input as RequestInfo, init) + } as typeof fetch & { __adobeMock?: true } + mockedFetch.__adobeMock = true + globalThis.fetch = mockedFetch + return () => { globalThis.fetch = realFetch } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/adobe/index.ts` around lines 11 - 30, The mockAdobeFetch function should be made idempotent and provide a restore mechanism: detect if globalThis.fetch is already our mock (use a unique marker like a Symbol or a hidden property on the wrapper) and if so, do nothing (or return the existing restore function); when first invoked capture the realFetch from the true original fetch, install the mockedFetch (attach the marker and a restore function that sets globalThis.fetch back to realFetch and clears the marker), and return that restore function; also optionally register that restore with the test framework (e.g. call afterAll to invoke the restore) so the mock is torn down automatically. Ensure references to mockAdobeFetch, realFetch, globalThis.fetch and the mockedFetch wrapper are used to locate and implement this change.test/fixtures/adobe/grx7wdj.css (1)
1-11: Excludetest/fixtures/adobe/from stylelint.These fixtures intentionally mirror Adobe Typekit’s actual output (which quotes
font-family), so thefont-family-name-quoteserrors flagged by Stylelint here (and intest/fixtures/adobe/sij5ufr.cssat the same lines) are noise — “fixing” them would make the fixtures diverge from what the real provider returns. Add an ignore entry rather than per-file disable comments so future fixtures stay clean.♻️ Suggested change to `.stylelintignore` (or `ignoreFiles` in your stylelint config)
+test/fixtures/adobe/**/*.css🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/adobe/grx7wdj.css` around lines 1 - 11, Add an ignore entry to the Stylelint config (either .stylelintignore or "ignoreFiles") to exclude the Adobe fixtures directory so Stylelint won't flag the intentionally quoted font-family values (e.g., font-family:"barlow-semi-condensed" found in the Adobe fixture CSS and the other fixture with the same issue) for the rule font-family-name-quotes; update the ignore pattern to cover the Adobe fixtures folder so future provider-mirroring files are not modified or annotated with per-file disables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/fixtures/adobe/grx7wdj.css`:
- Around line 1-11: Add an ignore entry to the Stylelint config (either
.stylelintignore or "ignoreFiles") to exclude the Adobe fixtures directory so
Stylelint won't flag the intentionally quoted font-family values (e.g.,
font-family:"barlow-semi-condensed" found in the Adobe fixture CSS and the other
fixture with the same issue) for the rule font-family-name-quotes; update the
ignore pattern to cover the Adobe fixtures folder so future provider-mirroring
files are not modified or annotated with per-file disables.
In `@test/fixtures/adobe/index.ts`:
- Around line 4-8: The readFixture helper currently concatenates paths using a
template literal which can produce duplicate slashes; update readFixture to use
path.join to build the file path (use the existing fixtureDir from
fileURLToPath(new URL('.', import.meta.url)) and join it with name), and add the
required import for path (or import { join } from 'path') so the function
becomes fsp.readFile(path.join(fixtureDir, name), 'utf8') — update references to
fixtureDir, readFixture, and fileURLToPath accordingly.
- Around line 11-30: The mockAdobeFetch function should be made idempotent and
provide a restore mechanism: detect if globalThis.fetch is already our mock (use
a unique marker like a Symbol or a hidden property on the wrapper) and if so, do
nothing (or return the existing restore function); when first invoked capture
the realFetch from the true original fetch, install the mockedFetch (attach the
marker and a restore function that sets globalThis.fetch back to realFetch and
clears the marker), and return that restore function; also optionally register
that restore with the test framework (e.g. call afterAll to invoke the restore)
so the mock is torn down automatically. Ensure references to mockAdobeFetch,
realFetch, globalThis.fetch and the mockedFetch wrapper are used to locate and
implement this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 117d0ec7-8db4-4ac5-930e-26a94f849bdb
📒 Files selected for processing (6)
test/basic.test.tstest/fixtures/adobe/grx7wdj.csstest/fixtures/adobe/index.tstest/fixtures/adobe/kit-grx7wdj.jsontest/fixtures/adobe/kit-sij5ufr.jsontest/fixtures/adobe/sij5ufr.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description
this aims to make suite less flaky in ecosystem-ci