Run the test suite against both deployment modes (full and lean) in CI#154
Merged
Conversation
#151) Add a [full, lean] deployment-mode matrix to the e2e/boot CI leg and the checks that make a mode-specific break fail CI before merge: - Split CI into a single hermetic unit job (vitest) and a matrixed e2e job; surface the mode in job and artifact names. - New tests/e2e/deployment-mode.spec.js: a hand-written feature inventory asserting each full-only feature is reachable in full and gone in lean, and the lean-only dashboard publish flow the reverse. One loop covers both directions. The gating step has no continue-on-error, so a mis-gated feature turns the leg red (acceptance #5). - New tests/ci/assert-gated-tables.js: gated-off features still have their DB tables in each mode (model sync is unconditional; only route mounts are gated), so a later mode flip needs no data migration (acceptance #4). - Lean leg disables the sidecar WITH_* flags so init-db and boot succeed without the services lean doesn't deploy (acceptance #3). - Short two-mode contributor note in README and AGENTS (author in full first, then confirm lean). No application/runtime source changes. Verified locally: both legs' spec passes 8/8, lean boots sidecar-free, and the gated tables exist in lean.
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "20" |
There was a problem hiding this comment.
Can we upgrade this version?
Author
|
Question: what are the ramifications of having the lean tests be the baseline tests and then the full tests import them? will that be cleaner than the current strategy? |
CarsonDavis
commented
Jun 29, 2026
Comment on lines
+105
to
+109
| echo "WITH_STAC=false" >> .env | ||
| echo "WITH_TIPG=false" >> .env | ||
| echo "WITH_TITILER=false" >> .env | ||
| echo "WITH_TITILER_PGSTAC=false" >> .env | ||
| echo "WITH_VELOSERVER=false" >> .env |
Author
There was a problem hiding this comment.
just a heads up to other reviewers, this is being simplified in #155, which depends on this PR for it's tests
The 'Acceptance #N' prefixes pointed at a numbered list that lives nowhere in the repo and read like GitHub issue links. The comments already explain their steps inline, so the prefixes only added confusion. Also trim a redundant restated-step-name sentence.
Cut the 30-line header to ~14: drop the dangling 'Acceptance #4' cross-ref, the both-legs/union paragraph (already explained inline at the table list), and the obvious exit-code line. Keep only what a reader can't infer from the code — why gated-off features still need tables, and the postgis-before-sync ordering trap.
Cut the dangling 'acceptance #2' ref and the redundant full/lean re-definition (that lives in AGENTS.md). Kept the two parts that earn their space: the hand-written-inventory warning (independence from the capability table is what makes the test trustworthy) and the present-vs-absent 404 discriminator the assertions rely on.
…label 'Guards a both-modes invariant' announced a rule instead of stating it. Open with the concrete requirement (gated-off features keep their DB tables) so line 1 says the thing, no 'invariant' to decode first.
…invariant, not migration The both-modes tables-exist check and the lean docs justified gated-off tables as 'so a mode flip needs no migration.' That reason doesn't hold: deployments never switch modes, and sequelize.sync() is additive and runs every boot. Reword to the accurate framing — only route mounts are gated; models register and sync unconditionally (ADR D2: keep, env-gated), so gated-off tables are created but unused. assert-gated-tables.js pins exactly that invariant (catches a model accidentally gated or dropped). Docs/comments only; no behavior change.
…es-ci # Conflicts: # .github/workflows/playwright-tests.yml
✅ Version Already UpdatedThis PR includes a manual version update to No automatic version bump needed. |
Trim verbose comments to terse notes; keep the non-obvious rationale (ADR-independent mapping, mounted-vs-404 discriminator, init-db ordering) and drop restated prose. Comments only; no behavior change.
deployment-mode.spec.js reads MMGIS_DEPLOYMENT_MODE from process.env, but the Playwright runner never loaded .env — so the spec defaulted to 'full' and the lean CI leg tested the wrong mode (all assertions failed). Mirror scripts/server.js by loading dotenv in the config. Verified via --list: the spec now resolves MODE=lean from .env.
They ran gated on development (no continue-on-error). The matrix split accidentally added continue-on-error: true, silently downgrading their coverage. Restore gating so a real regression fails the leg; they also now confirm smoke + event bus pass under lean, not just full.
sandesh-sp
approved these changes
Jul 1, 2026
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
3 participants
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.
What this does
MMGIS ships in two shapes from one codebase, selected by
MMGIS_DEPLOYMENT_MODE: the full application, and a lean deployment that turns a set of server features off (geodata management, drawing, the bundled sidecar services, the on-disk mission filesystem, the link shortener, server-side raster utilities) and the dashboard publish flow on. Today CI exercises essentially one of those shapes — a change that breaks the other can pass CI and reach production.This runs the e2e/boot suite once per shape and adds checks that verify the right features are present or absent in each mode, so a change that breaks either shape fails CI before merge. It's also the safety net for the planned (separate) gating-consolidation refactor, whose "no behavior change" claim is only trustworthy if the tests already run both modes.
Implements #151.
Changes
CI / test / docs only — no application or runtime source changed.
unitjob (Vitest, no DB/browser — it already covers both modes via require-cache busting, so it isn't matrixed) and a matrixede2ejob overmode: [full, lean]. The mode is written into.envand surfaced in the job name (e2e (full)/e2e (lean)) and per-mode artifact names.tests/e2e/deployment-mode.spec.js). A hand-written feature inventory (independent of any capability table — that independence is the point) asserting each full-only feature is reachable in full and gone in lean, and the lean-only dashboard publish flow the reverse. One loop covers both directions. Discriminator: a mounted route answers non-404; an unmounted one falls through to the catch-all 404. This step has nocontinue-on-error, so mis-gating a feature turns the leg red.playwright.config.js). The Playwright config loads.env(via dotenv, mirroringscripts/server.js) so the test process seesMMGIS_DEPLOYMENT_MODE. Without it the spec defaults tofulland the lean leg silently tests the wrong mode.tests/ci/assert-gated-tables.js). Features gated off in a mode still have their DB tables, because model registration + sync run unconditionally on boot and only route mounts are gated (the minimal-divergence gating from ADR D2 — models aren't per-mode-gated). This check pins that invariant — a change that accidentally gates model registration or drops a model fails CI. Asserted via the samegetBackendSetups+syncpath the server uses.WITH_*flags soinit-dband boot succeed without the services lean doesn't deploy (no sidecars, no spatial-catalog DB).README.md: two modes exist; author in full first, then confirm lean.Verification
Booted both modes locally and ran the new checks:
Full-only routes return non-404 in full / 404 in lean;
/api/deployments(publish flow) is the reverse.npx vitest runstays green (669/669).Notes for reviewers
development(already includes the merged Run unit tests in a jsdom environment via Vitest #150); no stacked prerequisites remain.smoke/eventbuse2e specs remain gating (nocontinue-on-error), matching their behavior ondevelopment; the split into their own step is organizational only. The new mode-shape and tables checks are also gating.Closes #151