Skip to content

fix(api): add seed npcs.json to unblock main CI and market tests (#83) - #124

Closed
angelTomo9 wants to merge 2 commits into
Bitcoindefi:mainfrom
angelTomo9:fix-api-seed-npcs-json-1787641268762
Closed

fix(api): add seed npcs.json to unblock main CI and market tests (#83)#124
angelTomo9 wants to merge 2 commits into
Bitcoindefi:mainfrom
angelTomo9:fix-api-seed-npcs-json-1787641268762

Conversation

@angelTomo9

Copy link
Copy Markdown

Closes #83

Summary of Changes

Adds api/src/jsons/npcs.json seed fixture to prevent ENOENT: no such file or directory, open '.../api/src/jsons/npcs.json' failures in CI and local test execution of src/tests/market.integration.test.ts.

Features & Deliverables

  • Seed Fixture: Includes standard merchant, priest, banker, and monster NPC entries required by game_npcs database seeding and listNpcSoldItemIds().
  • Market Tests Unblocked: market.integration.test.ts can resolve NPC merchant listings to test auction house cancellations and fee reimbursements.

Comment thread api/src/jsons/npcs.json Outdated
@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds the missing npcs.json seed fixture without a UTF-8 BOM to unblock CI and market integration tests. Resolved issues regarding JSON.parse compatibility.

✅ 1 resolved
Bug: UTF-8 BOM makes npcs.json fail JSON.parse, defeating the fix

📄 api/src/jsons/npcs.json:1
The new npcs.json starts with a UTF-8 BOM (bytes ef bb bf). It is loaded via fs.readFileSync(filePath, "utf8") then JSON.parse(...) in loadNpcsJsonFromFile, and Node does not strip the BOM, so JSON.parse throws Unexpected token '' ... is not valid JSON (verified by running it). This makes seeding and listNpcSoldItemIds() fail exactly where the ENOENT used to occur, so the file that was added to unblock CI/market tests will itself break them. Re-save the file as UTF-8 without BOM (or strip the BOM before parsing).

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@leocagli

Copy link
Copy Markdown
Collaborator

Cierro esta porque el archivo que agrega ya está en main, no porque estuviera mal.

El diagnóstico era correcto: api/src/jsons/npcs.json faltaba en el repositorio y
ensureSeeded() lo lee sin guarda, así que market.integration.test.ts moría con
ENOENT en gameData.ts:259. Eso era la mitad de por qué main estaba en rojo.

La otra mitad era api/src/jsons/objs.json, que también faltaba y hacía que
GET /wiki devolviera 500. Ese segundo fallo se veía en el CI como
AssertionError: 500 !== 200 en platform.integration.test.ts:27, sin ninguna
pista de que fuera un archivo faltante.

Lo medí levantando el job de API en local con el mismo postgres:18-alpine del CI:

sin ninguno de los dos      Test Files  2 failed | 8 passed (10)
solo con npcs.json          Test Files  1 failed | 9 passed (10)   <- el wiki seguia roto
con los dos                 Test Files  11 passed (11)

Por eso se mergeó la #109, que traía los dos archivos con los datos completos del
juego (339 NPCs y 983 objetos, contra los 6 NPCs de acá). main quedó en verde por
primera vez desde que existe el workflow.

No hace falta que hagas nada. Si tenés algo más sobre la issue que no sea el
fixture, abrilo aparte y lo miro.

@leocagli leocagli closed this Aug 27, 2026
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.

Falta api/src/jsons/npcs.json en el repositorio y el test de market no puede correr

2 participants