Migrated Media Collection functional tests to Vitest+Playwright+MSW

[carterworks]

Changed Packages

• core
• reactor-extension

Description

Migrates the Media Collection functional tests to the new Vitest+Playwright+MSW harness. MA1 (auto-ping mode) is preserved as test.skip — ping timing assertions require 11s sleep, impractical in CI. MA2 uses getMediaAnalyticsTracker directly in browser mode.

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/MediaCollection/MA1.js
• packages/browser/test/functional/specs/MediaCollection/MA2.js
• packages/browser/test/functional/specs/MediaCollection/MA3.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

[changeset-bot]

🦋 Changeset detected

Latest commit: 4366224

The changes in this PR will be included in the next version bump.

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

Not sure what this means? Click here to learn what changesets are.

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

[greptile-apps]

Greptile Summary

This PR migrates the Media Collection functional tests (MA1, MA2, MA3) from the TestCafe harness to Vitest+Playwright+MSW. MA1 (automatic ping mode) is correctly preserved as test.skip with a clear explanation; MA2 and MA3 are fully rewritten as browser-mode integration tests using the existing MSW handler infrastructure.

• MA3 (createMediaSession + sendMediaEvent non-automatic mode): covers 14 event types end-to-end, but media.chapterSkip — explicitly tested in both the original MA3 and MA1 — has been dropped and replaced with media.sessionComplete, keeping the count at 14 and silently hiding the gap.
• MA2 (legacy getMediaAnalyticsTracker): verifies session-start and three /va/ events, but the sessionCalls.length === 1 guard uses a broad edge.adobedc.net pattern rather than scoping to the /interact path, making the assertion fragile to additional edge traffic.

Confidence Score: 3/5

Safe to merge only after confirming the media.chapterSkip coverage gap is intentional; the missing event type means a routing regression for that event would go undetected.

The MA3 rewrite quietly drops media.chapterSkip — present in both original MA3 and MA1 — by swapping in media.sessionComplete to keep the event count at 14, so the count assertion passes without catching the omission. That is a real gap in the replacement test coverage for a regression surface that was previously guarded.

The single changed file packages/browser/test/integration/specs/MediaCollection/mediaCollection.spec.js needs a second look around the expectedEventTypes array and the accompanying event-send loop to confirm media.chapterSkip coverage.

Important Files Changed

Filename	Overview
packages/browser/test/integration/specs/MediaCollection/mediaCollection.spec.js	New Vitest+Playwright+MSW integration test replacing MA1/MA2/MA3 functional tests; MA1 correctly skipped, but MA3 drops media.chapterSkip coverage (swapped for media.sessionComplete) and the MA2 count assertion uses a broad URL pattern that could spuriously fail.

Sequence Diagram

