Skip to content

Run the database provider tests in CI against PostgreSQL - #409

Merged
Sev7eNup merged 1 commit into
mainfrom
ci/database-provider-tests
Sep 22, 2026
Merged

Sev7eNup merged 1 commit into
mainfrom
ci/database-provider-tests

Conversation

@Sev7eNup

Copy link
Copy Markdown
Owner

Closes the CI half of #379.

The DatabaseIntegration category covers the hand-written per-provider SQL — the atomic dispatch claim (FOR UPDATE SKIP LOCKED / UPDLOCK,READPAST,READCOMMITTEDLOCK), the consolidated InitialBaseline migration, startup recovery, and the dashboard duration trend with its three dialects. Each of those tests calls Assert.Skip without a server, and CI set neither NODEPILOT_TEST_POSTGRES nor NODEPILOT_TEST_SQLSERVER. So the PostgreSQL branch, which is the production default, was executed by nothing unless somebody remembered to start Docker locally.

The new database job starts the PostgreSQL the Windows runner already ships (installed, stopped, password root) and points the environment variable at it. Each fixture still creates its own nodepilot_test_<guid> database, so the admin connection only ever creates and drops those.

A job of its own rather than a step inside backend, so a database that fails to start reports as a database failure instead of hiding in the coverage run. It is wired into ci-gate and follows the same changes.code condition as the other code jobs.

SQL Server is not on the Windows image, so its InlineData cases keep skipping and stay a local concern through scripts/Test-DatabaseProviders.ps1. CONTRIBUTING.md says so where the script is documented.

The DatabaseIntegration category covers the hand-written per-provider SQL: the atomic dispatch claim, the consolidated baseline migration, startup recovery and the dashboard duration trend. Every one of those tests skips itself without a server, and CI set none, so the PostgreSQL branch - the production default - was executed by nothing until somebody remembered to start Docker locally.

The Windows runner ships PostgreSQL installed but stopped. The new job starts it and points NODEPILOT_TEST_POSTGRES at it. It is a job of its own so a database that fails to start reports as a database failure rather than hiding inside the coverage run. SQL Server is not on that image, so its cases keep skipping there and stay a local concern.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Sev7eNup
Sev7eNup force-pushed the ci/database-provider-tests branch from 8769f15 to e9e04f4 Compare September 22, 2026 21:56
@Sev7eNup
Sev7eNup merged commit 1e46d12 into main Sep 22, 2026
17 checks passed
@Sev7eNup
Sev7eNup deleted the ci/database-provider-tests branch September 22, 2026 22:05
Sev7eNup added a commit that referenced this pull request Sep 27, 2026
…eck)

Automated documentation-drift sync. Both claims were verified against the
checked-out code on main, not inferred from a diff.

CLAUDE.md - markdown exception lists the skipped CI jobs (from #409)
  The sentence enumerates which jobs a markdown-only PR skips, and named
  Frontend, Desktop and E2E. #409 added the `database` job, gated on the same
  `needs.changes.outputs.code == 'true'` as those three, so it skips with them.
  Database is added to the list.

docs/claude-reference.md - RestApi:AllowedHosts address rule (from #406)
  The row is specifically about targets that go through a proxy, and stated
  that one usable address suffices when a host resolves to several, citing
  NetworkGuard.AssertAnyAddressUsable and the per-address re-check in
  EnforceConnect. #406 inverted exactly that case: the method is now
  AssertAddressesUsable, and for a proxied destination every resolved address
  must pass, because the proxy resolves the destination DNS itself and
  EnforceConnect never sees those addresses. A host whose DNS mixes a routable
  record with a link-local one is now rejected when proxied, so the row told
  an operator the opposite of what the guard does. The one-address rule is
  scoped to direct targets and the proxied rule spelled out, naming the
  requireAllAddresses flag and RestApiHttpClientProvider.UsesProxyForDestination.

Text only; no code, config or test changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0122Ukd5gDBxgLcUPeCPQn7W
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant