Merge pull request 'fix(scripts): make the database the e2e suite reads a recorded fact (#273)' (#291) from fix/273-record-the-database into main

Reviewed-on: #291

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([]);
+  });
+});

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'

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