Skip to content

CI: run the Core tests, check vulnerable packages, and fail (not skip) when SQL Server is missing - #138

Merged
electricessence merged 7 commits into
masterfrom
ci/run-tests-and-require-sql
Nov 15, 2026
Merged

electricessence merged 7 commits into
masterfrom
ci/run-tests-and-require-sql

Conversation

@electricessence

Copy link
Copy Markdown
Collaborator

Pull requests now run the Core test suite in CI (net10.0 and net11.0 in Release, net11.0 in Debug) and fail when a package has a known vulnerability, and the tests that need a real SQL Server fail, instead of skipping, when CI cannot reach one, so a green run can no longer hide a suite that did not run.

This changes workflow files, so it needs the owner's merge.

What changed

  • New .github/workflows/tests.yml. Triggers: pull_request, push to master, workflow_dispatch. permissions: contents: read, the same env, checkout@v7 and setup-dotnet@v6 as docs.yml. One concurrency group per ref (tests-<ref>): a newer push to a pull request cancels that pull request's older run; a pull request's ref (refs/pull/N/merge) never equals master's, so a pull request run can never cancel a master run, and master and manual runs queue and are never cancelled.
    • Matrix (windows-latest, 30-minute timeout, fail-fast: false): net10.0 Release, net11.0 Release, net11.0 Debug. Each leg runs dotnet test Tests/Open.Database.Extensions.Core.Tests -c <configuration> -f <tfm> with a TRX logger and ODX_REQUIRE_SQL=1. TRX files are uploaded as artifacts (trx-<leg>), also when the tests fail.
    • SQL Server LocalDB. The runner image has the LocalDB runtime as a Visual Studio component. A step starts the automatic instance MSSQLLocalDB before the tests (a first start can outlast the fixture's 20-second connection timeout) and fails with a clear message if SqlLocalDB.exe is not on the image.
    • Vulnerable packages (separate job, 15-minute timeout): dotnet list <project> package --vulnerable --include-transitive for each library project and the test project. That command exits 0 even when it finds something, so the job reads its output and fails when it contains has the following vulnerable packages, when the command itself fails, or when the vulnerability data could not be read (NU1900). The full output is always printed.
  • docs.yml. The "Run the samples" step is no longer continue-on-error: a failing sample simply fails the run (the red step shows it), and the deploy job, which needs the build, does not run. The samples-failure block in the "Summarize warnings" step, which only repeated what the red step shows, is removed together with the step id it read; nothing else references that id. Nothing else changed.
  • Test harness. New environment variable ODX_REQUIRE_SQL (any non-empty value). In LocalDbFixture, when it is set and LocalDB cannot be reached, the tests that call SkipUnlessAvailable() fail with the reason instead of being skipped. When it is not set, nothing changes (skip). ODE_TEST_NO_LOCALDB (force a skip) does not turn a required run into a green skip: with both set the tests fail. The fixture's XML docs say so.

Verification

Local, on a machine with LocalDB, net11.0 Release, the 64 test cases of SqlServerPackageTests (42 of them call SkipUnlessAvailable()):

  • ODX_REQUIRE_SQL=1, LocalDB present: Passed 64, Failed 0, Skipped 0.
  • ODX_REQUIRE_SQL=1 and ODE_TEST_NO_LOCALDB=1: Failed 42, Passed 22, Skipped 0. Message: "ODX_REQUIRE_SQL is set, so a missing SQL Server is a failure, not a skip: ODE_TEST_NO_LOCALDB is set: ...".
  • ODE_TEST_NO_LOCALDB=1 only (how a machine without LocalDB behaves): Passed 22, Skipped 42, Failed 0, exactly as before.
  • With neither variable and LocalDB present, the live tests run and pass (as in the first run).
  • tests.yml and docs.yml parse as YAML; the PowerShell in every run: block parses; the LocalDB start step and the vulnerable-package step were run locally as written.

The branch has four commits on purpose; see the last section.

Verified locally

The vulnerable-package job's script (the run: block of the "List vulnerable packages" step) was run unchanged against three saved text files, with a stand-in for the dotnet command that prints the file and reports exit code 0 (as the real command does, which is why the script reads the text). The script is the workflow's text exactly: cmp of the extracted block against the de-indented run: lines of tests.yml reports no difference. It was run the way the runner runs a pwsh step: pwsh -Command ". '<script>'" with the runner's last line (if (Test-Path variable:\LASTEXITCODE) { exit $LASTEXITCODE }) appended. The fixtures are not committed.

  • (a) Vulnerable. A text in the shape the command prints for a project with a finding: the "The following sources were used" block, the line Project `<name>` has the following vulnerable packages, a [net10.0]: line, the table header Top-level Package Requested Resolved Severity Advisory URL and one row > Some.Package 1.0.0 1.0.0 High https://github.com/advisories/GHSA-xxxx-xxxx-xxxx. That shape is written from the command's documented output family (the .NET CLI docs show the same header and table layout for the package list command); it is not a capture of a real finding, so the exact header wording on a real finding is the one assumption here.
    Result: the text is printed, then one error per project, for example ::error title=Vulnerable packages::Open.Database.Extensions.Core.csproj: vulnerable packages found (listed above) (7 projects), exit code 1.
  • (b) Clean. A real capture of the command's output for a clean project: The given project `<name>` has no vulnerable packages given the current sources. (the same text the local run of the job printed for all 7 projects).
    Result: No vulnerable packages in 7 project(s)., exit code 0.
  • (c) NU1900. The message shape from the NuGet docs for warning NU1900 (warning NU1900: Error occurred while getting package vulnerability data: ...), followed by the "no vulnerable packages" line, which is the dangerous case: the command says clean because it could not look.
    Result: one error per project, for example ::error title=Vulnerable packages::Open.Database.Extensions.Core.csproj: the vulnerability data could not be read, so the check proved nothing (7 projects), exit code 1.

Not covered: the script was not run with the real command finding a real vulnerability, and a command that fails outright (non-zero exit) is handled by a separate branch of the script that was read, not run.

Not done / decisions for the owner

  • Commit B is red on purpose. The branch holds four commits: A (the workflows, the fixture and the docs.yml step), S (the two simplifications: the one concurrency group per ref and the removed samples-failure block), B (adds one test file, CiGuardProof.cs, with the single test CiGuardProof.ThisTestFailsOnPurpose, which calls Assert.Fail("deliberate: proves CI runs the tests")), and C (deletes that file). C has exactly the tree of S (git diff S C is empty). CI on S+B must be red, with that test named in the log; that is the proof that CI really runs the tests. CI on C must be green. The squash merge brings in neither the test nor any red state.
  • Cannot be verified locally: the workflow itself runs only on GitHub. Not run here: the matrix legs, the windows-latest LocalDB start, the artifact upload, and the vulnerability query against the live advisory feed on the runner. actionlint was not available offline, so the check was a YAML parse plus a PowerShell parse of each script block. No GitHub source states that (localdb)\MSSQLLocalDB was exercised on that image; the readme lists the component only.
  • Needs a decision when it goes red: if the vulnerable-package job reports an existing finding on the first run, the fix is a package update or an explicit, reviewed exception; this PR does not add an allow-list.
  • The Debug leg runs net11.0 only, as asked. No coverage, no Debug for net10.0.
  • Mark the PR ready only after the C run is green. Make Test net10.0-Release, Test net11.0-Release, Test net11.0-Debug and Vulnerable packages required checks in the branch protection when you want them to block merges (a repository setting, not part of this change).

🤖 Generated with Claude Code

electricessence and others added 6 commits November 14, 2026 15:48
…issing SQL Server fail there

New tests.yml: the Core suite on net10.0 Release, net11.0 Release and
net11.0 Debug on windows-latest, TRX files uploaded, SQL Server LocalDB
started first, and a job that fails when `dotnet list package
--vulnerable --include-transitive` reports anything.

ODX_REQUIRE_SQL=1 (set in that workflow) makes the tests that need LocalDB
fail instead of skip when it cannot be reached, also when
ODE_TEST_NO_LOCALDB forces a skip. Without it nothing changes.

docs.yml: a failing docs sample now fails the run.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
tests.yml: one concurrency group per ref. A pull request's ref never
equals master's, so the separate pull-request group was redundant.

docs.yml: drop the samples-failure block from the summary step and the
step id it read. A failing sample now simply fails the run and is shown
by the failed step itself.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Temporary: the next commit removes it. CI on this commit must be red with
CiGuardProof.ThisTestFailsOnPurpose named in the log.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Restores the tree of the first commit.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ework path

The "Run the samples" step ran docsite/samples/bin/Release/net10.0/Samples.dll,
but the samples project targets net11.0, so the file was not found. The step was
marked continue-on-error, which hid it. Run the project instead, so the target
framework comes from the project.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@electricessence
electricessence marked this pull request as ready for review November 15, 2026 03:33
@electricessence
electricessence merged commit 060d46c into master Nov 15, 2026
6 checks passed
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