Skip to content

Migrated Location Hints functional tests to Vitest+Playwright+MSW#1537

Open
carterworks wants to merge 8 commits into
migrate-integration/00-infrafrom
migrate-integration/05-location-hints
Open

Migrated Location Hints functional tests to Vitest+Playwright+MSW#1537
carterworks wants to merge 8 commits into
migrate-integration/00-infrafrom
migrate-integration/05-location-hints

Conversation

@carterworks

@carterworks carterworks commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Changed Packages

  • core
  • reactor-extension

Description

Migrates the Location Hints 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/Location Hints/C6589015.js
  • packages/browser/test/functional/specs/Location Hints/C6944931.js

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which does not add functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have added a Changeset file with a consumer-facing description of my changes.
  • I have signed the Adobe Open Source CLA or I'm an Adobe employee.
  • I have made any necessary test changes and all tests pass.
  • I have run the Sandbox successfully.

Stack

  1. Added MSW handlers, mock fixtures, and Vitest+Playwright test harness for functional test migration #1532
  2. Migrated Install SDK functional tests to Vitest+Playwright+MSW #1533

@carterworks carterworks added the ignore-for-release Do not include this PR in release notes label Jun 12, 2026
@carterworks carterworks self-assigned this Jun 12, 2026
@changeset-bot

changeset-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a20f511

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR migrates two TestCafe functional tests for Location Hints (C6589015 and C6944931) into the new Vitest+Playwright+MSW integration harness as a single spec file.

  • C6589015 verifies that the EdgeNetwork location hint returned in the first edge response is embedded in the second request's URL path. It uses the shared sendEventHandler, reads the cluster cookie after the first call, and then asserts the second request URL contains that hint segment.
  • C6944931 verifies that a pre-existing mboxEdgeCluster=38 cookie (legacy Adobe Target hint) is translated to the /t38/ path segment on the first request, and that the sgp3 hint returned in the mocked edge response takes over for the second request. A self-contained inline handler (sgp3LocationHintHandler) provides the mock response.

Confidence Score: 5/5

Safe to merge — this is a straightforward test migration with no changes to production code.

The change adds a single new test spec file with no production code modifications. Both tests correctly wire up MSW handlers, clean up cookies via the shared deleteCookies() utility (which covers all cookies including mboxEdgeCluster), and assert the exact URL path segments that prove location-hint propagation is working. No new logic is introduced that could regress existing behavior.

No files require special attention.

Important Files Changed

Filename Overview
packages/browser/test/integration/specs/Location Hints/locationHints.spec.js New spec file migrating C6589015 and C6944931 to Vitest+Playwright+MSW. Test logic is faithful to the originals; deleteCookies() covers all cookies including mboxEdgeCluster, so cross-test contamination is not a concern.

Sequence Diagram

sequenceDiagram
    participant T as Test
    participant A as Alloy SDK
    participant W as MSW Worker
    participant NR as NetworkRecorder

    Note over T,NR: C6589015 — EdgeNetwork hint propagation
    T->>T: deleteCookies()
    T->>W: worker.use(sendEventHandler)
    T->>A: configure(alloyConfig)
    A->>W: POST /ee/v1/interact (no hint)
    W-->>A: "sendEventResponse.json (hint: or2, state:store cluster=or2)"
    A->>A: "Set kndctr_..._cluster=or2 cookie"
    T->>T: "Read cluster cookie → locationHint=or2"
    T->>A: "sendEvent #2"
    A->>W: POST /ee/or2/v1/interact
    W-->>A: sendEventResponse.json
    T->>NR: findCalls(edge.adobedc.net, minCalls:2)
    NR-->>T: [call1, call2]
    T->>T: assert call1 URL has no hint segment
    T->>T: assert call2 URL contains /ee/or2/

    Note over T,NR: C6944931 — Legacy mboxEdgeCluster translation
    T->>T: deleteCookies()
    T->>T: "Set mboxEdgeCluster=38"
    T->>W: worker.use(sgp3LocationHintHandler)
    T->>A: configure(alloyConfig)
    A->>W: "POST /ee/t38/v1/interact (from mboxEdgeCluster=38)"
    W-->>A: "sgp3 response (hint: sgp3, state:store cluster=sgp3)"
    A->>A: "Set kndctr_..._cluster=sgp3 cookie"
    T->>A: "sendEvent #2"
    A->>W: POST /ee/sgp3/v1/interact
    W-->>A: sgp3 response
    T->>NR: findCalls(edge.adobedc.net, minCalls:2)
    NR-->>T: [call1, call2]
    T->>T: assert call1 URL contains /ee/t38/
    T->>T: assert call2 URL contains /ee/sgp3/
Loading

Reviews (2): Last reviewed commit: "test(integration): migrate location hint..." | Re-trigger Greptile

