Skip to content

test(compliance): make ComplianceRunner.Main observable - #7

Merged
sakanni merged 2 commits into
developfrom
test/compliance-main-observable
Aug 24, 2026
Merged

test(compliance): make ComplianceRunner.Main observable#7
sakanni merged 2 commits into
developfrom
test/compliance-main-observable

Conversation

@sakanni

@sakanni sakanni commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

#6 made ComplianceRunner's helper layer testable. Its entry point still had no
coverage.

ComplianceRunner.Main compiles against BHoM types in every branch — TestResult,
TestStatus, ITestInformation, BH.Engine.Test.CodeCompliance.Compute,
BH.Engine.Base.Query. So even the usage-and-exit path, which touches no BHoM type
at runtime, cannot be reached in-process without them. No test project referenced the
runner executables, and the only tests that exercised Main were the integration
files that do not run.

That left the runner's file accounting and exit code unobservable: the paths that
decide whether a compliance check passes.

What changed

Two new test files, using process invocation via the existing RunnerFixture,
which is already the convention for the integration tests and needs no production
change:

  • MainAccountingTests — the [SKIP] accounting for a relevant file absent from
    disk, the exit-code mapping, and the behaviour when every candidate file is missing.
  • PathspecFilterPairingTests — runs a real
    git diff --diff-filter=ACMRT HEAD^1 HEAD -- '*AssemblyInfo.cs' against
    FileFilter.IsRelevantFile in one test, which no single-layer suite can do.

A second workflow job that resolves the Test_Toolkit dependency graph before
testing, so Compliance.Tests compiles. This also un-inerts the six test files
already there. It uses resolve-dependencies in mode: seeds, the same mechanism
ci-compliance uses before publishing the runner, and asserts that
CodeComplianceTest_oM.dll is present afterwards rather than letting its absence
surface later as a bare CS0234.

Kept as a separate job from the hermetic suite, so a resolve-dependencies breakage
cannot also suppress the runner tests that would catch it — it is one of the actions
this repository ships. It uses the local ./.github/actions/resolve-dependencies
path, so a PR tests its own copy rather than the published one.

A build step the fixture always needed. RunnerFixture invokes runners with
dotnet run --no-build, but Compliance.Tests references neither executable
project, so dotnet test never built them. RunnerFixture.cs claims the solution
"is built automatically when running via the test project"; it is not. The first run
of this job failed 14 tests with exit 1 and empty stdout for that reason alone.
Three of the existing tests, which assert exit code 1 for the usage path, had been
passing for the wrong reason — a missing binary also exits 1.

What the tests capture

Some assert behaviour that is arguably wrong, deliberately, so that a change to it is
visible:

  • The runner exits 0 having examined nothing. Give it three relevant filenames
    that are not on disk and it reports success with status Pass and no annotation.
    Whether that should fail, warn, or stay as-is is an open question; the test
    documents today's answer and is commented with what it should become under each.
  • The changed-file pathspec and the runner's own filter disagree. The template
    pathspec *AssemblyInfo.cs selects Properties/NotAssemblyInfo.cs; the runner's
    FileFilter requires the filename to equal AssemblyInfo.cs exactly, so it is
    counted, handed to the runner, and silently discarded. Both halves are individually
    tested and individually defensible. Nothing tested them together until now.
  • [SKIP] diagnostics pollute machine-readable output. The skip line is an
    unconditional Console.WriteLine, not gated on the verbose flag the console format
    sets, so under --output json it precedes the payload and the raw stdout does not
    parse. Unaffected in production, where ci-compliance uses --output github.

Verification

114 tests, all passing, on a clean windows-latest runner with no BHoM install
before the resolve step.

CI on this PR:
ComplianceRunner tests ·
dotnet tests

No runner behaviour changed.

Relationship to #6

The two changes are semantically compatible and can merge in either order: this one
adds tests to Compliance.Tests and a job that lets them run, #6 splits the BHoM-free
tests into a separate project.

They are not conflict-free, though. Both append to test-tools.yml at the same
anchor, immediately after the VersioningRunner tests step, so whichever merges
second needs a resolution there. An earlier version of this section said this branch
touches only Compliance.Tests, which was wrong.

What to look at

  • The new job's 45-minute timeout is a guess. Observed wall clock is a few minutes;
    worth recalibrating after a handful of real runs.
  • RunnerFixture.cs's comment about automatic building is still wrong. Left alone
    here to keep the diff to one concern.
  • DatasetTestRunner is in no solution and still has no tests.

The entry point had no coverage reachable from the hermetic suite. Every branch of
Main compiles against BHoM types, so it cannot be unit-tested in-process without
restructuring the runner. Process invocation via the existing RunnerFixture observes
it without touching production code.

MainAccountingTests characterises the file accounting and the exit code, including
the path where every relevant file is absent and the runner still exits 0 with
status Pass. PathspecFilterPairingTests pairs a template pathspec against FileFilter
in one test, which nothing else does: the pathspec matches any name ending in
AssemblyInfo.cs, the filter requires the name to equal it. Both assert current
behaviour on purpose, and record what each assertion should become if that
behaviour is changed.

A second workflow job resolves the Test_Toolkit dependency graph so Compliance.Tests
compiles, which also un-inerts the six test files already there. resolve-dependencies
in mode: seeds is the same mechanism ci-compliance uses, and it does produce
CodeComplianceTest_oM.dll. Kept as a separate job so a resolve-dependencies breakage
cannot suppress the runner tests that would catch it.

The job also builds the runners before testing. RunnerFixture uses
'dotnet run --no-build' but Compliance.Tests references neither executable project,
so dotnet test never built them; the first validation run failed 14 tests with exit 1
and empty stdout for that reason alone.

114 tests pass on a bare runner with no BHoM install. No runner behaviour changed.
The characterisation tests were named and annotated against notes kept outside
this repository, so a reader here could not tell what an assertion was pinning
down or what would change it.

Method names now state what they assert. Comments on assertions that are
expected to change state the condition and the resulting assertion in their own
terms, rather than pointing at a decision recorded elsewhere. The workflow
comment explaining the longer timeout now gives the reason: the assembly cache
is keyed per dependency SHA set, so a new set builds the whole graph from
source.

Comments explaining behaviour are unchanged. No assertion, fixture or workflow
step changed, so the suite runs exactly as before.
@sakanni
sakanni force-pushed the test/compliance-main-observable branch from 57e61bd to 997f5fb Compare August 24, 2026 13:53
@sakanni
sakanni merged commit 8833b63 into develop Aug 24, 2026
5 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