Added documented stubs for At.js migration functional tests#1551
Added documented stubs for At.js migration functional tests#1551carterworks wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 7601646 The changes in this PR will be included in the next version bump. This PR includes no changesetsWhen 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 |
|
| Filename | Overview |
|---|---|
| packages/browser/test/integration/specs/Migration/migration.spec.js | New file adding 8 test.skip stubs for At.js migration tests that require live Target edge infrastructure; no executable logic introduced, but test.todo would be a more semantically accurate marker than test.skip with empty bodies. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TestCafe Functional Tests\nC8085773–C8085780] -->|"still exist (not deleted)"| B[packages/browser/test/functional/specs/Migration/]
C[This PR] -->|adds| D[migration.spec.js\n8x test.skip stubs]
D --> E{test.skip with empty body}
E -->|skipped in CI| F[Vitest reports: SKIPPED]
E -.->|alternative| G[test.todo — reports: TODO\nno function body needed]
Reviews (1): Last reviewed commit: "test(integration): migrate migration fun..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| test.skip( | ||
| "C8085773: Web SDK → At.js 1.x: same session ID and edge cluster (requires At.js 1.x page + real Target edge)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085774: Web SDK → At.js 2.x: same session ID and edge cluster (requires At.js 2.x page + real Target edge)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085775: At.js 1.x → Web SDK: same session ID and edge cluster (requires At.js 1.x start page + real Target edge)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085776: At.js 2.x → Web SDK: same session ID and edge cluster (requires At.js 2.x start page + real Target edge)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085777: Web SDK profile update → At.js 2.x offer fetch (requires real Target edge + cross-page navigation)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085778: Web SDK profile update → At.js 1.x offer fetch (requires real Target edge + cross-page navigation)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085779: At.js 1.x profile update → Web SDK proposition fetch (requires real Target edge + cross-page navigation)", | ||
| () => {}, | ||
| ); | ||
|
|
||
| test.skip( | ||
| "C8085780: At.js 2.x profile update → Web SDK proposition fetch (requires real Target edge + cross-page navigation)", | ||
| () => {}, | ||
| ); |
There was a problem hiding this comment.
test.skip vs test.todo for unimplemented stubs
All eight entries use test.skip(() => {}) with empty bodies. Vitest's test.todo is purpose-built for this situation — it marks a test that has not been written yet, shows up distinctly in the test reporter as "todo" (rather than "skipped"), and does not require a function body at all. Using test.skip with () => {} bodies implies the test was once implemented and is now temporarily disabled, which is semantically misleading here. Callers relying on test report counts to track migration progress will see these counted as "skipped" instead of "todo," making it harder to distinguish legitimately-skipped active tests from not-yet-written stubs.
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!
| /* | ||
| 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. | ||
| */ | ||
|
|
||
| // These tests require real At.js 1.x/2.x pages and live Target endpoints; skipped | ||
| // until suitable end-to-end infrastructure is available. | ||
|
|
||
| import { test, describe } from "../../helpers/testsSetup/extend.js"; |
There was a problem hiding this comment.
The eight TestCafe functional tests listed in the PR description (C8085773.js through C8085780.js) still exist in packages/browser/test/functional/specs/Migration/. If the migration plan calls for eventually deleting the old suite once the integration counterparts are implemented, the co-existence of both the functional originals and the new (empty) integration stubs could cause confusion about what coverage is actually active. Is removing the TestCafe files planned for this PR or a later step in the stack? Are the original TestCafe functional files (C8085773.js–C8085780.js) intended to be deleted in this PR or in a later PR in the stacked series?
carterworks
force-pushed
the
migrate-integration/18-rules-engine
branch
from
c86ea05 to
4793858
Compare
carterworks
force-pushed
the
migrate-integration/19-migration
branch
from
e439ac3 to
bd0c3d7
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
force-pushed
the
migrate-integration/18-rules-engine
branch
from
4793858 to
90fa13b
Compare
carterworks
force-pushed
the
migrate-integration/19-migration
branch
from
bd0c3d7 to
f97659c
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/18-rules-engine
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/19-migration
branch
4 times, most recently
from
7b5ab06 to
e93cf01
Compare
carterworks
force-pushed
the
migrate-integration/19-migration
branch
from
e93cf01 to
ce94ec2
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
carterworks
force-pushed
the
migrate-integration/19-migration
branch
from
ce94ec2 to
7601646
Compare
|
Stub PR — structurally sound, parity deferred. Approve. Checks
C-number → functional file → skip reason
Findings
Reviewer: automated stub-PR review. Parity, mock coverage, and timing checks are deferred and not applicable here. |
1 participant
Changed Packages
Description
All 8 migration tests preserved as test.skip with detailed explanations. They require At.js 1.x/2.x pages, real Target edge, and cross-page navigation — fundamentally incompatible with the Vitest+Playwright 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/Migration/C8085773.jspackages/browser/test/functional/specs/Migration/C8085774.jspackages/browser/test/functional/specs/Migration/C8085775.jspackages/browser/test/functional/specs/Migration/C8085776.jspackages/browser/test/functional/specs/Migration/C8085777.jspackages/browser/test/functional/specs/Migration/C8085778.jspackages/browser/test/functional/specs/Migration/C8085779.jspackages/browser/test/functional/specs/Migration/C8085780.jsTypes of changes
Checklist:
Stack