fix: wire npm run test:server into CI to establish green test baseline - #370
Merged
Merged
Conversation
Agent-Logs-Url: https://github.com/driver727-pixel/Punch-Skater/sessions/5d2e9a58-4d81-4865-88c8-e94620db61af Co-authored-by: driver727-pixel <269849721+driver727-pixel@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix server test failure due to missing firebase-admin configuration
fix: wire Apr 24, 2026
npm run test:server into CI to establish green test baseline
driver727-pixel
marked this pull request as ready for review
April 24, 2026 05:51
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
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.
PR #353 noted "26/27 server tests pass" — the failing test was
server/test/firebaseAdmin.test.jsfailing at module load withERR_MODULE_NOT_FOUNDin environments missingfirebase-admin. The deeper problem:ci.ymlnever rannpm run test:serverat all, so every PR's "tests pass" claim was unenforced.Changes
.github/workflows/ci.yml— addedTeststep (npm run test:server) betweenLintandBuildNotes
The test file itself was already correct —
firebaseAdmin.test.jsuses full dependency injection to mock allfirebase-adminfunctions, so no credentials are needed at runtime.firebase-adminis a properdependency(notdevDependency), sonpm installalways brings it in. All 30 server tests pass.No production code modified.
Original prompt
Background
PR #353 (Sprint 0 Foundation) noted in its testing summary:
This pre-existing failure is rotting the CI signal — every new PR's "tests pass" claim has to carry an asterisk. We want a green
npm run test:serverbaseline so future PRs have a clean signal to compare against.Task
npm installandnpm run test:serverlocally in the agent environment to identify the failing test.firebase-adminnot being available / configured in CI environments whereFIREBASE_SERVICE_ACCOUNT_JSONand the related env vars are unset.firebase-adminis unconfigured (mirroring the pattern used in PR fix: image generation 503 when Firebase Admin is not configured #306 for image routes and the existing admin/battle route registrations), update the test to assert that graceful-degradation behavior.firebase-admin, mockfirebase-admin(or theadminAuthexport) at the test boundary so the test is self-contained and does not depend on env vars.TODOcomment explaining the gap.Out of scope
Validation
npm run test:serverexits 0 with all 27 tests passing.npm run lintis clean.The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Take a look at the total repo, and help me figure out what to do next?
Assistant: # What to do next — a triage + roadmap
Here's my read on the current state of the repo and a concrete action plan, ordered by impact and urgency.
Snapshot
README.mdThe picture
You're in a rapid-iteration polish phase on the existing forge (board art, card layout, weather rate limits) while a coordinated "engagement loop" expansion (streaks, missions, weekly heat, battle pass, charge-up) is staged but not landed. That tension is the thing to resolve next.
Recommended order of work
1. Triage open PRs first (do this today)
You have 9 open PRs. Decide each: