test(e2e): maps-coreと重複する既存E2Eを削除しwrapper-onlyに縮約#497
Conversation
maps-core E2E (display/feature) と重複する embed の既存 E2E を削除し、 embed E2E を wrapper-only に縮約する。 - 削除: map.spec.ts / external-style.spec.ts / sample.spec.ts (maps-core の map.spec / external-style.spec / attribution.spec でカバー済み。所在は #495 のコメント参照) - 削除(孤立fixture): nocontrol.html / external-style.html / data-key.html - basic.spec.ts: 1.1 を統合スモーク1本に slim 化 (script→auto-render→canvas) - basic.spec.ts 1.2 (デフォルト中心0,0) は parse-atts 既定値由来の embed 固有挙動なので data-attributes.spec.ts へ移設 Refs #495, #411
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough複数の既存 E2E テストファイルおよび E2E 用 HTML ドキュメントを削除し、embed 固有の統合スモーク（bundle → auto-render → canvas 表示確認）と data-* 属性未指定時の既定値検証に再構成しました。 ChangesE2E テスト体系の再構成：maps-core 重複排除と embed 固有スモークへの集約
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/data-attributes.spec.ts`:
- Around line 51-52: The test uses toBeCloseTo(0, 0) for center.lat and
center.lng which is too coarse; update both assertions to a stricter precision
(e.g. toBeCloseTo(0, 5)) so the latitude/longitude default-forwarding
regressions are detected—modify the two expectations referencing center.lat and
center.lng in e2e/data-attributes.spec.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c8ab993-1979-45ab-9f5b-d7ece46ca6be
📒 Files selected for processing (8)
docs/e2e/data-key.htmldocs/e2e/external-style.htmldocs/e2e/nocontrol.htmle2e/basic.spec.tse2e/data-attributes.spec.tse2e/external-style.spec.tse2e/map.spec.tse2e/sample.spec.ts
💤 Files with no reviewable changes (6)
- docs/e2e/external-style.html
- docs/e2e/data-key.html
- e2e/external-style.spec.ts
- docs/e2e/nocontrol.html
- e2e/map.spec.ts
- e2e/sample.spec.ts
toBeCloseTo(0, 0) は許容±0.5 で粗く、既定転送のリグレッションを取りこぼす。 既定値は厳密に 0 なので精度 5 に厳密化。 Refs #495
yuiseki
left a comment
yuiseki
left a comment
There was a problem hiding this comment.
#496 に続く削除フェーズ、ありがとうございます。#495 の所在確認コメントを根拠に1件ずつ maps-core の対応テストへ紐づけてあり、削除の判断過程が追えるのがとても良いです。
特に良かった点:
- 旧 1.2（デフォルト中心 0,0）を単純削除せず
data-attributes.spec.tsの「data-* 無し → 既定値」へ移設し、さらに精度をtoBeCloseTo(0, 0)→toBeCloseTo(0, 5)に厳格化しているところ。embed 固有（parse-atts の既定値由来）の挙動を wrapper-only 側で正しく残せています。 - 削除した spec（map の tile 描画 / attribution / console error）が maps-core 側のどのテストに対応するかを表で明示している点。
- sample.spec が「充実したら削除してよい」と自己申告していた PR0 sanity なので、削除が筋に合っています。
1点、非ブロッキングの指摘です（ご判断にお任せします）:
external-style.spec.ts の削除で、data-style に外部スタイル URL を渡したときのフォワーディング/解決パスの E2E カバレッジが無くなります。
- 判定ロジック
isGeoloniaStyleCheck自体はkeyring.test.tsでユニット担保済み data-style→styleオプションの転送はparse-atts.tsの...container.datasetでジェネリックに行われる（lat/lng と同機構）
なので低リスクではあるのですが、parse-atts.test.ts は既定の geolonia/basic-v2 しか検証しておらず、「data-style=外部 URL → isGeoloniaStyle=false で外部スタイルとして転送される」という embed 固有の組み合わせを直接押さえたテストは、この PR 後どこにも残りません。これは embed の責任分担「wrapper = data-* フォワーディング」にまさに該当する箇所なので、data-attributes.spec.ts に lat/lng と同様の data-style フォワーディングケースを1件足すか、parse-atts.test.ts に外部 style のアサーションを足しておくと、削除フェーズで取りこぼしが無くなると思います。本 PR で対応しても、別 issue にしても構いません。
削除自体は妥当で CI もグリーンなので approve します 🙌
smellman
left a comment
smellman
left a comment
There was a problem hiding this comment.
LGTM!
一旦取り入れて問題無いと思います。
3 participants
概要
#495 の実装。maps-core E2E（表示・機能）と重複する embed の既存 E2E を削除し、embed E2E を wrapper-only + 統合スモーク1本 に縮約する。#491（maps-core 置き換え）の前準備であり、#496（embed 固有 E2E の追加）に続く削除フェーズ。
所在確認の根拠は #495 のコメント参照: #495 (comment)
変更内容
削除した spec（maps-core でカバー済み）
e2e/map.spec.ts（2.1.1/2.1.2）map.spec.ts"render a canvas element" / "render tile content in upper half"e2e/external-style.spec.ts（×2）external-style.spec.ts"render a map with external style URL" / "fetch the external style URL"e2e/sample.spec.ts（helper sanity ×2）helper.ts本体は #496 specs が使用するため残す削除した孤立フィクスチャ（削除 spec 専用で他から未参照）
docs/e2e/nocontrol.htmldocs/e2e/external-style.htmldocs/e2e/data-key.htmlbasic.spec.tsの縮約map.spec"should not produce application-level console errors" と重複 → 削除attribution.specと重複 → 削除parse-atts既定値由来の embed 固有挙動で maps-core に対応なし →data-attributes.spec.tsの「data-* 無し → 既定値」ケースへ移設残る embed E2E（全て wrapper-only + smoke）
basic(smoke) /globals/auto-render/data-attributes/lazy-loading/cleanup/plugin/iframe/pmtiles結果
docs/e2e/sample-basic-style.jsonは helper のモックが揃える基準参照（PR Reject from iframe #81/test(e2e): backport maps-core PR #81 — robust minimal style fixture #492 由来、実行時未ロード）であり本 PR の削除対象ではないため温存メモ
Closes #495
Part of #411
Summary by CodeRabbit
Tests
Documentation