Migrated Identity functional tests to Vitest+Playwright+MSW#1540
Migrated Identity functional tests to Vitest+Playwright+MSW#1540carterworks wants to merge 14 commits into
Conversation
|
|
| Filename | Overview |
|---|---|
| packages/browser/test/integration/specs/Identity/identity.spec.js | New 1082-line integration test file replacing 20 functional TestCafe tests; two test assertions are trivially vacuous — the expired-adobe_mc and cookie-over-FPID tests check mock response values instead of request bodies, so they cannot detect the regressions they're designed to catch. |
Reviews (2): Last reviewed commit: "test(integration): migrate identity func..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return HttpResponse.json( | ||
| { | ||
| type: "https://ns.adobe.com/aep/errors/EXEG-0003-400", | ||
| status: 400, | ||
| title: "Invalid request", | ||
| detail: "INVALID_ID is not a valid ECID value.", | ||
| report: {}, | ||
| }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
| return HttpResponse.text( | ||
| await readFile( | ||
| `${server.config.root}/packages/browser/test/integration/helpers/mocks/sendEventWithIdentityCookieResponse.json`, | ||
| ), | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| // NOTE: This local acquireHandler is intentionally NOT replaced by the shared | ||
| // acquireHandler exported from helpers/mswjs/handlers.js. The shared handler | ||
| // uses acquireResponse.json (no identity cookie, no locationHint:result, no | ||
| // x-adobe-edge header), while this handler uses identityAcquireResponse.json |
There was a problem hiding this comment.
Content-Type on valid-path response
interactInvalidIdErrorHandler's fallback (non-INVALID_ID) path returns the JSON file via HttpResponse.text(), which MSW sets to Content-Type: text/plain. If alloy's edge-response parser requires application/json (or relies on the header to decide whether to call .json() vs .text()), it will silently fail to deserialize the response when this handler is active. The skipped test using this handler (test.skip("identity established after a failed sendEvent"…)) masks the problem now, but anyone removing the test.skip will encounter a confusing failure that isn't described in the skip comment. Use HttpResponse.json() or set the Content-Type header explicitly — the same pattern the sibling interactWithIdentityHandler already follows.
| }); | ||
| await alloy("sendEvent", {}); | ||
|
|
||
| const warnCalls = warnSpy.mock.calls.map((args) => args.join(" ")); | ||
| const identityWarning = warnCalls.find( | ||
| (msg) => | ||
| msg.includes("Identity cookie not found") || | ||
| msg.includes("invalid-org-id"), | ||
| ); | ||
| expect(identityWarning).toBeDefined(); | ||
|
|
||
| warnSpy.mockRestore(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("C6842980: FPID from the identityMap is used to generate an ECID", () => { | ||
| const fpidEvent = { | ||
| xdm: { | ||
| identityMap: { |
There was a problem hiding this comment.
warnSpy.mockRestore() is called unconditionally at the end of the test body. If either alloy("configure", …) or alloy("sendEvent", {}) throws, the spy is never restored, and console.warn remains patched for every subsequent test in the suite. This can produce false positives or false negatives in other tests that check for warnings. Wrap the spy in a try/finally block or call warnSpy.mockRestore() in a finally clause.
| /* | ||
| Copyright 2026 Adobe. All rights reserved. | ||
| This file is licensed to you under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. You may obtain a copy | ||
| of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software distributed under | ||
| the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS | ||
| OF ANY KIND, either express or implied. See the License for the specific language | ||
| governing permissions and limitations under the License. | ||
| */ | ||
|
|
||
| import { http, HttpResponse } from "msw"; | ||
| // eslint-disable-next-line import/no-unresolved | ||
| import { server } from "vitest/browser"; | ||
| import { | ||
| test, | ||
| expect, | ||
| describe, | ||
| beforeEach, | ||
| vi, | ||
| } from "../../helpers/testsSetup/extend.js"; | ||
| import alloyConfig from "../../helpers/alloy/config.js"; | ||
| import deleteCookies from "../../helpers/utils/deleteCookies.js"; | ||
| import { withTemporaryUrl } from "../../helpers/utils/location.js"; | ||
| import { | ||
| MAIN_IDENTITY_COOKIE_NAME, | ||
| LEGACY_IDENTITY_COOKIE_NAME, | ||
| } from "../../helpers/constants/cookies.js"; | ||
|
|
||
| const { readFile } = server.commands; | ||
|
|
||
| // The ECID that all mock responses return | ||
| const MOCK_ECID = "41861666193140161934276845651148876988"; | ||
|
|
||
| const interactWithIdentityHandler = http.post( | ||
| /https:\/\/edge\.adobedc\.net\/ee\/.*\/?v1\/interact/, | ||
| async () => { | ||
| return new HttpResponse( | ||
| await readFile( | ||
| `${server.config.root}/packages/browser/test/integration/helpers/mocks/sendEventWithIdentityCookieResponse.json`, | ||
| ), | ||
| { | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "x-adobe-edge": "or2;35", | ||
| }, | ||
| }, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
The PR description lists 20 functional tests as "replaced", but six have no corresponding describe/test (nor a test.skip with a rationale) in this file:
- C10922 — demdex usage (requires third-party cookies and a live demdex domain)
- C5594865 — identity maintained across domains via
adobe_mc(requires multi-origin page navigation) - C5594866 — identity changed via
adobe_mcacross domains (same reason) - C15325238 — multiple
adobe_mcparameters, last wins (requires navigation) - C21636436 — ECID preserved after
documentUnloading: true/ collect beacon - C21636437 — demdex fallback when demdex is blocked
Compare with the C14699834 sendEvent sub-test, which is properly marked test.skip with an explanation. Adding analogous skipped stubs for these six cases would make the coverage gap explicit and prevent them from silently disappearing when the functional files are eventually deleted.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
carterworks
force-pushed
the
migrate-integration/07-data-collector
branch
from
758f9f6 to
21d9792
Compare
carterworks
force-pushed
the
migrate-integration/08-identity
branch
from
ca0e47e to
d2306e5
Compare
carterworks
force-pushed
the
migrate-integration/07-data-collector
branch
from
21d9792 to
e563a97
Compare
carterworks
force-pushed
the
migrate-integration/08-identity
branch
from
d2306e5 to
d8c5f10
Compare
There was a problem hiding this comment.
carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
carterworks
changed the base branch from
migrate-integration/07-data-collector
to
migrate-integration/00-infra
There was a problem hiding this comment.
carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
carterworks
force-pushed
the
migrate-integration/08-identity
branch
4 times, most recently
from
27f519e to
3f983d0
Compare
carterworks
force-pushed
the
migrate-integration/08-identity
branch
from
3f983d0 to
ac1ac86
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Port the functional C2581 assertion that the second, queued interact request carries the resolved identity. The migrated test only checked call count and status; now it asserts request #2's body contains the identity cookie value established by the first response (the kndctr cookie rides in state entries).
The migrated C21636438 proved in-memory caching (two getIdentity calls in one session), not the functional test's cookie-decode-on-init behavior. Replace it with three tests that use a fresh alloy plus a pre-set cookie: - a valid identity cookie is decoded to the ECID with zero acquire requests - a non-base64 cookie falls back to an acquire request - a base64-but-invalid-protobuf cookie falls back to an acquire request Restores parity with the two malformed-cookie fallback cases that were previously dropped as known baseline failures.
Functional C19160486 has six sub-cases; the migration faithfully covered only two (ECID-only fetch when 3p disabled, and the CORE-disabled error). Replace the third test — which proved in-memory caching rather than the cookie-decode it claimed, and whose functional origin was CORE-from-cookie — with skip stubs for the four CORE-requiring sub-cases (cross-domain x2, ECID/CORE separately, CORE-from-cookie). CORE is only minted via demdex under third-party-cookie support, the same blocker as C10922; faking it would test the mock. Real ECID-from-cookie coverage now lives in C21636438.
…0-82 Alloy has no FPID-specific code; FPID->ECID derivation and cookie-over-FPID precedence are Experience Edge behaviors that a static mock cannot prove. The migrated tests asserted ECID equality against a mock that always returns the same ECID, which proved nothing about alloy. Reframe to alloy's actual contract: - C6842980/81: alloy forwards the identityMap FPID on every request - C6842982: after an identity is established, a later request carries that identity (kndctr cookie via state entries) alongside the user FPID Also correct the C6842981 note, which claimed a nonexistent fpidCookieName alloy option; the custom-FPID-cookie mapping is datastream/edge-side, not in alloy.
- C5594871: drop the echo handler's adobeMcEcid fallback so identity.ECID is
only satisfied when alloy actually forwards the adobe_mc ECID (was passing
vacuously against a hard-coded value)
- C5598188: require both functional warnings ("Identity cookie not found" and
the org id) instead of either one
- C14699834: match the specific "myinvalidoverride" token in the getIdentity
error rather than a loose 400|invalid regex, matching the functional assert
The migrated C5287654 only asserted the mock response payload's SameSite value, which is vacuous. Read the stored browser cookie via the CookieStore API and assert alloy actually writes it with sameSite=none and secure=true (the browser only honors SameSite=None alongside Secure; localhost is a secure context).
The test asserts alloy carries the established identity alongside a later FPID; cookie-over-FPID precedence is decided at the edge, not by alloy. Retitle the describe so it matches what is actually verified.
Delete the functional Identity specs whose behavior is now covered by real tests in test/integration/specs/Identity/identity.spec.js: C2581, C25822, C1703722, C1703723, C5287654, C5594871, C5594872, C5598188, C6842980, C6842981, C6842982, C21636438. Kept: specs whose integration coverage is (or contains) a documented test.skip because the behavior cannot run in the hermetic harness — C5594865, C5594866, C15325238, C10922, C21636436, C21636437 (skip-only), and C14699834, C19160486 (partially migrated, edge-only sub-cases still skipped).
C6842980 and C6842982 verify Experience Edge behaviors that a mock cannot prove: the edge deriving a stable ECID from an FPID, and an established identity taking precedence over a later FPID. Like the original functional tests, both hit the real int edge by leaving /interact unhandled (worker uses onUnhandledRequest: bypass) and assert on the ECID the edge returns — no mocks. C6842981 keeps its hermetic check that alloy forwards the FPID. Adds reinitializeAlloy() to get a fresh session mid-test (mirrors the functional reloadPage).
carterworks
force-pushed
the
migrate-integration/08-identity
branch
from
ed13a8d to
8acb09e
Compare
Keep the original testcafe functional specs alongside the new Vitest+Playwright+MSW integration suite until these migration branches merge, so reviewers retain the pre-migration signal.
1 participant
Changed Packages
Description
Migrates the Identity functional tests to the new Vitest+Playwright+MSW harness.
Related Issue
Part of the functional test → integration test migration. See
packages/browser/test/FUNCTIONAL_MIGRATION_PLAN.md.Motivation and Context
The existing TestCafe functional test suite is being migrated to Vitest+Playwright+MSW to enable faster, more reliable CI testing without a running server. This PR is part of a stacked series — each PR migrates one test file.
Functional tests replaced:
packages/browser/test/functional/specs/Identity/C2581.jspackages/browser/test/functional/specs/Identity/C10922.jspackages/browser/test/functional/specs/Identity/C25822.jspackages/browser/test/functional/specs/Identity/C5287654.jspackages/browser/test/functional/specs/Identity/C5594865.jspackages/browser/test/functional/specs/Identity/C5594866.jspackages/browser/test/functional/specs/Identity/C5594871.jspackages/browser/test/functional/specs/Identity/C5594872.jspackages/browser/test/functional/specs/Identity/C5598188.jspackages/browser/test/functional/specs/Identity/C6842980.jspackages/browser/test/functional/specs/Identity/C6842981.jspackages/browser/test/functional/specs/Identity/C6842982.jspackages/browser/test/functional/specs/Identity/C1703722.jspackages/browser/test/functional/specs/Identity/C1703723.jspackages/browser/test/functional/specs/Identity/C14699834.jspackages/browser/test/functional/specs/Identity/C15325238.jspackages/browser/test/functional/specs/Identity/C19160486.jspackages/browser/test/functional/specs/Identity/C21636436.jspackages/browser/test/functional/specs/Identity/C21636437.jspackages/browser/test/functional/specs/Identity/C21636438.jsTypes of changes
Checklist:
Stack