Skip to content

Feat: Playwright /docker + vite watcher on html + css #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
schoettler wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Feat: Playwright /docker + vite watcher on html + css #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5ab6a26
chore: update package.json dependencies
Jul 26, 2025
13d164f
feat: add hot-reload watcher for frontend TypeScript compilation
Jul 26, 2025
747a4a6
chore: update chokidar dependency to version 3.6.0
Jul 26, 2025
153cfa9
feat: add custom frontend watcher for HTML, CSS, and TypeScript files
Jul 26, 2025
c15e1f2
refactor: replace custom watcher with Vite for frontend development
Jul 26, 2025
d496bd3
feat: add Docker run script for fullstack challenge project
Jul 26, 2025
03bf550
chore: update run.sh to launch Docker with Playwright and start services
Jul 26, 2025
c4f9461
chore: remove run.sh script with Docker and service launch commands
Jul 26, 2025
95382c1
feat: add run.sh script for Docker Playwright development and testing
Jul 26, 2025
0ce8774
chore: update run script with improved Docker and service launch process
Jul 26, 2025
5701088
fix: run dev services in background with sleep for startup
Jul 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions bin/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash

set -e
# Launch Playwright using Docker
docker run -d --rm \
--name "e2e-fullstack-challenge" \
-v "$PWD":/app \
-w /app \
-p 3000:3000 \
-p 3001:3001 \
mcr.microsoft.com/playwright:v1.52.0-noble
Comment on lines +5 to +11
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Docker container setup seems disconnected from test execution.

The script creates a Docker container but then runs npm run test:ui on the host, not inside the container. This appears to contradict the PR objectives of running Playwright tests "inside the container."

Consider clarifying the intended workflow. If tests should run inside the container:

-docker run -d --rm \
+docker run -d --rm \
    --name "e2e-fullstack-challenge" \
    -v "$PWD":/app \
    -w /app \
    -p 3000:3000 \
    -p 3001:3001 \
    mcr.microsoft.com/playwright:v1.52.0-noble
+   tail -f /dev/null  # Keep container running

Then execute tests inside the container:

-npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
+npx wait-on --http-get http://localhost:3000 http://localhost:3001 && docker exec e2e-fullstack-challenge npm run test:ui

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In bin/run.sh around lines 5 to 11, the script starts a Docker container but
runs the Playwright tests on the host machine, not inside the container as
intended. To fix this, modify the script to execute the test command (e.g., npm
run test:ui) inside the running container using docker exec or run the tests as
part of the container startup command. This ensures the tests run within the
container environment as per the PR objectives.


npm run dev &
echo "Starting services..."
npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
Comment on lines +13 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing process cleanup and error handling.

The script starts background processes but doesn't clean them up on exit or failure, which could leave orphaned processes running.

Add proper cleanup and error handling:

+#!/bin/bash
+
+set -e
+
+# Cleanup function
+cleanup() {
+    echo "Cleaning up..."
+    pkill -f "npm run dev" || true
+    docker stop e2e-fullstack-challenge || true
+    docker rm e2e-fullstack-challenge || true
+}
+
+# Set trap to cleanup on exit
+trap cleanup EXIT INT TERM
+
 # Launch Playwright using Docker
 docker run -d --rm \
    --name "e2e-fullstack-challenge" \
    -v "$PWD":/app \
    -w /app \
    -p 3000:3000 \
    -p 3001:3001 \
    mcr.microsoft.com/playwright:v1.52.0-noble
 
 npm run dev &
+DEV_PID=$!
 echo "Starting services..."
 npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
npm run dev &
echo "Starting services..."
npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
#!/bin/bash
set -e
# Cleanup function
cleanup() {
echo "Cleaning up..."
pkill -f "npm run dev" || true
docker stop e2e-fullstack-challenge || true
docker rm e2e-fullstack-challenge || true
}
# Ensure cleanup on exit, interrupt, or termination
trap cleanup EXIT INT TERM
# Launch Playwright using Docker
docker run -d --rm \
--name "e2e-fullstack-challenge" \
-v "$PWD":/app \
-w /app \
-p 3000:3000 \
-p 3001:3001 \
mcr.microsoft.com/playwright:v1.52.0-noble
# Start the dev server in the background and remember its PID
npm run dev &
DEV_PID=$!
echo "Starting services..."
npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
🤖 Prompt for AI Agents 
In bin/run.sh around lines 13 to 15, the script starts background processes
without handling cleanup or errors, risking orphaned processes. Modify the
script to capture the background process ID after starting npm run dev, add a
trap to kill this process on script exit or interruption, and include error
handling to stop the script if npx wait-on or npm run test:ui fail. This ensures
proper cleanup and prevents orphaned processes.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider timeout and better error messaging for service readiness.

The wait-on command should have a timeout and better error handling to avoid hanging indefinitely.

-npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
+echo "Waiting for services to be ready..."
+npx wait-on --timeout 60000 --http-get http://localhost:3000 http://localhost:3001
+echo "Services are ready, running tests..."
+npm run test:ui
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
npx wait-on --http-get http://localhost:3000 http://localhost:3001 && npm run test:ui
echo "Waiting for services to be ready..."
npx wait-on --timeout 60000 --http-get http://localhost:3000 http://localhost:3001
echo "Services are ready, running tests..."
npm run test:ui
🤖 Prompt for AI Agents 
In bin/run.sh at line 15, the wait-on command currently lacks a timeout and
error handling, which can cause the script to hang indefinitely if the services
do not become ready. Add a timeout option to the wait-on command to limit how
long it waits for the services, and modify the script to check the exit status
of wait-on. If wait-on fails or times out, output a clear error message and
prevent the subsequent npm run test:ui command from running.

8 changes: 5 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"scripts": {
"dev": "concurrently \"npm run dev:backend\" \"npm run dev:frontend\"",
"dev:backend": "tsx watch src/backend/server.ts",
"dev:frontend": "http-server src/frontend -p 3001 -c-1",
"dev:frontend": "vite src/frontend --port 3001 --open",
"build": "npm run build:backend && npm run build:frontend",
"build:backend": "tsc",
"build:frontend": "tsc -p tsconfig.frontend.json",
Expand All @@ -27,6 +27,8 @@
"concurrently": "^8.2.2",
"http-server": "^14.1.1",
"tsx": "^4.7.0",
"typescript": "^5.3.3"
"typescript": "^5.3.3",
"vite": "^5.4.19",
"wait-on": "^8.0.4"
}
}
}
12 changes: 12 additions & 0 deletions playwright.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
FROM mcr.microsoft.com/playwright:v1.52.0-noble

WORKDIR /app

COPY package.json package-lock.json ./

RUN npm i --frozen-lockfile;

COPY tests ./
COPY playwright.config.ts .

CMD ["npx", "playwright", "test"]
8 changes: 8 additions & 0 deletions vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineConfig } from 'vite';

export default defineConfig({
root: 'src/frontend',
server: {
port: 3001,
}
});