CI/Docs: test:charts job isolation and ChartsPage integration scope #230

Closed
opened 2026-05-29 05:41:52 +00:00 by PlasticDigits · 9 comments
PlasticDigits commented 2026-05-29 05:41:52 +00:00 (Migrated from gitlab.com)

Summary

Clarify CI boundaries for npm run test:charts (optional dedicated job) and ChartsPage.integration.test.tsx scope so HTTP integration tests do not imply canvas coverage, while chart-real tests remain required and documented.

Bundled scope: separate CI job for test:charts · ChartsPage integration scope/docs


Current codebase

Area Files
CI .github/workflows/test.yml — frontend job: test:run then test:charts (same job)
Charts Vitest vitest.config.charts.ts — Node canvas native dep via chartsSetup.ts
Makefile test-frontend-charts → npm run test:charts
Integration frontend-dapp/src/pages/ChartsPage.integration.test.tsx — live Postgres/indexer HTTP; uses default Vitest config → stubbed lightweight-charts
Config vitest.config.integration.ts includes lightweightChartsJsdomMock.ts
Docs docs/testing.md — split documented (#211)
Gap gaps/GAP_1780023683.md — /charts row notes stub + test:charts

test:charts shares the frontend job with 662+ unit tests — if canvas install or native bindings fail, entire frontend job fails. Integration tests do not document that canvas is intentionally out of scope.


Why this is needed

  1. CI isolation: Separate job allows canvas system deps (libcairo, etc.) and longer timeouts without blocking fast unit signal.
  2. Developer clarity: Contributors may assume ChartsPage.integration.test.tsx validates chart render — it only validates indexer → React data path.
  3. #211 follow-up: Operational hardening as chart suite grows (perf tests from related issue).

Constraints and guardrails

  1. Do not require Postgres for test:charts — keep chart job independent.
  2. Integration tests stay HTTP-focused — no pulling real library into vitest.config.integration.ts unless explicitly justified.
  3. Default PR signal: Either keep combined job or make test:charts required in a dedicated job — never optional/skip silently.
  4. Document in ChartsPage.integration.test.tsx header comment + docs/testing.md.
  5. Makefile parity for local dev.

Relevant files

  • .github/workflows/test.yml
  • frontend-dapp/vitest.config.charts.ts
  • frontend-dapp/vitest.config.integration.ts
  • frontend-dapp/src/pages/ChartsPage.integration.test.tsx
  • Makefile
  • docs/testing.md
  • gaps/GAP_1780023683.md
  • skills/AGENTS_TESTING_P2_EPIC.md, skills/AGENTS_LOCAL_POSTGRES_DEV.md

  1. Evaluate split: New job frontend-charts-vitest (or extend existing frontend-charts-integration naming carefully) running only npm run test:charts with canvas OS packages if needed on ubuntu-latest.
  2. Remove duplicate from frontend job only if new job is required branch protection check.
  3. ChartsPage.integration.test.tsx: Top-of-file comment: “Indexer HTTP only; canvas: npm run test:charts (#211).” Optional test asserting page renders without requiring canvas (already stubbed).
  4. docs/testing.md: Matrix row for integration vs charts vs Playwright.

Acceptance criteria

  • CI strategy documented (combined vs split) with rationale in PR.
  • If split: dedicated required job runs test:charts; frontend job still runs test:run.
  • ChartsPage.integration.test.tsx documents non-canvas scope.
  • docs/testing.md + gaps/GAP_1780023683.md updated.
  • make test-frontend-charts unchanged for local use.

Test plan — functional paths

# Path Expected
1 CI frontend job test:run green
2 CI charts job (if split) test:charts green
3 make test-charts-integration HTTP integration green (existing)
4 Local npm run test:charts Documented deps
5 Integration file comment Points to #211 / #229

Test plan — attack vectors

Vector Approach Expected
Missing canvas native module CI without deps Job fails loudly with install doc
Skipped charts job Branch protection Required check blocks merge
False confidence Dev runs only integration Docs state canvas suite required

Verification criteria

  1. Push to branch; all required GitLab CI jobs green.
  2. README/docs: contributor path for chart tests clear.
  3. Cross-link #211.

## Summary Clarify **CI boundaries** for `npm run test:charts` (optional dedicated job) and **`ChartsPage.integration.test.tsx`** scope so HTTP integration tests do not imply canvas coverage, while chart-real tests remain required and documented. **Bundled scope:** separate CI job for `test:charts` · ChartsPage integration scope/docs --- ## Current codebase | Area | Files | |------|--------| | CI | `.github/workflows/test.yml` — `frontend` job: `test:run` then `test:charts` (same job) | | Charts Vitest | `vitest.config.charts.ts` — Node `canvas` native dep via `chartsSetup.ts` | | Makefile | `test-frontend-charts` → `npm run test:charts` | | Integration | `frontend-dapp/src/pages/ChartsPage.integration.test.tsx` — live Postgres/indexer HTTP; uses default Vitest config → **stubbed** lightweight-charts | | Config | `vitest.config.integration.ts` includes `lightweightChartsJsdomMock.ts` | | Docs | `docs/testing.md` — split documented ([#211](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/211)) | | Gap | `gaps/GAP_1780023683.md` — `/charts` row notes stub + `test:charts` | `test:charts` shares the `frontend` job with 662+ unit tests — if `canvas` install or native bindings fail, entire frontend job fails. Integration tests do not document that canvas is intentionally out of scope. --- ## Why this is needed 1. **CI isolation:** Separate job allows `canvas` system deps (`libcairo`, etc.) and longer timeouts without blocking fast unit signal. 2. **Developer clarity:** Contributors may assume `ChartsPage.integration.test.tsx` validates chart render — it only validates indexer → React data path. 3. **#211 follow-up:** Operational hardening as chart suite grows (perf tests from related issue). --- ## Constraints and guardrails 1. **Do not require Postgres** for `test:charts` — keep chart job independent. 2. **Integration tests stay HTTP-focused** — no pulling real library into `vitest.config.integration.ts` unless explicitly justified. 3. **Default PR signal:** Either keep combined job **or** make `test:charts` required in a dedicated job — never optional/skip silently. 4. **Document** in `ChartsPage.integration.test.tsx` header comment + `docs/testing.md`. 5. **Makefile** parity for local dev. --- ## Relevant files - `.github/workflows/test.yml` - `frontend-dapp/vitest.config.charts.ts` - `frontend-dapp/vitest.config.integration.ts` - `frontend-dapp/src/pages/ChartsPage.integration.test.tsx` - `Makefile` - `docs/testing.md` - `gaps/GAP_1780023683.md` - `skills/AGENTS_TESTING_P2_EPIC.md`, `skills/AGENTS_LOCAL_POSTGRES_DEV.md` --- ## Recommended direction 1. **Evaluate split:** New job `frontend-charts-vitest` (or extend existing `frontend-charts-integration` naming carefully) running only `npm run test:charts` with `canvas` OS packages if needed on `ubuntu-latest`. 2. **Remove duplicate** from `frontend` job only if new job is **required** branch protection check. 3. **`ChartsPage.integration.test.tsx`:** Top-of-file comment: “Indexer HTTP only; canvas: `npm run test:charts` ([#211](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/211)).” Optional test asserting page renders without requiring canvas (already stubbed). 4. **docs/testing.md:** Matrix row for integration vs charts vs Playwright. --- ## Acceptance criteria - [ ] CI strategy documented (combined vs split) with rationale in PR. - [ ] If split: dedicated required job runs `test:charts`; `frontend` job still runs `test:run`. - [ ] `ChartsPage.integration.test.tsx` documents non-canvas scope. - [ ] `docs/testing.md` + `gaps/GAP_1780023683.md` updated. - [ ] `make test-frontend-charts` unchanged for local use. --- ## Test plan — functional paths | # | Path | Expected | |---|------|----------| | 1 | CI `frontend` job | `test:run` green | | 2 | CI charts job (if split) | `test:charts` green | | 3 | `make test-charts-integration` | HTTP integration green (existing) | | 4 | Local `npm run test:charts` | Documented deps | | 5 | Integration file comment | Points to #211 / #229 | --- ## Test plan — attack vectors | Vector | Approach | Expected | |--------|----------|----------| | Missing `canvas` native module | CI without deps | Job fails loudly with install doc | | Skipped charts job | Branch protection | Required check blocks merge | | False confidence | Dev runs only integration | Docs state canvas suite required | --- ## Verification criteria 1. Push to branch; all required GitLab CI jobs green. 2. README/docs: contributor path for chart tests clear. 3. Cross-link [#211](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/211). --- ## Related issues - [#211](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/211), [#205](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/205) (charts integration fixtures), [#105](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/105)
PlasticDigits commented 2026-05-29 05:41:53 +00:00 (Migrated from gitlab.com)

marked as related to #211

marked as related to #211
PlasticDigits commented 2026-05-29 05:41:54 +00:00 (Migrated from gitlab.com)

marked as related to #205

marked as related to #205
PlasticDigits commented 2026-05-29 05:41:56 +00:00 (Migrated from gitlab.com)

marked as related to #105

marked as related to #105
PlasticDigits commented 2026-05-29 09:12:27 +00:00 (Migrated from gitlab.com)

mentioned in commit e71928b0de

mentioned in commit e71928b0decc39f884bba2c706ae07dad79dc53d
PlasticDigits commented 2026-05-29 09:20:55 +00:00 (Migrated from gitlab.com)

mentioned in commit 308a04a3c3

mentioned in commit 308a04a3c310a8f02e11bc3bb03a67ecff01b5f5
PlasticDigits commented 2026-05-29 09:21:02 +00:00 (Migrated from gitlab.com)

Implementation complete (issue stays open for verification)

Commits on main:

  • e71928b — CI split: dedicated frontend-charts-vitest job (npm run test:charts + canvas OS deps); frontend job runs only test:run (no canvas native deps blocking 600+ unit tests).
  • 308a04a — Doc follow-up: AGENTS_LOCAL_POSTGRES_DEV.md + vitest.config.integration.ts header (Postgres vs canvas layers).

What changed

Area Change
CI .github/workflows/test.yml — new required job frontend-charts-vitest; frontend no longer runs test:charts
Integration scope ChartsPage.integration.test.tsx header — indexer HTTP only; stubbed lightweight-charts
Docs docs/testing.md — charts layer matrix (unit / real Vitest / HTTP integration / Playwright)
Gap doc gaps/GAP_1780023683.md — /charts row + testing inventory
Agent playbooks skills/AGENTS_TESTING_P2_EPIC.md, skills/AGENTS_FRONTEND_PRICE_CHART.md, skills/AGENTS_LOCAL_POSTGRES_DEV.md

Local verification (this agent): make test-frontend-charts — 21 passed, 1 skipped (CI-only 2000-candle soak).

Verification checklist

  • GitLab CI: job Frontend — tsc, lint, npm run test:run green
  • GitLab CI: job Frontend charts Vitest — npm run test:charts green (canvas apt packages installed)
  • GitLab CI: job Frontend charts integration — Postgres + indexer + npm run test:integration green
  • Branch protection: frontend-charts-vitest is a required check (not silently skipped)
  • ChartsPage.integration.test.tsx header matches intent (no canvas claims)
  • make test-frontend-charts works locally (install Ubuntu canvas deps from docs/testing.md if native module fails)
  • make test-charts-integration still exercises indexer HTTP path (Postgres + indexer on :3001)

Cross-links: #211, #229.

@brouie — please run through the checklist above on the latest main pipeline and confirm branch-protection includes frontend-charts-vitest. Leaving this issue open until verified.

## Implementation complete (issue stays open for verification) **Commits on `main`:** - `e71928b` — CI split: dedicated **`frontend-charts-vitest`** job (`npm run test:charts` + canvas OS deps); **`frontend`** job runs only `test:run` (no canvas native deps blocking 600+ unit tests). - `308a04a` — Doc follow-up: `AGENTS_LOCAL_POSTGRES_DEV.md` + `vitest.config.integration.ts` header (Postgres vs canvas layers). ### What changed | Area | Change | |------|--------| | CI | `.github/workflows/test.yml` — new required job `frontend-charts-vitest`; `frontend` no longer runs `test:charts` | | Integration scope | `ChartsPage.integration.test.tsx` header — indexer HTTP only; stubbed `lightweight-charts` | | Docs | `docs/testing.md` — charts layer matrix (unit / real Vitest / HTTP integration / Playwright) | | Gap doc | `gaps/GAP_1780023683.md` — `/charts` row + testing inventory | | Agent playbooks | `skills/AGENTS_TESTING_P2_EPIC.md`, `skills/AGENTS_FRONTEND_PRICE_CHART.md`, `skills/AGENTS_LOCAL_POSTGRES_DEV.md` | **Local verification (this agent):** `make test-frontend-charts` — 21 passed, 1 skipped (CI-only 2000-candle soak). ### Verification checklist - [ ] GitLab CI: job **`Frontend`** — `tsc`, lint, `npm run test:run` green - [ ] GitLab CI: job **`Frontend charts Vitest`** — `npm run test:charts` green (canvas apt packages installed) - [ ] GitLab CI: job **`Frontend charts integration`** — Postgres + indexer + `npm run test:integration` green - [ ] Branch protection: `frontend-charts-vitest` is a **required** check (not silently skipped) - [ ] `ChartsPage.integration.test.tsx` header matches intent (no canvas claims) - [ ] `make test-frontend-charts` works locally (install Ubuntu canvas deps from `docs/testing.md` if native module fails) - [ ] `make test-charts-integration` still exercises indexer HTTP path (Postgres + indexer on `:3001`) Cross-links: [#211](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/211), [#229](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/229). @brouie — please run through the checklist above on the latest `main` pipeline and confirm branch-protection includes **`frontend-charts-vitest`**. Leaving this issue **open** until verified.
PlasticDigits commented 2026-05-30 06:42:15 +00:00 (Migrated from gitlab.com)

mentioned in commit a86df80471

mentioned in commit a86df80471ad87ca94676ed9bb0fa78677866912
PlasticDigits commented 2026-05-30 06:42:32 +00:00 (Migrated from gitlab.com)

Verification complete — issue left open for @brouie (branch protection / CI hosting)

Worktree: verify/issue-230 at /home/answorld/repos/cl8y-dex-terraclassic-verify-230
Merged to main: a86df80 — docs(ci): add frontend + frontend-charts-vitest rows to .github/workflows/README.md (local job mapping follow-up for #230).

Implementation already on main (prior commits)

Commit Change
e71928b CI split: dedicated frontend-charts-vitest job; frontend runs only test:run
308a04a Docs cross-link chart Vitest layers (docs/testing.md, skills, gap doc)

Local verification (this agent — main @ a86df80)

Check Result
make test-frontend (test:run) 710 passed (116 files)
make test-frontend-charts (test:charts) 21 passed, 1 skipped (CI-only 2000-candle soak)
make test-charts-integration 5 passed (ChartsPage HTTP integration); limit-order ref tests skipped (no LocalTerra LCD)
ChartsPage.integration.test.tsx header Documents indexer HTTP only; points to #211 / #229 / #230 and frontend-charts-vitest
vitest.config.integration.ts header Stubbed canvas; real library → test:charts
docs/testing.md charts layer matrix Present (unit / real Vitest / HTTP integration / Playwright)
gaps/GAP_1780023683.md /charts row + testing inventory updated
Skills cross-links AGENTS_TESTING_P2_EPIC.md, AGENTS_FRONTEND_PRICE_CHART.md, AGENTS_LOCAL_POSTGRES_DEV.md
Makefile test-frontend-charts unchanged → npm run test:charts

Infra used: Postgres + indexer on :3001 (left running). No frontend dev server started (not needed for this issue).

Acceptance criteria status

  • CI strategy documented (combined → split; rationale in docs/testing.md § CI + .github/workflows/test.yml)
  • Split: frontend-charts-vitest runs test:charts; frontend runs test:run only
  • ChartsPage.integration.test.tsx documents non-canvas scope
  • docs/testing.md + gaps/GAP_1780023683.md updated
  • make test-frontend-charts works locally

Checklist items not verifiable here

  • GitLab CI pipeline green — project policy (docs/testing.md § CI, .github/workflows/README.md): no GitHub Actions or GitLab CI pipelines run today; workflow YAML is a reference matrix. Local equivalents above all passed.
  • Branch protection: frontend-charts-vitest as required check — needs maintainer confirmation (and CI hosting if/when enabled).

Manual re-verify checklist

  1. make test-frontend-charts — expect 21 passed, 1 skipped without Postgres
  2. make test-charts-integration — Postgres + indexer on :3001; expect ChartsPage integration green
  3. Confirm .github/workflows/test.yml: frontend has no test:charts; frontend-charts-vitest installs canvas OS deps
  4. Read ChartsPage.integration.test.tsx header — no canvas claims
  5. When CI is enabled: confirm Frontend charts Vitest job is required in branch protection

Cross-links: #211, #229.

@brouie — please confirm branch-protection / CI hosting plan for frontend-charts-vitest. Leaving open until that item is confirmed or CI is live and green on main.

## Verification complete — issue left open for @brouie (branch protection / CI hosting) **Worktree:** `verify/issue-230` at `/home/answorld/repos/cl8y-dex-terraclassic-verify-230` **Merged to `main`:** `a86df80` — docs(ci): add `frontend` + `frontend-charts-vitest` rows to [`.github/workflows/README.md`](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/blob/main/.github/workflows/README.md) (local job mapping follow-up for #230). ### Implementation already on `main` (prior commits) | Commit | Change | |--------|--------| | `e71928b` | CI split: dedicated **`frontend-charts-vitest`** job; **`frontend`** runs only `test:run` | | `308a04a` | Docs cross-link chart Vitest layers (`docs/testing.md`, skills, gap doc) | ### Local verification (this agent — `main` @ `a86df80`) | Check | Result | |-------|--------| | `make test-frontend` (`test:run`) | **710 passed** (116 files) | | `make test-frontend-charts` (`test:charts`) | **21 passed**, 1 skipped (CI-only 2000-candle soak) | | `make test-charts-integration` | **5 passed** (ChartsPage HTTP integration); limit-order ref tests skipped (no LocalTerra LCD) | | `ChartsPage.integration.test.tsx` header | Documents indexer HTTP only; points to #211 / #229 / #230 and `frontend-charts-vitest` | | `vitest.config.integration.ts` header | Stubbed canvas; real library → `test:charts` | | `docs/testing.md` charts layer matrix | Present (unit / real Vitest / HTTP integration / Playwright) | | `gaps/GAP_1780023683.md` | `/charts` row + testing inventory updated | | Skills cross-links | `AGENTS_TESTING_P2_EPIC.md`, `AGENTS_FRONTEND_PRICE_CHART.md`, `AGENTS_LOCAL_POSTGRES_DEV.md` | | Makefile | `test-frontend-charts` unchanged → `npm run test:charts` | **Infra used:** Postgres + indexer on `:3001` (left running). No frontend dev server started (not needed for this issue). ### Acceptance criteria status - [x] CI strategy documented (combined → **split**; rationale in `docs/testing.md` § CI + `.github/workflows/test.yml`) - [x] Split: `frontend-charts-vitest` runs `test:charts`; `frontend` runs `test:run` only - [x] `ChartsPage.integration.test.tsx` documents non-canvas scope - [x] `docs/testing.md` + `gaps/GAP_1780023683.md` updated - [x] `make test-frontend-charts` works locally ### Checklist items **not** verifiable here - [ ] **GitLab CI pipeline green** — project policy ([`docs/testing.md` § CI](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/blob/main/docs/testing.md#ci), [`.github/workflows/README.md`](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/blob/main/.github/workflows/README.md)): **no GitHub Actions or GitLab CI pipelines run today**; workflow YAML is a reference matrix. Local equivalents above all passed. - [ ] **Branch protection:** `frontend-charts-vitest` as required check — needs maintainer confirmation (and CI hosting if/when enabled). ### Manual re-verify checklist 1. `make test-frontend-charts` — expect 21 passed, 1 skipped without Postgres 2. `make test-charts-integration` — Postgres + indexer on `:3001`; expect ChartsPage integration green 3. Confirm `.github/workflows/test.yml`: `frontend` has no `test:charts`; `frontend-charts-vitest` installs canvas OS deps 4. Read `ChartsPage.integration.test.tsx` header — no canvas claims 5. When CI is enabled: confirm **`Frontend charts Vitest`** job is required in branch protection Cross-links: #211, #229. @brouie — please confirm branch-protection / CI hosting plan for `frontend-charts-vitest`. Leaving **open** until that item is confirmed or CI is live and green on `main`.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-30 09:13:52 +00:00
PlasticDigits commented 2026-05-30 09:13:53 +00:00 (Migrated from gitlab.com)

We are not using gitlab ci pipleline or github actions, nor branch protection, so closing.

We are not using gitlab ci pipleline or github actions, nor branch protection, so closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-dex-terraclassic#230
No description provided.