Skip to content

Conversation

@angelayu0530
Copy link
Collaborator

@angelayu0530 angelayu0530 commented Jan 15, 2026

621

Description of changes

Checklist before review

  • I have done a thorough self-review of the PR
  • Copilot has reviewed my latest changes, and all comments have been fixed and/or closed.
  • If I have made database changes, I have made sure I followed all the db repo rules listed in the wiki here. (check if no db changes)
  • All tests have passed
  • I have successfully deployed this PR to staging
  • I have done manual QA in both dev (and staging if possible) and attached screenshots below.

Screenshots

Screenshot 2026-01-15 at 9 42 04 PM

Staging

Screenshot 2026-01-15 at 9 58 34 PM

@github-actions
Copy link
Contributor

Available PR Commands

  • /ai - Triggers all AI review commands at once
  • /review - AI review of the PR changes
  • /describe - AI-powered description of the PR
  • /improve - AI-powered suggestions
  • /deploy - Deploy to staging

See: https://github.com/tahminator/codebloom/wiki/CI-Commands

@angelayu0530
Copy link
Collaborator Author

/ai

@tahminator
Copy link
Owner

/review

@tahminator
Copy link
Owner

/describe

@tahminator
Copy link
Owner

/improve

@github-actions
Copy link
Contributor

Title

621: Fix backend tests playwright silent error


PR Type

Enhancement, Tests


Description

  • Add frontend instance for backend tests

  • Start and monitor Vite dev server

  • Integrate frontend lifecycle into test script

  • Ensure cleanup with logs on exit


Diagram Walkthrough

flowchart LR
  A["run-backend-tests.sh"] --> B["source run-frontend-instance.sh"]
  A --> C["db_startup"]
  A --> D["frontend_startup"]
  D --> E["pnpm dev (js/)"]
  E -- "PID saved" --> F["frontend_pid.txt"]
  E -- "logs" --> G["frontend.log"]
  A -- "EXIT trap" --> H["db_cleanup + frontend_cleanup"]
Loading

File Walkthrough

Relevant files
Enhancement
run-backend-tests.sh
Integrate frontend lifecycle into backend test script       

.github/scripts/run-backend-tests.sh

  • Source run-frontend-instance.sh.
  • Add EXIT trap to cleanup DB and frontend.
  • Call frontend_startup before Maven tests.
+4/-1     
run-frontend-instance.sh
New script to start and manage frontend dev server             

.github/scripts/run-frontend-instance.sh

  • Implement frontend_startup to run pnpm dev.
  • Health-check localhost:3000 with retry loop.
  • Log output to frontend.log and track PID.
  • Implement frontend_cleanup with log dump and kill.
+38/-0   

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Cleanup Robustness

The EXIT trap calls both db_cleanup and frontend_cleanup unconditionally; ensure these functions exist in all invocation contexts and that they handle missing state (e.g., absent PID/log files) without failing the trap. Validate that frontend_cleanup cannot mask original test failure exit codes.

trap 'db_cleanup; frontend_cleanup' EXIT
Port Assumption

The readiness check hardcodes http://localhost:3000 and searches for a <title> tag. Confirm that the dev server in CI runs on 3000 and renders HTML (not just JSON or 404). Consider making the port configurable via env to match CI.

for i in {1..30}; do
    if curl -s http://localhost:3000 | grep -q '<title>' ; then
        echo "Frontend is up!"
        frontend_started=true
        break
    fi
    echo "Waiting for frontend... ($i/30)"
    sleep 2
done

if [ "$frontend_started" = false ]; then
    echo "Frontend failed to start in time."
    cat frontend.log || true
    exit 1
Process Lifecycle

The script writes frontend_pid.txt and later kills that PID. Add safeguards if the PID file is missing/corrupted or if the process has forked/changed (e.g., Vite spawning children). Consider using process groups or pkill on a known command to avoid orphan processes.

    if kill $(cat frontend_pid.txt) >/dev/null 2>&1; then
        echo "Frontend process killed successfully."
    else
        echo "Frontend was not running or already stopped."
    fi
}

