Repository navigation
CI: run the Core tests, check vulnerable packages, and fail (not skip) when SQL Server is missing - #138
Merged
Conversation
…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
marked this pull request as ready for review
November 15, 2026 03:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
.github/workflows/tests.yml. Triggers:pull_request, push tomaster,workflow_dispatch.permissions: contents: read, the sameenv,checkout@v7andsetup-dotnet@v6asdocs.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.windows-latest, 30-minute timeout,fail-fast: false):net10.0Release,net11.0Release,net11.0Debug. Each leg runsdotnet test Tests/Open.Database.Extensions.Core.Tests -c <configuration> -f <tfm>with a TRX logger andODX_REQUIRE_SQL=1. TRX files are uploaded as artifacts (trx-<leg>), also when the tests fail.MSSQLLocalDBbefore the tests (a first start can outlast the fixture's 20-second connection timeout) and fails with a clear message ifSqlLocalDB.exeis not on the image.dotnet list <project> package --vulnerable --include-transitivefor 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 containshas 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 longercontinue-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.ODX_REQUIRE_SQL(any non-empty value). InLocalDbFixture, when it is set and LocalDB cannot be reached, the tests that callSkipUnlessAvailable()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 callSkipUnlessAvailable()):ODX_REQUIRE_SQL=1, LocalDB present: Passed 64, Failed 0, Skipped 0.ODX_REQUIRE_SQL=1andODE_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=1only (how a machine without LocalDB behaves): Passed 22, Skipped 42, Failed 0, exactly as before.tests.ymlanddocs.ymlparse as YAML; the PowerShell in everyrun: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 thedotnetcommand 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:cmpof the extracted block against the de-indentedrun:lines oftests.ymlreports no difference. It was run the way the runner runs apwshstep:pwsh -Command ". '<script>'"with the runner's last line (if (Test-Path variable:\LASTEXITCODE) { exit $LASTEXITCODE }) appended. The fixtures are not committed.Project `<name>` has the following vulnerable packages, a[net10.0]:line, the table headerTop-level Package Requested Resolved Severity Advisory URLand 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.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.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
CiGuardProof.cs, with the single testCiGuardProof.ThisTestFailsOnPurpose, which callsAssert.Fail("deliberate: proves CI runs the tests")), and C (deletes that file). C has exactly the tree of S (git diff S Cis 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.windows-latestLocalDB start, the artifact upload, and the vulnerability query against the live advisory feed on the runner.actionlintwas not available offline, so the check was a YAML parse plus a PowerShell parse of each script block. No GitHub source states that(localdb)\MSSQLLocalDBwas exercised on that image; the readme lists the component only.Test net10.0-Release,Test net11.0-Release,Test net11.0-DebugandVulnerable packagesrequired checks in the branch protection when you want them to block merges (a repository setting, not part of this change).🤖 Generated with Claude Code