chore: remove web-core json tests#2362
Conversation
|
📝 WalkthroughWalkthroughThis pull request establishes Lighthouse CI testing infrastructure and expands E2E test coverage for the web-core-e2e package. Changes include a new Lighthouse configuration file, test case definitions, GitHub Actions workflow updates, build configuration enhancements with virtual files middleware, and multiple new E2E test entry points covering browser configuration, simultaneous event handlers, and query selector functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 14.56%
Performance Changes
Comparing Footnotes
|
Web Explorer#8348 Bundle Size — 521.22KiB (0%).7088ed1(current) vs 518c310 main#8343(baseline) Bundle metrics
Bundle size by type
|
| Current #8348 |
Baseline #8343 |
|
|---|---|---|
317.23KiB |
317.23KiB |
|
202KiB |
202KiB |
|
1.99KiB |
1.99KiB |
Bundle analysis report Branch PupilTong:p/hw/remove-old-test Project dashboard
Generated by RelativeCI Documentation Report issue
…ntation (#2322) Plan: - [x] pass all old web tests - [x] fix REPL tests - [ ] introduce a benchmark result #2362 - [x] document breaking change and migration methods - [ ] remove all old tests #2362 <!-- Thank you for submitting a pull request! We appreciate the time and effort you have invested in making these changes. Please ensure that you provide enough information to allow others to review your pull request. Upon submission, your pull request will be automatically assigned with reviewers. If you want to learn more about contributing to this project, please visit: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md. --> <!-- The AI summary below will be auto-generated - feel free to replace it with your own. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Unified WASM-based core engine as primary implementation * New `__QuerySelector` and `__InvokeUIMethod` APIs * `<lynx-view>` gains `browser-config` attribute for pixelRatio/viewport control * Better CSS handling: keyframes, scoped/root/type selector alignment * **Bug Fixes** * Backward-compatibility fixes for legacy JSON templates and lazy-loading execution mode * **Breaking Changes** * Removed `thread-strategy`, `overrideLynxTagToHTMLTagMap`, `customTemplateLoader`, `inject-head-links` * **Security** * CSP hardening: `nonce` support for iframe/srcdoc-executed scripts <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist <!--- Check and mark with an "x" --> - [x] Tests updated (or not required). - [x] Documentation updated (or not required). - [x] Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required). --------- Co-authored-by: Xuan Huang (黄玄) <5563315+Huxpro@users.noreply.github.com>
…taneous bindtap, and main thread query selector
…updated configurations and dependencies.
734f4bc to
a310ad3
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts (1)
242-249: Add a partial-overridebrowserConfigcase too.This only validates the fully overridden object. A regression in the fallback path where
pixelWidth/pixelHeightget recomputed from an overriddenpixelRatiowould still pass here. Please add a case that overrides onlypixelRatioand asserts the original width/height fallback.Based on learnings:
browserConfigparameters (pixelRatio,pixelWidth,pixelHeight) are intentionally independently controllable, and when onlypixelRatiois overridden the fallback width/height should still use the original screen dimensions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts` around lines 242 - 249, Add a new test case alongside api-createLynxView-browserConfig that exercises a partial browserConfig override: call the same helper (goto) with a browserConfig object that overrides only pixelRatio (e.g., 2) but does not supply pixelWidth/pixelHeight, then assert that the width/height locators (`#width` and `#height`) still show the original values (1234 and 5678). Use the same test scaffolding (test(..., async ({ page }, { title }) => { ... })) and same wait/expect sequence as api-createLynxView-browserConfig so the fallback path (recomputing width/height) is explicitly verified.packages/web-platform/web-core-e2e/tests/middleware.spec.ts (1)
38-41: Prefer Playwright's built-in locator assertions for auto-retrying.Using
await expect(await target.getAttribute('style'))bypasses Playwright's auto-waiting behavior, making the test reliant on explicitwait()calls that can be flaky. UsetoHaveAttributewith a regex ortoHaveCSSfor style assertions.♻️ Suggested refactor using Playwright assertions
- await wait(100); - await expect(await target.getAttribute('style')).toContain('green'); + await expect(target).toHaveAttribute('style', /green/); await target.click(); - await wait(100); - await expect(await target.getAttribute('style')).toContain('pink'); + await expect(target).toHaveAttribute('style', /pink/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core-e2e/tests/middleware.spec.ts` around lines 38 - 41, The test is bypassing Playwright's auto-retry by awaiting getAttribute and then asserting on the string; update the assertions to use Playwright locator matchers (e.g., replace the current checks on target.getAttribute('style') with expect(target).toHaveAttribute('style', /green/) and later expect(target).toHaveAttribute('style', /pink/) or use expect(target).toHaveCSS(...) for specific CSS properties), remove the explicit wait(100) and rely on Playwright to auto-wait; locate the assertions around the locator named target in middleware.spec.ts and change them to use toHaveAttribute/toHaveCSS so they auto-retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Line 80: The workflow step uses "pnpm --filter `@lynx-js/web-core-e2e` run lh ||
echo 'Lighthouse failed'" which masks non-zero exits from lhci autorun; remove
the "|| echo 'Lighthouse failed'" (or replace it with "|| exit 1") so the pnpm
command's non-zero exit propagates and Lighthouse assertion failures fail the CI
job; update the step that runs pnpm --filter `@lynx-js/web-core-e2e` run lh
accordingly.
- Around line 101-131: The Playwright job for `@lynx-js/web-tests`
("playwright-linux" job that uses ./.github/workflows/workflow-test.yml and runs
pnpm --filter `@lynx-js/web-tests` run test and coverage:ci) was commented out;
restore or replace it so Playwright specs continue gating merges — either
uncomment the existing job (including the matrix/thread/render/shard matrix,
runs-on/is-web/web-report-name/codecov-flags settings and the run script that
sets ENABLE_MULTI_THREAD/ENABLE_SSR, NODE_OPTIONS and
PLAYWRIGHT_JUNIT_OUTPUT_NAME) or add an equivalent job that executes pnpm
--filter `@lynx-js/web-tests` run test and pnpm --filter `@lynx-js/web-tests` run
coverage:ci to ensure the `@lynx-js/web-tests` suites remain executed in CI.
In `@packages/web-platform/web-core-e2e/lighthouserc.js`:
- Around line 53-59: The screenEmulation block currently sets mobile-specific
properties (mobile, width, height, deviceScaleFactor) while also setting
'disabled': true which causes Lighthouse to ignore those settings; update the
'screenEmulation' configuration in lighthouserc.js by either removing the
'disabled': true flag so the specified mobile dimensions and deviceScaleFactor
are applied, or keep 'disabled': true but add a concise comment in the
'screenEmulation' block explaining why emulation is intentionally disabled
(e.g., handled externally or testing host viewport) so the intent is clear.
In
`@packages/web-platform/web-core-e2e/tests/reactlynx/api-createLynxView-browserConfig/index.jsx`:
- Around line 1-10: The component App creates an unnecessary initial render by
initializing info with {} and then populating it in useEffect; instead
initialize state directly from SystemInfo so the first paint has the correct
values and you can remove the effect. Replace the useState call in App to seed
info with { pixelWidth: SystemInfo.pixelWidth, pixelHeight:
SystemInfo.pixelHeight } (and remove the setInfo/useEffect block), referencing
the symbols App, useState, SystemInfo, info, and setInfo to locate the code to
change.
---
Nitpick comments:
In `@packages/web-platform/web-core-e2e/tests/middleware.spec.ts`:
- Around line 38-41: The test is bypassing Playwright's auto-retry by awaiting
getAttribute and then asserting on the string; update the assertions to use
Playwright locator matchers (e.g., replace the current checks on
target.getAttribute('style') with expect(target).toHaveAttribute('style',
/green/) and later expect(target).toHaveAttribute('style', /pink/) or use
expect(target).toHaveCSS(...) for specific CSS properties), remove the explicit
wait(100) and rely on Playwright to auto-wait; locate the assertions around the
locator named target in middleware.spec.ts and change them to use
toHaveAttribute/toHaveCSS so they auto-retry.
In `@packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts`:
- Around line 242-249: Add a new test case alongside
api-createLynxView-browserConfig that exercises a partial browserConfig
override: call the same helper (goto) with a browserConfig object that overrides
only pixelRatio (e.g., 2) but does not supply pixelWidth/pixelHeight, then
assert that the width/height locators (`#width` and `#height`) still show the
original values (1234 and 5678). Use the same test scaffolding (test(..., async
({ page }, { title }) => { ... })) and same wait/expect sequence as
api-createLynxView-browserConfig so the fallback path (recomputing width/height)
is explicitly verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15ea07e8-8373-4b1f-9583-717732b11ef6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/workflows/test.ymlpackages/web-platform/web-core-e2e/lighthouserc.jspackages/web-platform/web-core-e2e/package.jsonpackages/web-platform/web-core-e2e/rsbuild.config.tspackages/web-platform/web-core-e2e/shell-project/index.tspackages/web-platform/web-core-e2e/tests/lighthouse.cases.jspackages/web-platform/web-core-e2e/tests/middleware.spec.tspackages/web-platform/web-core-e2e/tests/reactlynx.spec.tspackages/web-platform/web-core-e2e/tests/reactlynx/api-createLynxView-browserConfig/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/basic-bindtap-simultaneous/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/basic-main-query-selector/index.jsx
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
Checklist