sequenceDiagram
    participant Test
    participant Alloy
    participant MSW_interact as MSW /v1/interact (mediaSessionHandler)
    participant MSW_va as MSW /va/v1/* (mediaEventHandler)
    participant NetworkRecorder

    Note over Test,NetworkRecorder: MA3 — non-automatic mode
    Test->>Alloy: configure(streamingMediaConfig)
    Test->>Alloy: createMediaSession(xdm)
    Alloy->>MSW_interact: POST /ee/.../v1/interact (media.sessionStart)
    MSW_interact-->>Alloy: 200 mediaSessionResponse.json (sessionId)
    MSW_interact-->>NetworkRecorder: captureRequest / captureResponse
    Alloy-->>Test: "{ sessionId }"
    loop 14 sendMediaEvent calls
        Test->>Alloy: sendMediaEvent(xdm)
        Alloy->>MSW_va: "POST /ee/.../va/v1/<eventType>"
        MSW_va-->>Alloy: 204
        MSW_va-->>NetworkRecorder: captureRequest / captureResponse
    end
    Test->>NetworkRecorder: "findCalls(/\/va\//, { minCalls: 14 })"
    NetworkRecorder-->>Test: 14 va calls

    Note over Test,NetworkRecorder: MA2 — legacy getMediaAnalyticsTracker
    Test->>Alloy: configure(streamingMediaConfig)
    Test->>Alloy: getMediaAnalyticsTracker()
    Alloy-->>Test: Media object
    Test->>Alloy: tracker.trackSessionStart(mediaInfo, ctx)
    Alloy->>MSW_interact: POST /ee/.../v1/interact (media.sessionStart)
    MSW_interact-->>NetworkRecorder: captureRequest / captureResponse
    Test->>NetworkRecorder: "findCalls(/edge\.adobedc\.net/) — expects length == 1"
    Test->>Alloy: trackPlay() / trackPause() / trackComplete()
    Alloy->>MSW_va: POST /va/ x3
    MSW_va-->>NetworkRecorder: captureRequest / captureResponse
    Test->>NetworkRecorder: "findCalls(/\/va\//, { minCalls: 3 })"
    NetworkRecorder-->>Test: 3 va calls

Loading

_{Reviews (1): Last reviewed commit: "test(integration): migrate media collect..." | Re-trigger Greptile}

[greptile-apps]

media.chapterSkip event type missing from MA3 coverage

Both the original MA3 and MA1 functional tests explicitly covered media.chapterSkip. The new test swaps it out for media.sessionComplete, keeping the count at 14 and making the expect(vaCalls.length).toBe(14) assertion pass silently. A regression where media.chapterSkip fails to route to the /va/ endpoint would now go undetected — none of the non-skipped tests exercise it.

[greptile-apps]

Broad URL pattern makes exact-count assertion fragile in MA2

findCalls(/edge\.adobedc\.net/) matches any call to the edge domain, not only the session-start /interact call. If Alloy ever emits an additional edge call between configure and trackSessionStart (e.g., an identity/acquire or collect call), sessionCalls.length would exceed 1 and the assertion would fail with an opaque message. Scoping the pattern to /v1/interact would make the intent explicit and shield the count check from unrelated edge traffic.

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!

[greptile-apps]

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

[greptile-apps]

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

[greptile-apps]

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

[carterworks]

Integration Test Migration Review — MediaCollection (#16)

Verdict: 🟡 Comment / Request Changes — two non-trivial assertion weakenings and a structural gap (no-ping invariant never migrated) need resolution before the functional files can be retired.

---

Parity Matrix

Old file	Mapped to	Subtests old → new	Active?
MA1.js — auto-ping mode, sleep(11000)	test.skip("MA1 - automatic (ping) mode …")	1 → 0 active	🟡 skip (see §2)
MA2.js — legacy getMediaAnalyticsTracker, 17 event assertions	MA2 - legacy getMediaAnalyticsTracker	17 → 3 active	🔴 significant drop
MA3.js — non-automatic mode, 13 event assertions + no-ping guard	MA3 - non-automatic mode	13 → 14 (swap) but weaker + 0 no-ping	🔴 core assertion dropped

Note on meta IDs: MA2.js carries test.meta({ ID: "MA3" }) — a pre-existing collision with MA3.js. Mapping below is by fixture title and behavior, not meta ID.

---

1. Parity (🔴🟡)

1a. MA3 — media.chapterSkip swapped out, sessionID round-trip dropped

Old MA3.js sends and asserts 13 events including media.chapterSkip. New spec swaps that out for media.sessionComplete (also 14 /va/ events) — netting even on count but the swap is silent: no comment explains why chapterSkip was dropped.

More importantly, old MA3.js assertEventIsSent (line 119) verified sessionID in every event body equals the value returned by createMediaSession. The new test (mediaCollection.spec.js:61) only does:

expect(session.sessionId).toBeDefined();

The mock always returns "test-session-id-123456789012345678901234", so a strict .toBe("test-session-id-123456789012345678901234") is trivially available. More importantly, asserting the correct sessionID is forwarded in each /va/ event is the core behavioral check — that it's defined on the session object alone doesn't prove the ID is threaded through subsequent events.

Similarly, old MA3 checked sessionStart playhead=0 at line 76; the new test doesn't.

Requests:

1. Restore media.chapterSkip (or add a comment explaining why it's intentionally dropped).
2. Assert sessionID in at least one /va/ event body against session.sessionId.
3. Assert sessionStart playhead=0.

1b. MA3 — no-ping-automatic invariant entirely absent (🔴)

Old MA3.js is titled "Test that MC component doesn't send pings automatically" and its defining assertion (assertPingsNotSent) waits 10s then checks that no ping was ever sent. This is the entire behavioral property the test existed to prove.

The new MA3 verifies event routing to /va/ but has zero ping-related assertion — grep for ping in mediaCollection.spec.js returns 0 matches. Combined with MA1's skip, ping cadence is now entirely unverified in the active integration suite.

The 10s wait is impractical in CI for the same reason as MA1, so this needs explicit acknowledgment. Suggested approach: add a short-window negative check (e.g., assert no /va/ event with eventType: "media.ping" appears in the first 2s after session creation — fast, deterministic, and covers the regression case where the ping timer fires immediately). Or document the gap in a comment and a tracking issue.

1c. MA2 — legacy tracker coverage drops from 17 to 3 assertions (🔴)

Old MA2.js (the legacy getMediaAnalyticsTracker test) exercises and asserts against 17 event types via assertEventIsSent: play, pauseStart, chapterStart, chapterComplete, chapterSkip, adBreakStart, adStart, adComplete, adBreakComplete, adSkip, error, bufferStart/Complete, seekStart/Complete, bitrateChange, statesUpdate ×2, plus a ping check.

New MA2 test (mediaCollection.spec.js:280–349) exercises only play, pause, and sessionComplete — 3 /va/ event assertions. The legacy tracker API has many event-type → XDM mappings that are now completely uncovered at the integration level.

This is the largest coverage drop in the PR. If there are unit tests for each mapping, cite them; otherwise this needs either broader exercise of the tracker API or an explicit statement that coverage is intentionally deferred to unit tests.

---

2. Skip Justification (🟡)

test.skip(
  "MA1 - automatic (ping) mode: pings are sent every 10s when using playerId " +
    "(skipped: ping timing assertions require sleep(11000) which is impractical in CI; " +
    "session-start and event routing are covered by the MA3 test and StreamingMedia/mediaEvents.spec.js)",
  async () => {},
);

Old MA1 status: Not skipped — it was an active P0/Regression test (MA1.js:51–54). The two sleep(11000) calls and one sleep(10000) call (lines 149, 299, 113) are real; the timing objection is valid.

Justification quality: 🟡 The comment is honest about the reason but overreaches in claiming session-start and event routing "cover" MA1. The auto-ping mode uses playerId + getPlayerDetails callback (MA1.js:58–80) — a code path not exercised by MA3. The skip is acceptable if the getPlayerDetails/auto-ping code path is covered in unit tests. Verify and reference those in the skip message.

Recommendation: Keep the skip, but update the justification to reference the specific unit test(s) covering getPlayerDetails / ping dispatch. Don't delete MA1.js until a CI-safe ping test (even a mocked-timer version) lands.

---

3. Mock Fidelity (🟢🟡)

mediaSessionHandler (handlers.js:285–302) intercepts /v1/interact and returns mediaSessionResponse.json with a fixed sessionId. mediaEventHandler (handlers.js:304–310) intercepts /ee(\/[^/]+)?\/va\/v1\/ and returns 204.

The handlers are functional. Two observations:

• No ping endpoint handler. Since the new MA3 test makes no ping assertions, this is not a blocker — but it means any accidental ping emission would silently pass (204 from mediaEventHandler covers it). If a no-ping check is added, you'd need a way to distinguish ping calls in the recorder.
• mediaEventHandler regex (/https:\/\/edge.adobedc.net\/ee(\/[^/]+)?\/va\/v1\//) matches the correct /va/ sub-path pattern. 🟢

---

4. Timing / Flake (🟡)

MA3: networkRecorder.findCalls(/\/va\//, { retries: 30, delayMs: 200, minCalls: 14 }) gives a 6s ceiling. All 14 events are fire-and-forget synchronous sendMediaEvent calls; 6s is generous. 🟢

MA2: findCalls(/\/va\//, { retries: 20, delayMs: 100, minCalls: 3 }) — 2s ceiling for 3 events. Fine. 🟢

MA2 session detection: findCalls(/edge\.adobedc\.net/, { retries: 10, delayMs: 100 }) at line 314 — 1s ceiling, no minCalls. If the session request is slightly slow, sessionCalls.length could be 0 and the find(...) at line 320 would return undefined, causing the expect(sessionStartEvent).toBeDefined() to fail non-deterministically. Add minCalls: 1. 🟡

---

5. Isolation (🟢)

extend.js resets networkRecorder before and after each test (lines 47, 50) and calls worker.resetHandlers() after each test (line 42). Cookie jar is cleared before each test (lines 57–63). Handlers are registered per-test via worker.use(...). Isolation is correct.

---

6. Hygiene (🟡)

• License year: mediaCollection.spec.js uses Copyright 2026 while all helpers use 2025. This is fine if the project convention is to use the authoring year — just flag for awareness.
• No test.meta tags: Integration tests don't use test.meta, which is consistent with the rest of the integration suite. 🟢
• vitest.projects.js: The new spec falls under packages/browser/test/integration/** which is already covered by the browser/integration project in packages/browser/vitest.projects.js. No project config change needed. 🟢
• Functional files not deleted: MA1.js, MA2.js, MA3.js remain in the tree. Given that MA1's auto-ping path and MA2's full tracker coverage are not replicated, do not delete these files yet. They document the intended behavior and serve as the reference implementation until the gaps above are closed.
• MA2.js meta ID collision: MA2.js has test.meta({ ID: "MA3" }), duplicating MA3.js. Pre-existing issue not introduced by this PR, but worth cleaning up in a follow-on.
• Comments: The eslint-disable-next-line no-empty-function on the MA1 skip body is correct practice. 🟢

---

Summary

#	Finding	Severity	Blocking?
1	MA3 drops per-event sessionID assertion (non-env-forced)	🔴	Recommend fix
2	MA3 no-ping-automatic invariant entirely absent	🔴	Recommend fix or tracking issue
3	MA2 coverage drops from 17 to 3 event assertions	🔴	Recommend fix
4	media.chapterSkip dropped without comment	🟡	Restore or document
5	MA2 session findCalls missing minCalls: 1 (flake risk)	🟡	Easy fix
6	MA1 skip justification overreaches on coverage claim	🟡	Update comment
7	Functional files MA1–MA3 should not be deleted yet	🟡	Hold
8	Copyright 2026 vs. 2025 in helpers	info	No action needed
9	MA2.js ID:"MA3" meta collision (pre-existing)	info	Follow-on cleanup

The two functional tests that can be retired now are none — MA1's auto-ping path and the full MA2 legacy tracker exercise are both uncovered. The MA3 non-automatic path is migrated but with weakened assertions that should be tightened.