Develop to test code merge - #1996
Merged
Merged
Conversation
* feat: Added playwright and config for e2e testing, converted testcase to scripts and dry running scripts generated with claude code * feat: Updated the constants, helpers and test cases to handle the initial 16 test cases * feat: Fm-937 Improve Gameplay Drag-and-Drop Automation Coverage in Feed the Monster & Fm-939 Pause Gameplay When Assessment Is Triggered * feat: FM-939 FM-940 FM-941 FM-942 FM-943 Fm-944 QA automation * feat: FM-909 QA automation for FTM to Assessment survey * Delete CLAUDE.md * chore: Gitignore updates * feat: comments addressed * chore: Gitignore updates * feat: comments resolved with orchestration and added spec file for testcases * fix: npm run test issue fixed
…ment Trigger and Complete Level Progression (#1988) * feat: Added playwright and config for e2e testing, converted testcase to scripts and dry running scripts generated with claude code * feat: Updated the constants, helpers and test cases to handle the initial 16 test cases * feat: Fm-937 Improve Gameplay Drag-and-Drop Automation Coverage in Feed the Monster & Fm-939 Pause Gameplay When Assessment Is Triggered * feat: FM-939 FM-940 FM-941 FM-942 FM-943 Fm-944 QA automation * feat: FM-909 QA automation for FTM to Assessment survey * Delete CLAUDE.md * chore: Gitignore updates * feat: comments addressed * chore: Gitignore updates * feat: comments resolved with orchestration and added spec file for testcases * fix: npm run test issue fixed * feat: FM-945 Refine Gameplay E2E Automation to Support Dynamic Assessment Trigger and Complete Level Progressio * fix: mock helper changes
refactor: update Urdu language curriculum sequence and increment minor version.
… to scripts and dry running scripts generated with claude code (#1989) * feat: Added playwright and config for e2e testing, converted testcase to scripts and dry running scripts generated with claude code * feat: Updated the constants, helpers and test cases to handle the initial 16 test cases * feat: Fm-937 Improve Gameplay Drag-and-Drop Automation Coverage in Feed the Monster & Fm-939 Pause Gameplay When Assessment Is Triggered * feat: FM-939 FM-940 FM-941 FM-942 FM-943 Fm-944 QA automation * feat: FM-909 QA automation for FTM to Assessment survey * Delete CLAUDE.md * chore: Gitignore updates * feat: comments addressed * chore: Gitignore updates * feat: comments resolved with orchestration and added spec file for testcases * fix: npm run test issue fixed * feat: FM-945 Refine Gameplay E2E Automation to Support Dynamic Assessment Trigger and Complete Level Progressio * fix: mock helper changes * feat: Fm-946 Refine QA Automation for Assessment Survey Gameplay Flow * feat: FM-946 Refine QA Automation for Assessment Survey Gameplay Flow
Deploying MR-67 From Test to Prod
chore: bump to 1.6.0
Deploying MR-145 from Test to Production
chore: bump version to 1.6.1
…#1992) * feat: FM-949 Added e2e playwright automation to circle ci/cd pipeline * fix: handling artifact storage using s3 * fix: Fixed the build issue and final path changes * fix: Fixed the build issue and final path changes
Contributor
📝 WalkthroughWalkthroughThe PR adds a Playwright E2E framework covering launch through level completion, exposes non-production game internals for automation, adjusts assessment and mini-game timing, and requires E2E tests to pass before deployment. ChangesFeedTheMonster E2E coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
e2e/helpers/game-state-helpers.ts (2)
351-366: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
waitForPositiveFeedbackdoes not check positivity.The predicate only requires non-empty text, so it resolves on negative feedback too. Every caller re-asserts the phrase afterwards, so consider renaming to
waitForFeedbackText(or moving the positive-phrase list here) to avoid future misuse.🤖 Prompt for 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. In `@e2e/helpers/game-state-helpers.ts` around lines 351 - 366, Rename waitForPositiveFeedback to waitForFeedbackText, including all callers and references, because its predicate only waits for non-empty feedback text and does not validate positivity. Preserve the existing selector, timeout behavior, and predicate logic.
533-546: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
speedUpAssessmentTimerrewrites every long one-shot timer.Any unrelated non-loop timer with
remaining > 1000is also collapsed totargetMs, which can fire game logic (evolution, audio, level-end) far earlier than intended and produce hard-to-diagnose flakes. Consider narrowing by the scheduled callback identity or by an expectedremainingwindow near 5500 ms.🤖 Prompt for 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. In `@e2e/helpers/game-state-helpers.ts` around lines 533 - 546, Update speedUpAssessmentTimer so it targets only the assessment timer rather than every non-loop timer with remaining > 1000. Narrow the condition using the scheduled callback identity or a validated remaining-time range near 5500 ms, while preserving the targetMs override for the matched timer.e2e/tests/isolated/tc-009-013-assessment.spec.ts (2)
348-371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated puzzle-advance polling block.
The same
waitForFunction(scene resolution →currentPuzzleIndex >= expected→correctStonePos/stonesHasLoaded) appears at lines 222-241, 351-371, 669-689 and 740-760. A single helper (an extendedwaitForPuzzleAdvance) would keep the four copies from drifting.🤖 Prompt for 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. In `@e2e/tests/isolated/tc-009-013-assessment.spec.ts` around lines 348 - 371, Extract the repeated puzzle-advance polling logic into a shared helper, such as an extended waitForPuzzleAdvance, and replace the duplicate blocks at all four call sites. Preserve the existing scene resolution, currentPuzzleIndex threshold, correctStonePos/stonesHasLoaded condition, timeout, and swallowed-timeout behavior.
89-122: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePixel fallback contradicts the stated rationale.
The comment explains that opaque pixels appear at
frame = 0and are therefore not a valid readiness signal, yet the fallback branch resolves exactly on pixel presence — so whenstoneHandleris unreachable the drop can still fire mid-animation and be ignored. Consider failing loudly instead of falling back.🤖 Prompt for 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. In `@e2e/tests/isolated/tc-009-013-assessment.spec.ts` around lines 89 - 122, The waitForStonesToRender readiness check must not use canvas pixel presence as a fallback, since pixels appear before stone animation completes. Remove the pixel-based fallback in waitForStonesToRender and fail clearly when stoneHandler is unavailable, while preserving the stonesHasLoaded === true readiness condition.e2e/tests/ftm-assessment-survey-flow.spec.ts (1)
54-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose the browser context, and guard
pageinafterAll.
ctxis never closed, so the context (and its service workers/storage) leaks for the whole run. Also, ifbeforeAllfails beforepageis assigned,afterAllthrows aTypeErrorthat masks the original failure.♻️ Proposed fix
let page: Page; + let ctx: import('`@playwright/test`').BrowserContext; ... test.beforeAll(async ({ browser }) => { - const ctx = await browser.newContext(); + ctx = await browser.newContext(); page = await ctx.newPage(); ... test.afterAll(async () => { - await page.close(); + await ctx?.close(); });🤖 Prompt for 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. In `@e2e/tests/ftm-assessment-survey-flow.spec.ts` around lines 54 - 66, Update the test.beforeAll/afterAll lifecycle in the FTM assessment survey flow to retain the created browser context, close it during cleanup, and guard page before calling page.close(). Ensure cleanup remains safe when setup fails before page is assigned, without masking the original failure.e2e/tests/isolated/tc-004-005-level-selection.spec.ts (1)
46-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFixed
waitForTimeout(1500)after navigation clicks. Both specs sleep a fixed 1.5 s after a click even though the very next step already waits on a locator withTimeouts.sceneTransition; the sleeps only add runtime and hide real transition timing.
e2e/tests/isolated/tc-004-005-level-selection.spec.ts#L46-L49: drop the sleep after the level-2 click and rely on TC_005'smainCanvasvisibility wait.e2e/tests/isolated/tc-002-003-start-screen.spec.ts#L69-L78: drop the sleep after the start-screen click and rely on the followingLevelSelectionPage.SELECTORvisibility wait.🤖 Prompt for 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. In `@e2e/tests/isolated/tc-004-005-level-selection.spec.ts` around lines 46 - 49, Remove the fixed waitForTimeout(1500) after the level-2 click in e2e/tests/isolated/tc-004-005-level-selection.spec.ts lines 46-49, relying on TC_005’s mainCanvas visibility wait. Also remove the corresponding post-click sleep in e2e/tests/isolated/tc-002-003-start-screen.spec.ts lines 69-78 and rely on the following LevelSelectionPage.SELECTOR visibility wait.
🤖 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/helpers/canvas-helpers.ts`:
- Around line 45-55: The pixel sampling logic in the canvas helper must use
backing-store coordinates rather than CSS bounding-box dimensions. Update the
page.evaluate callback to calculate x and y from canvas.width and canvas.height
using the provided ratios, clamp both coordinates to valid bitmap bounds, and
then pass the clamped values to getImageData.
In `@e2e/helpers/game-state-helpers.ts`:
- Around line 384-392: Update the CORRECT_STONE_POSITION subscription callback
around unsub in the helper so unsubscription is safe when subscribe returns no
function and when the event fires synchronously before assignment completes.
Avoid directly invoking a TDZ-prone const; defer or guard the unsubscribe using
a variable initialized before subscription, and preserve the one-shot cleanup
behavior after capture.
In `@e2e/pages/gameplay-page.ts`:
- Around line 48-50: Update the GameplayPage promptText getter to use the
dedicated prompt-text selector rather than
GameplayPage.SELECTORS.promptContainer, ensuring getPromptText() reads only the
advertised prompt text.
In `@e2e/pages/level-selection-page.ts`:
- Around line 41-43: Update levelButtonForLevel so game level 11 is not resolved
to the pagination control nextNavButton. Paginate before resolving the level
locator, or reject unsupported levels through the helper’s existing API
contract; preserve correct level-button mapping for supported levels.
In `@e2e/tests/isolated/tc-006-008-gameplay.spec.ts`:
- Around line 146-190: Validate that the bounding box returned by the canvas
locator in the “Verify stone is rendered near captured coordinates via canvas
pixel inspection” step is non-null before accessing its width and height. Add
the same explicit assertion used by the preceding step, then retain the existing
normalized coordinate calculations.
In `@e2e/tests/isolated/tc-009-013-assessment.spec.ts`:
- Line 217: Replace the hardcoded startingPuzzleIndex assignment in the test
setup with the live value returned by getCurrentPuzzleIndexFromManager. Ensure
downstream calculations such as remaining and alreadyTriggered use this
retrieved index rather than assuming TC_008 completed puzzle 1.
In `@e2e/tests/isolated/tc-014-015-mini-game.spec.ts`:
- Around line 73-99: Bound the polling loop in the page.evaluate callback within
the “Auto-click 5 stones” test step by adding a deadline or maximum-attempt
counter. When the cap is reached without finding active stones, resolve early so
the step cannot hang indefinitely, while preserving the existing immediate start
and successful five-click behavior.
In `@playwright.config.ts`:
- Line 11: Update the Playwright configuration’s trace setting alongside retries
to use retain-on-failure, ensuring failed CI tests preserve traces while keeping
retries set to zero.
In `@src/scenes/gameplay-scene/gameplay-scene.ts`:
- Line 86: Update the mini-game completion flow around isMiniGamePaused so it is
set only when the handler actually invokes pauseGamePlay(). Only clear
isPauseButtonClicked and call resumeGame() when that flag indicates this handler
paused the game, preserving any pre-existing manual pause.
---
Nitpick comments:
In `@e2e/helpers/game-state-helpers.ts`:
- Around line 351-366: Rename waitForPositiveFeedback to waitForFeedbackText,
including all callers and references, because its predicate only waits for
non-empty feedback text and does not validate positivity. Preserve the existing
selector, timeout behavior, and predicate logic.
- Around line 533-546: Update speedUpAssessmentTimer so it targets only the
assessment timer rather than every non-loop timer with remaining > 1000. Narrow
the condition using the scheduled callback identity or a validated
remaining-time range near 5500 ms, while preserving the targetMs override for
the matched timer.
In `@e2e/tests/ftm-assessment-survey-flow.spec.ts`:
- Around line 54-66: Update the test.beforeAll/afterAll lifecycle in the FTM
assessment survey flow to retain the created browser context, close it during
cleanup, and guard page before calling page.close(). Ensure cleanup remains safe
when setup fails before page is assigned, without masking the original failure.
In `@e2e/tests/isolated/tc-004-005-level-selection.spec.ts`:
- Around line 46-49: Remove the fixed waitForTimeout(1500) after the level-2
click in e2e/tests/isolated/tc-004-005-level-selection.spec.ts lines 46-49,
relying on TC_005’s mainCanvas visibility wait. Also remove the corresponding
post-click sleep in e2e/tests/isolated/tc-002-003-start-screen.spec.ts lines
69-78 and rely on the following LevelSelectionPage.SELECTOR visibility wait.
In `@e2e/tests/isolated/tc-009-013-assessment.spec.ts`:
- Around line 348-371: Extract the repeated puzzle-advance polling logic into a
shared helper, such as an extended waitForPuzzleAdvance, and replace the
duplicate blocks at all four call sites. Preserve the existing scene resolution,
currentPuzzleIndex threshold, correctStonePos/stonesHasLoaded condition,
timeout, and swallowed-timeout behavior.
- Around line 89-122: The waitForStonesToRender readiness check must not use
canvas pixel presence as a fallback, since pixels appear before stone animation
completes. Remove the pixel-based fallback in waitForStonesToRender and fail
clearly when stoneHandler is unavailable, while preserving the stonesHasLoaded
=== true readiness condition.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 862aeb94-0c09-4fc5-8827-948a536648e0
⛔ Files ignored due to path filters (70)
CHANGELOG.mdis excluded by!**/*.mde2e/tests/README.mdis excluded by!**/*.mde2e/tsconfig.jsonis excluded by!**/*.jsonlang/urdu/audios/amazing.mp3is excluded by!**/*.mp3lang/urdu/audios/fantastic.mp3is excluded by!**/*.mp3lang/urdu/audios/great.mp3is excluded by!**/*.mp3lang/urdu/audios/ا.mp3is excluded by!**/*.mp3lang/urdu/audios/ب.mp3is excluded by!**/*.mp3lang/urdu/audios/بـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ت.mp3is excluded by!**/*.mp3lang/urdu/audios/تـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ث.mp3is excluded by!**/*.mp3lang/urdu/audios/ثـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ج.mp3is excluded by!**/*.mp3lang/urdu/audios/جـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ح.mp3is excluded by!**/*.mp3lang/urdu/audios/حـ.mp3is excluded by!**/*.mp3lang/urdu/audios/خ.mp3is excluded by!**/*.mp3lang/urdu/audios/خـ.mp3is excluded by!**/*.mp3lang/urdu/audios/د.mp3is excluded by!**/*.mp3lang/urdu/audios/ذ.mp3is excluded by!**/*.mp3lang/urdu/audios/ر.mp3is excluded by!**/*.mp3lang/urdu/audios/ز.mp3is excluded by!**/*.mp3lang/urdu/audios/س.mp3is excluded by!**/*.mp3lang/urdu/audios/سـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ش.mp3is excluded by!**/*.mp3lang/urdu/audios/شـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ص.mp3is excluded by!**/*.mp3lang/urdu/audios/صـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ض.mp3is excluded by!**/*.mp3lang/urdu/audios/ضـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ط.mp3is excluded by!**/*.mp3lang/urdu/audios/ظ.mp3is excluded by!**/*.mp3lang/urdu/audios/ع.mp3is excluded by!**/*.mp3lang/urdu/audios/عـ.mp3is excluded by!**/*.mp3lang/urdu/audios/غ.mp3is excluded by!**/*.mp3lang/urdu/audios/غـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ف.mp3is excluded by!**/*.mp3lang/urdu/audios/فـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ق.mp3is excluded by!**/*.mp3lang/urdu/audios/قـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ل.mp3is excluded by!**/*.mp3lang/urdu/audios/لـ.mp3is excluded by!**/*.mp3lang/urdu/audios/م.mp3is excluded by!**/*.mp3lang/urdu/audios/مـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ن.mp3is excluded by!**/*.mp3lang/urdu/audios/نـ.mp3is excluded by!**/*.mp3lang/urdu/audios/و.mp3is excluded by!**/*.mp3lang/urdu/audios/ٹ.mp3is excluded by!**/*.mp3lang/urdu/audios/پ.mp3is excluded by!**/*.mp3lang/urdu/audios/پـ.mp3is excluded by!**/*.mp3lang/urdu/audios/چ.mp3is excluded by!**/*.mp3lang/urdu/audios/چـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ڈ.mp3is excluded by!**/*.mp3lang/urdu/audios/ڑ.mp3is excluded by!**/*.mp3lang/urdu/audios/ژ.mp3is excluded by!**/*.mp3lang/urdu/audios/ک.mp3is excluded by!**/*.mp3lang/urdu/audios/گ.mp3is excluded by!**/*.mp3lang/urdu/audios/گـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ں.mp3is excluded by!**/*.mp3lang/urdu/audios/ھ.mp3is excluded by!**/*.mp3lang/urdu/audios/ہ.mp3is excluded by!**/*.mp3lang/urdu/audios/ہـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ی.mp3is excluded by!**/*.mp3lang/urdu/audios/یـ.mp3is excluded by!**/*.mp3lang/urdu/audios/ے.mp3is excluded by!**/*.mp3lang/urdu/ftm_urdu.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.json,!package-lock.jsonpackage.jsonis excluded by!**/*.jsontsconfig.jsonis excluded by!**/*.json
📒 Files selected for processing (37)
.circleci/config.yml.gitignoree2e/constants/index.tse2e/constants/selectors.tse2e/constants/timeouts.tse2e/constants/urls.tse2e/fixtures/game-fixtures.tse2e/helpers/canvas-helpers.tse2e/helpers/game-state-helpers.tse2e/helpers/index.tse2e/helpers/mock-helpers.tse2e/helpers/navigation-helpers.tse2e/pages/base-page.tse2e/pages/gameplay-page.tse2e/pages/index.tse2e/pages/level-end-page.tse2e/pages/level-selection-page.tse2e/pages/loading-page.tse2e/pages/pause-popup-page.tse2e/pages/start-page.tse2e/tests/ftm-assessment-survey-flow.spec.tse2e/tests/isolated/tc-001-app-launch.spec.tse2e/tests/isolated/tc-002-003-start-screen.spec.tse2e/tests/isolated/tc-004-005-level-selection.spec.tse2e/tests/isolated/tc-006-008-gameplay.spec.tse2e/tests/isolated/tc-009-013-assessment.spec.tse2e/tests/isolated/tc-014-015-mini-game.spec.tse2e/tests/isolated/tc-016-level-completion.spec.tsplaywright.config.tspublic/index.htmlsrc/components/stone-handler/stone-handler.tssrc/feedTheMonster.tssrc/miniGame/miniGames/treasureChest/treasureChestAnimation.tssrc/scenes/gameplay-scene/gameplay-flow-manager.spec.tssrc/scenes/gameplay-scene/gameplay-flow-manager.tssrc/scenes/gameplay-scene/gameplay-scene.tssrc/scenes/progress-scene/progress-scene.ts
amitsinghsutara
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Summary by CodeRabbit