frontend_startup() {
    corepack enable pnpm
    cd js
    pnpm i --frozen-lockfile
    pnpm run dev >../frontend.log 2>&1 &
    echo $! >../frontend_pid.txt
    cd ..

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Working Dir Assumption

The trap calls both db_cleanup and frontend_cleanup unconditionally; confirm that sourcing run-frontend-instance.sh always succeeds and that frontend_* functions are defined in all CI contexts, and that current working directory matches expectations for paths used by frontend_startup/frontend_cleanup.

source "$DIR/run-frontend-instance.sh"


trap 'db_cleanup; frontend_cleanup' EXIT

java -version
Process Cleanup Robustness

Killing by PID file without checking file existence or process ownership can fail or target the wrong process if PID is reused; add checks for file existence, numeric PID, and that the process matches the expected command before kill.

if kill $(cat frontend_pid.txt) >/dev/null 2>&1; then
    echo "Frontend process killed successfully."
else
    echo "Frontend was not running or already stopped."
fi
Port/Health Check

Health check assumes dev server on localhost:5173 returns an HTML title; this may differ across environments or frameworks; consider configurable port and a more deterministic endpoint or readiness signal, plus a longer/more adaptive timeout.

frontend_started=false
for i in {1..30}; do
    if curl -s http://localhost:3000 | grep -q '<title>' ; then
        echo "Frontend is up!"
        frontend_started=true
        break
    fi
    echo "Waiting for frontend... ($i/30)"
    sleep 2
done

if [ "$frontend_started" = false ]; then
    echo "Frontend failed to start in time."
    cat frontend.log || true
    exit 1
fi

@github-actions
Copy link
Contributor

Title

621: Fix backend tests playwright silent error


PR Type

Enhancement, Tests


Description

  • Add frontend instance startup script

  • Integrate frontend lifecycle into backend tests

  • Stream logs and ensure graceful cleanup

  • Health-check frontend before running tests


Diagram Walkthrough

flowchart LR
  A["run-backend-tests.sh"] --> B["source run-frontend-instance.sh"]
  A --> C["db_startup"]
  B --> D["frontend_startup"]
  D -- "pnpm dev & log" --> E["frontend.log / frontend_pid.txt"]
  A -- "on EXIT" --> F["db_cleanup + frontend_cleanup"]
Loading

File Walkthrough

Relevant files
Enhancement
run-backend-tests.sh
Integrate frontend lifecycle into backend test runner       

.github/scripts/run-backend-tests.sh

  • Source run-frontend-instance.sh utilities
  • Add frontend_startup before Maven tests
  • Extend trap to run frontend_cleanup
+4/-1     
Tests
run-frontend-instance.sh
New script to start and cleanup frontend instance               

.github/scripts/run-frontend-instance.sh

  • Add script to start frontend with pnpm dev
  • Implement health check on localhost:3000
  • Log output and record PID for cleanup
  • Provide robust cleanup with log dump
+38/-0   

@github-actions
Copy link
Contributor

Overall Project 75.91% 🍏

There is no coverage information present for the Files changed

@github-actions
Copy link
Contributor

Commit Validation Failed

The following commits do not start with the required Notion ID 621:

reverting changes

Please rebase and update your commit messages.
All messages should be of the following format: 621: Example commit

@angelayu0530 angelayu0530 force-pushed the 621 branch 2 times, most recently from 948c468 to f083ca0 Compare January 15, 2026 22:23
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@angelayu0530 angelayu0530 force-pushed the 621 branch 7 times, most recently from bf2b510 to d51b293 Compare January 16, 2026 02:29
slight bug

types error fixes

reverting changes

Fixing where frontend runs before

Type fixes

bug fixes

react email fixes
@angelayu0530
Copy link
Collaborator Author

/deploy

@github-actions
Copy link
Contributor

The command to deploy to staging for the commit bc91764 has been triggered. View action run

@github-actions
Copy link
Contributor

Staging deployment failed for commit bc91764

View run

@github-actions
Copy link
Contributor

Staging deployment succeeded for commit bc91764

View run

@angelayu0530 angelayu0530 merged commit bda36de into main Jan 16, 2026
22 checks passed
@angelayu0530 angelayu0530 deleted the 621 branch January 16, 2026 22:56
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.

4 participants