diff --git a/backend/tests/unit/e2eDatabaseHandoff.test.ts b/backend/tests/unit/e2eDatabaseHandoff.test.ts new file mode 100644 index 0000000..171d767 --- /dev/null +++ b/backend/tests/unit/e2eDatabaseHandoff.test.ts @@ -0,0 +1,120 @@ +import { readFileSync } from 'fs'; +import path from 'path'; + +/** + * The guard for #273. + * + * Three files have to agree about which database the end-to-end suite reads: + * `start-local.ps1` chooses it and records it, `run-tests.ps1` reads that record + * into the environment, and `frontend/tests/e2e/support/db.ts` opens the + * connection from those variables. Nothing compared them, and they disagreed: + * with `-E2eDb` the application ran on `redefined_e2e` at 55501 while the helper + * kept its `redefined_local` default at 55500, so the app wrote to one database + * and the suite read another. + * + * That is the same shape as #107 and #118, where `envValidation` and a compose + * file disagreed and nothing noticed until a deploy refused to boot — and the + * same shape again as #287, where the workflow and `start-local.ps1` disagreed + * about REMBG_URL and three tests failed on main. The lesson each time is that + * a convention shared between two files is not a contract until something + * checks it. + * + * These are text assertions, which is weaker than executing the scripts — a + * PowerShell run is not something this suite can do. They are still worth + * having: every one of them fails if a name is changed in one file and not the + * others, which is the drift that actually happened. + */ + +const REPO_ROOT = path.resolve(__dirname, '..', '..', '..'); + +function read(relative: string): string { + return readFileSync(path.join(REPO_ROOT, relative), 'utf8'); +} + +const START_LOCAL = read('scripts/start-local.ps1'); +const RUN_TESTS = read('scripts/run-tests.ps1'); +const DB_HELPER = read('frontend/tests/e2e/support/db.ts'); + +/** The five settings a connection needs, as the JSON record names them. */ +const RECORD_KEYS = ['host', 'port', 'user', 'password', 'database'] as const; + +/** The same five, as the environment variables the helper reads. */ +const ENV_NAMES = [ + 'TEST_PGHOST', + 'TEST_PGPORT', + 'TEST_PGUSER', + 'TEST_PGPASSWORD', + 'TEST_PGDATABASE' +] as const; + +describe('the record start-local.ps1 writes', () => { + it('is written to .local/database.json', () => { + expect(START_LOCAL).toContain("Join-Path $StateDir 'database.json'"); + }); + + it.each(RECORD_KEYS)('records %s', (key) => { + // The writer is a hashtable literal, so each key appears as `key =`. + expect(START_LOCAL).toMatch(new RegExp(`^\\s*${key}\\s*=`, 'm')); + }); + + // Written after the database is up, so the file never names one that failed + // to start, and removed on -Stop so a stopped stack does not leave a record + // pointing at a container that is gone. + it('is written only once the database has started', () => { + const startDatabase = START_LOCAL.indexOf(' Start-Database'); + const writeChoice = START_LOCAL.indexOf(' Write-DatabaseChoice'); + expect(startDatabase).toBeGreaterThan(-1); + expect(writeChoice).toBeGreaterThan(startDatabase); + }); + + it('is removed when the stack is stopped', () => { + expect(START_LOCAL).toContain('Remove-Item $DatabaseFile'); + }); +}); + +describe('what run-tests.ps1 does with it', () => { + it('reads the same file start-local.ps1 writes', () => { + expect(RUN_TESTS).toContain('.local/database.json'); + }); + + it.each(ENV_NAMES)('sets %s for the end-to-end run', (name) => { + expect(RUN_TESTS).toMatch(new RegExp(`\\$env:${name}\\s*=`)); + }); + + it.each(RECORD_KEYS)('takes %s from the record rather than a default', (key) => { + expect(RUN_TESTS).toContain(`$db.${key}`); + }); + + /** + * The second half of #273, and the reason all five are set rather than the + * port alone. Invoke-IntegrationSuite sets TEST_PGPORT and PowerShell keeps + * it for the rest of the session, so a `-Suite all` run leaked the + * integration port into the e2e run that followed — with none of the matching + * credentials. Setting every one of them is what overrides that. + */ + it('sets every connection variable, so a leaked one cannot survive', () => { + const leaks = ENV_NAMES.filter((name) => !RUN_TESTS.includes(`$env:${name} =`)); + expect(leaks).toEqual([]); + }); + + // A default is what caused both faults: always plausible, silently wrong, and + // it fails in ways that look like application bugs rather than configuration. + it('refuses to guess when the record is missing', () => { + expect(RUN_TESTS).toMatch(/if \(-not \(Test-Path \$databaseFile\)\) \{\s*throw/); + }); +}); + +describe('what the helper actually connects with', () => { + it.each(ENV_NAMES)('reads %s', (name) => { + expect(DB_HELPER).toContain(`process.env.${name}`); + }); + + // The chain is only closed if the helper reads no connection setting that + // run-tests.ps1 does not set. A sixth variable added here and nowhere else + // would reintroduce exactly the drift this guards. + it('reads no connection variable the runner does not set', () => { + const used = [...DB_HELPER.matchAll(/process\.env\.(TEST_PG[A-Z]+)/g)].map((m) => m[1]!); + const unset = [...new Set(used)].filter((name) => !RUN_TESTS.includes(`$env:${name} =`)); + expect(unset).toEqual([]); + }); +}); diff --git a/scripts/run-tests.ps1 b/scripts/run-tests.ps1 index 5897386..00c300c 100644 --- a/scripts/run-tests.ps1 +++ b/scripts/run-tests.ps1 @@ -145,6 +145,53 @@ function Invoke-IntegrationSuite { finally { Pop-Location } } +<# +.SYNOPSIS +Points the end-to-end helper at the database the running stack actually uses. + +.DESCRIPTION +Closes both halves of #273. + +The helper in frontend/tests/e2e/support/db.ts opens its own connection to read +a password-reset token, and it defaults to redefined_local on 55500. With +start-local.ps1 -E2eDb the application is on redefined_e2e on 55501 instead, so +the app wrote to one database and the suite read another. + +Separately, Invoke-IntegrationSuite sets TEST_PGPORT to the integration +database and PowerShell keeps it for the rest of the session, so a -Suite all +run leaked that port into the e2e run that followed — with none of the matching +credentials, which left the helper offering redefined_local's password to the +integration database. Setting all five here overrides that leak as well, which +is why one change closes both faults. + +Throws rather than falling back to a default. A default is what produced both +faults: it is always plausible and silently wrong, and a suite that reads the +wrong database fails in ways that look like application bugs. +#> +function Set-E2eDatabaseEnv { + $databaseFile = Join-Path $RepoRoot '.local/database.json' + if (-not (Test-Path $databaseFile)) { + throw @" +No .local/database.json, so there is no way to know which database the stack is +using — and guessing is what #273 was about. + +It is written by start-local.ps1 when it brings the database up. Start the stack +first: + + .\scripts\start-local.ps1 (development database) + .\scripts\start-local.ps1 -E2eDb (throwaway end-to-end database) +"@ + } + + $db = Get-Content $databaseFile -Raw | ConvertFrom-Json + $env:TEST_PGHOST = $db.host + $env:TEST_PGPORT = "$($db.port)" + $env:TEST_PGUSER = $db.user + $env:TEST_PGPASSWORD = $db.password + $env:TEST_PGDATABASE = $db.database + Write-Good "end-to-end helper pointed at $($db.database) on $($db.port)" +} + function Invoke-E2eSuite { # Playwright's own webServer starts the dev server, but the backend it talks # to is not its job. Checked here rather than left to twenty-five specs @@ -167,6 +214,10 @@ running stack. Start it first: "@ } Write-Good 'backend is up' + # Before Playwright starts, so every spec inherits it — and after the + # backend check, so a stack that is not running is reported as that rather + # than as a missing state file. + Set-E2eDatabaseEnv Write-Note "dev server on $WebPort is started by Playwright if it is not already running" Write-Step 'Frontend end-to-end tests' diff --git a/scripts/start-local.ps1 b/scripts/start-local.ps1 index ec0ef67..857bb23 100644 --- a/scripts/start-local.ps1 +++ b/scripts/start-local.ps1 @@ -65,6 +65,9 @@ $ErrorActionPreference = 'Stop' $RepoRoot = Split-Path -Parent $PSScriptRoot $StateDir = Join-Path $RepoRoot '.local' $PidFile = Join-Path $StateDir 'pids.json' +# Which database the stack is actually using, written for the test runner to +# read. See Write-DatabaseChoice and #273. +$DatabaseFile = Join-Path $StateDir 'database.json' # Whether this run is starting from an empty database. Set below, and read by # Start-Backend, which must refuse to reuse a backend that is still serving the @@ -144,6 +147,34 @@ function Stop-Tracked { } } +<# +.SYNOPSIS +Records the database this run chose, so the test runner does not have to guess. + +.DESCRIPTION +The end-to-end helper (frontend/tests/e2e/support/db.ts) opens its own +connection to read a password-reset token, and nothing guaranteed it pointed at +the same database as the application under test. With -E2eDb the app moves to +redefined_e2e on 55501 while the helper stayed on its redefined_local default, +so the app wrote to one database and the suite read another and the specs failed +for a reason that had nothing to do with them. + +Writing it here makes the answer a fact rather than a convention: it is recorded +by the thing that made the choice, at the moment it made it, and a missing file +is a clear failure in run-tests.ps1 rather than a silent connection to the wrong +data. See #273. +#> +function Write-DatabaseChoice { + @{ + host = 'localhost' + port = $DbPort + user = $DbUser + password = $DbPassword + database = $DbName + } | ConvertTo-Json | Set-Content $DatabaseFile + Write-Note "recorded $DbName on $DbPort in .local/database.json" +} + function Set-Tracked { param([string]$Name, [int]$ProcessId) $tracked = Get-TrackedProcesses @@ -180,6 +211,9 @@ function Stop-Environment { if ($LASTEXITCODE -eq 0) { Write-Good "stopped container $Container" } else { Write-Note "container $Container was not running" } Remove-Item $PidFile -ErrorAction SilentlyContinue + # Removed with the pids: a stopped stack must not leave behind a file saying + # which database is up, or the next test run trusts a container that is gone. + Remove-Item $DatabaseFile -ErrorAction SilentlyContinue Write-Host '' Write-Host 'Stopped.' -ForegroundColor Green } @@ -377,6 +411,10 @@ Use-PinnedNode @NodeOut try { Assert-Docker Start-Database + # Recorded as soon as the database is actually up, and before anything reads + # it. Written after Start-Database rather than before, so the file never + # claims a database that failed to start. + Write-DatabaseChoice Install-IfMissing (Join-Path $RepoRoot 'backend') Install-IfMissing (Join-Path $RepoRoot 'frontend') Start-Backend