Comment on lines +23 to +55
/https:\/\/edge.adobedc.net\/ee\/.*\/?v1\/interact/,
() => {
return HttpResponse.json({
requestId: "sgp3-location-hint-test",
handle: [
{
payload: [
{
id: "41861666193140161934276845651148876988",
namespace: { code: "ECID" },
},
],
type: "identity:result",
},
{
payload: [
{ scope: "Target", hint: "38", ttlSeconds: 1800 },
{ scope: "AAM", hint: "3", ttlSeconds: 1800 },
{ scope: "EdgeNetwork", hint: "sgp3", ttlSeconds: 1800 },
],
type: "locationHint:result",
},
{
payload: [
{
key: MAIN_CLUSTER_COOKIE_NAME,
value: "sgp3",
maxAge: 1800,
},
],
type: "state:store",
},
],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 sgp3LocationHintHandler does not validate configId

Every handler in helpers/mswjs/handlers.js extracts and checks configId before responding (e.g. sendEventHandler, pullDestinationsHandler, setConsentHandler). The inline sgp3LocationHintHandler skips that check entirely, so it will intercept and return the sgp3 response for any POST to the interact endpoint regardless of the configured datastream. While the handler is reset after each test, adding the same configId guard (using alloyConfig.datastreamId) would keep the handler consistent with the rest of the test harness and prevent a misconfigured alloy instance from silently matching.

Comment on lines +98 to +134
new RegExp(`edge\\.adobedc\\.net/ee/${locationHint}/v1/interact`),
);
});

// C6944931 - The legacy Adobe Target location hint is used.
test("C6944931 - legacy mboxEdgeCluster cookie is translated to a location hint on the first request", async ({
alloy,
worker,
networkRecorder,
}) => {
await deleteCookies();

// Set mboxEdgeCluster=38 (Singapore, Konductor region ID 3 = sgp3)
// Alloy reads this cookie and uses it as the initial location hint (/t38/)
document.cookie = "mboxEdgeCluster=38; path=/";

worker.use(sgp3LocationHintHandler);

await alloy("configure", {
...alloyConfig,
thirdPartyCookiesEnabled: false,
debugEnabled: false,
});
await alloy("sendEvent", {});
await alloy("sendEvent", {});

const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, {
minCalls: 2,
});
expect(calls.length).toBe(2);

// First request uses the legacy mboxEdgeCluster=38 hint (/t38/ in URL path)
expect(calls[0].request.url).toMatch(/edge\.adobedc\.net\/ee\/t38\//);
// Second request uses the sgp3 hint returned in the edge response
expect(calls[1].request.url).toMatch(/edge\.adobedc\.net\/ee\/sgp3\//);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Missing intermediate cookie assertion from original C6944931 test

The original C6944931.js (line 56) explicitly verified that after the first sendEvent the cluster cookie was set to "sgp3". The new test drops this assertion and jumps straight to URL-pattern checks. If the state:store response were mishandled (cookie name mismatch, wrong key, alloy bug in storing location hints), the second URL would silently fall back to the t38 path and the second assertion would fail — but the reason (the cookie was never set) would not be surfaced clearly. Adding the explicit cookie check before the second sendEvent preserves the diagnostic value of the original test.

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 carterworks force-pushed the migrate-integration/04-library-info branch from 6d4a68b to 71ad5f5 Compare June 12, 2026 17:31
@carterworks carterworks force-pushed the migrate-integration/05-location-hints branch from 2fb56e2 to e183d80 Compare June 12, 2026 17:31
This was referenced Jun 12, 2026
@carterworks carterworks force-pushed the migrate-integration/04-library-info branch from 71ad5f5 to 10f1006 Compare June 12, 2026 17:55
@carterworks carterworks force-pushed the migrate-integration/05-location-hints branch from e183d80 to 8aa9f6b Compare June 12, 2026 17:56

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@carterworks carterworks changed the base branch from migrate-integration/04-library-info to migrate-integration/00-infra June 12, 2026 17:57

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@carterworks carterworks force-pushed the migrate-integration/05-location-hints branch 4 times, most recently from 2126b41 to 8e6aa0d Compare June 12, 2026 21:20
@carterworks carterworks marked this pull request as draft June 15, 2026 16:38
@carterworks carterworks force-pushed the migrate-integration/05-location-hints branch from 8e6aa0d to 0fa5f26 Compare June 15, 2026 19:59
@carterworks carterworks marked this pull request as ready for review June 16, 2026 16:36
… CookieStore API

- Migrate document.cookie reads/writes to CookieStore API (cookieStore.get/set)
- Add configId guard to inline sgp3LocationHintHandler matching convention in
  consent_gate.spec.js; throws when configId does not match the configured
  datastreamId so misconfigured handlers surface immediately
- Restore intermediate cluster-cookie assertion in C6944931 between the two
  sendEvent calls so a storage failure surfaces here rather than as a confusing
  URL-path mismatch on the second request
@carterworks carterworks force-pushed the migrate-integration/05-location-hints branch from 1d7dc76 to 04be6b9 Compare June 26, 2026 20:19
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore-for-release Do not include this PR in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant