bermudalamb
f76db6c8fe
The guard added for #107 finds nothing on a checkout with CRLF line endings, which is every fresh clone on Windows. Splitting on a bare newline leaves a trailing carriage return, the end-of-line anchor in the entry pattern then cannot match, and all ten assertions in the file fail together. Worth being precise about why this shipped, because the process that was supposed to prevent it ran and did not. That guard was fired deliberately before committing: the UPLOADS_DIR line was removed, two tests failed, the line was restored, ten passed. What the exercise never varied was the file's line endings — and by then the working copy happened to be LF, because the backup-and-restore used to fire the guard had rewritten it that way. So the deliberate firing proved the guard catches a missing variable, on a file shaped exactly as the test run had shaped it, and proved nothing about the shape it meets in a clean clone. The failure mode is the one the file already worried about: parsing that matches nothing makes every other assertion vacuously true. Here it failed loudly instead only because the "parsed some entries at all" case exists — which is the case that turned a silent pass into a visible failure, and is the reason this was noticed at all rather than sitting green and checking nothing. Splitting on an optional carriage return fixes it. 172 unit tests pass on the CRLF checkout that was failing. Found while verifying #106, whose branch could not go green until this was fixed, which is why the fix lands there rather than on its own. Refs #107 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
83 lines
3.6 KiB
TypeScript
83 lines
3.6 KiB
TypeScript
import { readFileSync } from 'fs';
|
|
import path from 'path';
|
|
import { ALWAYS_REQUIRED } from '../../src/envValidation';
|
|
|
|
/**
|
|
* The guard for #107.
|
|
*
|
|
* That failure was not the missing variable — it was that nothing connected two
|
|
* files. envValidation.ts gained a required variable and docker-compose.qa.yml
|
|
* did not set it, and nothing noticed until the container refused to boot on a
|
|
* deploy. CI passed throughout, because CI sets its own environment and never
|
|
* reads the compose file.
|
|
*
|
|
* So this reads the real list from the validator rather than a copy. A copy
|
|
* would pass forever while the next added variable went unguarded in precisely
|
|
* the same way.
|
|
*
|
|
* What this cannot cover: production runs from a Portainer stack outside this
|
|
* repository, so nothing here can check it. Adding a required variable still
|
|
* means updating that stack by hand, and this test is not evidence that it was
|
|
* done.
|
|
*/
|
|
|
|
const COMPOSE_PATH = path.resolve(__dirname, '..', '..', '..', 'docker-compose.qa.yml');
|
|
const compose = readFileSync(COMPOSE_PATH, 'utf8');
|
|
|
|
// Only real environment entries — `- NAME=value` at an indented list position.
|
|
// A mention inside a comment cannot match, because a comment line starts with #.
|
|
function environmentEntries(source: string): Map<string, string> {
|
|
const entries = new Map<string, string>();
|
|
// The split pattern tolerates carriage returns. A checkout with CRLF line
|
|
// endings, which is every fresh clone on Windows, otherwise leaves a stray
|
|
// carriage return that the end-of-line anchor below cannot match, and every
|
|
// assertion in this file then silently finds nothing at all. That is exactly
|
|
// the failure the "parsed some entries" case exists to catch, and it is how
|
|
// this guard was found to be broken.
|
|
for (const line of source.split(/\r?\n/)) {
|
|
const match = /^\s+- ([A-Z_0-9]+)=(.*)$/.exec(line);
|
|
if (match) {
|
|
entries.set(match[1], match[2].trim());
|
|
}
|
|
}
|
|
return entries;
|
|
}
|
|
|
|
const entries = environmentEntries(compose);
|
|
|
|
describe('the QA compose file provides everything the app requires to boot', () => {
|
|
it('parsed some environment entries at all', () => {
|
|
// Guards the guard: a regex that matched nothing would make every
|
|
// assertion below vacuously true.
|
|
expect(entries.size).toBeGreaterThan(5);
|
|
});
|
|
|
|
it.each([...ALWAYS_REQUIRED])('sets %s', (name) => {
|
|
expect(entries.has(name)).toBe(true);
|
|
});
|
|
|
|
// DEMO_MODE is required too, but validated separately from ALWAYS_REQUIRED
|
|
// because its rule is stricter than presence — it must be exactly 'true' or
|
|
// 'false'. Named explicitly here so it is not missed by reading only the list.
|
|
it('sets DEMO_MODE, to one of the two values that are allowed', () => {
|
|
expect(entries.has('DEMO_MODE')).toBe(true);
|
|
expect(['true', 'false']).toContain(entries.get('DEMO_MODE'));
|
|
});
|
|
|
|
// The reason UPLOADS_DIR is hardcoded rather than taken from a stack
|
|
// variable is that it has to agree with the volume mapping. That claim is
|
|
// only worth making if something checks it.
|
|
it('points UPLOADS_DIR at the directory the uploads volume is mounted on', () => {
|
|
const uploadsDir = entries.get('UPLOADS_DIR');
|
|
expect(uploadsDir).toBeTruthy();
|
|
expect(compose).toContain(`:${uploadsDir}`);
|
|
});
|
|
|
|
// Interpolated rather than hardcoded, so the secret itself never enters the
|
|
// repository. Present as a reference is what matters; an unset stack variable
|
|
// resolves to empty, which the validator and the gate both read as "off".
|
|
it('references ADMIN_GATE_SECRET from the stack rather than holding a value', () => {
|
|
expect(entries.get('ADMIN_GATE_SECRET')).toBe('${ADMIN_GATE_SECRET}');
|
|
});
|
|
});
|