Skip to content

Conversation

@devksingh4
Copy link
Member

@devksingh4 devksingh4 commented Nov 9, 2025

Summary by CodeRabbit

  • Chores
    • Enabled fully parallel test execution to speed up runs and remove CI-specific worker limits.
    • Switched build/install workflow to Yarn and updated lockfile handling; removed legacy lockfile tooling.
  • Tests
    • Updated an end-to-end test to interact with the correct UI control type, improving reliability.
  • Refactor
    • Adjusted server logger configuration shape without changing runtime behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Walkthrough

Enabled fully parallel Playwright test execution and removed CI worker override; switched Makefile and CI build steps from npm to Yarn (added yarn params/env, updated targets); changed lockfile-management script and removed synp from devDependencies; adjusted one E2E test to click a button; refactored Fastify logger router options shape.

Changes

Cohort / File(s) Change Summary
Test execution configuration
playwright.config.ts
Set fullyParallel to true and removed the CI-specific worker count override so worker count is determined by runtime defaults.
E2E test adjustment
tests/e2e/docs.spec.ts
Updated interaction to click a button element (role) instead of a link for the "Retrieve calendar events with" action.
Build / install workflow (Makefile)
Makefile
Replaced npm-centric variables/steps with Yarn: removed npm_install_params, added yarn_install_params and yarn_env; Docker and local targets now install/run Yarn and create dist_devel/ for local dev.
Lockfile and scripts (package.json)
package.json
Replaced lockfile-manage script to copy src/api/package.lambda.json into dist targets and copy yarn.lock into dist; removed postlockfile-manage script; removed synp from devDependencies.
Server configuration refactor
src/api/index.ts
Moved ignoreTrailingSlash and ignoreDuplicateSlashes under a new routerOptions key inside the Fastify logger config (shape change only).

Sequence Diagram(s)

sequenceDiagram
  participant Test as E2E Test
  participant Page as Browser Page
  participant App as App DOM

  Note over Test,Page: E2E step changed element role
  Test->>Page: navigate to docs page
  Page->>App: querySelector(button[...])  -- (was link before)
  App-->>Page: button found
  Test->>Page: click(button)
  Page->>App: trigger click handler -> action executed
  App-->>Page: UI updates / response
  Page-->>Test: assertions run
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify Playwright concurrency change won't introduce flaky tests or CI resource exhaustion (playwright.config.ts).
  • Confirm E2E selector change matches current DOM and test stability (tests/e2e/docs.spec.ts).
  • Validate Makefile Yarn migration across CI and Docker variants, and that yarn_env works on target platforms (Makefile).
  • Check new lockfile-manage behavior and that removal of synp has no remaining usages (package.json).
  • Ensure Fastify routerOptions placement in src/api/index.ts is compatible with the runtime Fastify version.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title only addresses one aspect (playwright config) but the changeset includes significant modifications to Makefile, package.json, Fastify logger configuration, and a test selector change. Revise the title to reflect all major changes, such as 'Migrate to Yarn, enable parallel tests, and update configurations' or break into multiple focused PRs.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dsingh14/update-playwright-ci

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 65524e6 and 56c920b.

📒 Files selected for processing (2)
  • Makefile (2 hunks)
  • src/api/index.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run Unit Tests
  • GitHub Check: Build Application
🔇 Additional comments (4)
Makefile (3)

9-10: LGTM! Yarn configuration aligns with Lambda ARM64 deployment.

The --frozen-lockfile flag ensures reproducible builds by preventing lock file updates, and the yarn_env variables correctly target AWS Lambda's ARM64 architecture with glibc. The --production flag appropriately excludes dev dependencies from the Lambda bundle.


36-36: LGTM! Docker build commands correctly install and configure Yarn.

Both Lambda and SQS Consumer builds follow the same pattern: install Yarn globally in the SAM build container, apply platform-specific environment variables, and run Yarn with frozen lockfile. This ensures consistent, reproducible builds across both deployment targets.

Also applies to: 42-42


48-48: LGTM! Defensive directory creation before dev server.

The mkdir -p command ensures the dist_devel/ directory exists before starting the development server, preventing potential errors. The -p flag safely handles the case where the directory already exists.

src/api/index.ts (1)

94-97: Configuration is correct for Fastify v5.

The routerOptions structure with ignoreTrailingSlash and ignoreDuplicateSlashes is the proper way to configure router options in Fastify v5. The code change properly implements this pattern.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2025

💰 Infracost report

Monthly estimate generated

This comment will be updated when code changes.

This keeps the hash of the output the same
@devksingh4 devksingh4 merged commit ca87a36 into main Nov 9, 2025
10 of 11 checks passed
@devksingh4 devksingh4 deleted the dsingh14/update-playwright-ci branch November 9, 2025 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants