Rewrite subsystem tests to exercise subsystem units directly

[Copilot]

Pull Request

Description

Subsystem test files were using Runner.Run() to spawn an external process against the compiled DLL — a black-box approach identical to integration tests that does not prove the subsystem units are sufficient to implement the subsystem requirements.

CliTests.cs — All Program.Run() / Program.Main() calls removed. Tests now exercise Context exclusively, proving the CLI unit satisfies subsystem requirements without reaching outside the subsystem:

• Cli_VersionFlag_SetsVersionFlag / Cli_HelpFlag_SetsHelpFlag — verify parsed flag properties on Context
• Cli_SilentFlag_SuppressesOutput — calls context.WriteLine and asserts nothing reaches Console.Out
• Cli_LogFile_WritesOutputToFile — calls context.WriteLine and asserts the log file receives content
• Cli_EnforceFlag_SetsEnforceFlag — verifies --enforce sets context.Enforce == true
• Cli_WriteError_SetsExitCodeToOne — verifies context.WriteError writes to stderr and sets ExitCode == 1 (separate from enforce-flag parsing)
• Cli_UnknownArgument_ThrowsArgumentException — Assert.ThrowsExactly<ArgumentException>(() => Context.Create([...])) directly
• Six new tests added for --validate, --sarif, --report, --report-depth, --heading, and --results

SarifTests.cs — SARIF tests call SarifResults.Read() and ToMarkdown() directly; exception paths use Assert.ThrowsExactly<T>(). Sarif_MissingSarifParameter_ShowsError removed (it called Program.Main, a Program-unit concern already covered by integration tests). Three tests renamed to reflect exception-based behavior with guaranteed-nonexistent temp paths:

• Sarif_NonExistentSarifFile_ThrowsFileNotFoundException
• Sarif_GenerateReport_ProducesMarkdownContent
• Sarif_InvalidSarifFile_ThrowsInvalidOperationException

SelfTestTests.cs — All validate tests use Context.Create(["--validate"]) + Validation.Run(context) directly. SelfTest_EnforcementTest_RunsWithinValidation calls Validation.Run(context) and asserts "SarifMark_Enforcement - Passed" in the output, proving the Validation unit exercises its internal enforcement test.

docs/reqstream/sarifmark/cli/cli.yaml — Requirement titles corrected to describe CLI-level services (not system-level claims); section renamed to "CLI Subsystem"; SarifMark-Cli-Enforce now covers only the --enforce flag parsing; new SarifMark-Cli-WriteError requirement added for error output and exit-code behavior; SarifMark-Cli-InvalidArgs test ref updated to Cli_UnknownArgument_ThrowsArgumentException; six missing subsystem requirements added (SarifMark-Cli-Validate, SarifMark-Cli-Sarif, SarifMark-Cli-Report, SarifMark-Cli-ReportDepth, SarifMark-Cli-Heading, SarifMark-Cli-Results) to document the full interface the CLI subsystem exposes.

docs/reqstream/sarifmark/sarif/sarif.yaml — Removed SarifMark-Sarif-Required (a Program-level concern misplaced in the SARIF subsystem); test name refs updated to match renamed tests; SarifMark-Sarif-FilePaths cleaned up.

docs/reqstream/sarifmark/sarifmark.yaml — Added SarifMark-System-SarifRequired at the correct system level, linked to IntegrationTest_MissingSarifParameter_ShowsError.

docs/reqstream/sarifmark/sarif/report.yaml — Updated test name ref to Sarif_GenerateReport_ProducesMarkdownContent.

docs/reqstream/sarifmark/self-test/self-test.yaml — Updated SarifMark-Enforce-Mode and SarifMark-Enforce-ExitCode to reference the renamed test method SelfTest_EnforcementTest_RunsWithinValidation.

docs/design/sarifmark/cli/cli.md — Interfaces section updated to document all parsed-flag properties (Version, Help, Silent, Validate, Enforce, SarifFile, ReportFile, ReportDepth, Heading, ResultsFile, ExitCode) — the primary data the CLI subsystem exposes to the application layer.

docs/design/sarifmark/sarif/sarif.md — Requirement reference list and CLI Integration note updated to reflect removal of SarifMark-Sarif-Required.

UtilitiesTests.cs — already correct; no changes.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code quality improvement

Related Issues

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

• Code builds successfully: dotnet build --configuration Release
• All unit tests pass: dotnet test --configuration Release
• Self-validation tests pass:
  dotnet run --project src/DemaConsulting.SarifMark --configuration Release --framework net10.0
--no-build -- --validate
• Code produces zero warnings

Code Quality

• Code formatting is correct: dotnet format --verify-no-changes
• New code has appropriate XML documentation comments
• Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

• All linters pass: ./lint.sh (Unix/macOS) or cmd /c lint.bat / ./lint.bat (Windows)

Testing

• Added unit tests for new functionality
• Updated existing tests if behavior changed
• All tests follow the AAA (Arrange, Act, Assert) pattern
• Test coverage is maintained or improved

Documentation

• Updated README.md (if applicable)
• Updated ARCHITECTURE.md (if applicable)
• Added code examples for new features (if applicable)
• Updated requirements.yaml (if applicable)

Additional Notes

134 tests pass across net8.0/net9.0/net10.0. CodeQL and code review clean.

[Copilot]

Pull request overview

Reworks subsystem-level tests to avoid spawning dotnet processes and instead exercise CLI and SARIF/self-test units in-process (via Program.Main, Context.Create, Validation.Run, and SarifResults.Read/ToMarkdown).

Changes:

• Updated CLI subsystem tests to call Program.Main(...) with captured Console.Out/Console.Error instead of Runner.Run(...).
• Updated SARIF subsystem tests to validate SarifResults.Read(...) and ToMarkdown(...) output directly, using exception assertions for error cases.
• Updated self-test subsystem tests to run validation via Context.Create(...) + Validation.Run(...), retaining CLI-level enforcement coverage via Program.Main(...).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs	Switches CLI subsystem tests from external process execution to in-process Program.Main with console capture.
test/DemaConsulting.SarifMark.Tests/Sarif/SarifTests.cs	Switches SARIF tests to direct unit calls (SarifResults.Read/ToMarkdown) and exception-based assertions.
test/DemaConsulting.SarifMark.Tests/SelfTest/SelfTestTests.cs	Switches self-validation tests to direct Context/Validation execution; keeps enforcement path via Program.Main.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Malcolmnixon]

@copilot The CLI subsystem tests are supposed to prove the CLI units can come together to meet the CLI subsystem requirements. Why then is it calling Program.Main? Is there an over-reach in the CLI subsystem requirements, or is there a missing test at the System level?

The same question arises in the SelfTest system tests. They are supposed to be proving the SelfTest units can come together to satisfy the SelfTest system requirements. It shouldn't be calling Program.Main - that suggests either an over-reach in the SelfTest requirements, or a missing test at the System level.

[Malcolmnixon]

@copilot apply changes based on the comments in this thread

[Malcolmnixon]

@copilot NO! Why do you insist on having Subsystem tests reach up and run the program. The Cli subsystem tests should ONLY use stuff under the CLI. The CLI is supposed to provide services that "could be" used to make an application, but they ABSOLUTELY SHOULD NOT run the application. If something is driving you to think that the CLI subsystem tests absolutely have to run the program then something is bad with the requirements - if a CLI subsystem requirement is making claims about the system or application then it's wrong.

[Copilot]

@copilot The CLI subsystem tests are supposed to prove the CLI units can come together to meet the CLI subsystem requirements. Why then is it calling Program.Main? Is there an over-reach in the CLI su...

Both observations are correct — this was an over-reach in both cases.

CLI subsystem tests (commit 1f6fdec): Replaced Program.Main(args) with Context.Create(args) + Program.Run(context). Program.Main is the application entry point (includes exception handling); Program.Run(context) is the execution method that the CLI unit (Context) is passed to. Cli_UnknownArgument_ShowsError now uses Assert.ThrowsExactly<ArgumentException>(() => Context.Create([...])) directly, since argument rejection is a Context behavior (and Program.Main's only role there was to catch and display the exception).

SelfTest subsystem SelfTest_EnforceFlag_ReturnsNonZeroOnIssues (commit 51ed22e): This test was calling Program.Main(["--sarif", sarifFile, "--enforce"]) — testing SARIF analysis enforcement, which is entirely unrelated to Validation. The requirement SarifMark-Enforce-Mode has child SarifMark-Validation-EnforcementTest, which refers to Validation.RunEnforcementTest. The fix is to call Validation.Run(context) and assert "SarifMark_Enforcement - Passed" in the output — confirming that the Validation unit specifically ran and passed its enforcement test. The test was also renamed to SelfTest_EnforcementTest_RunsWithinValidation to match the actual behavior.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js ost:*;127.0.0.1;install (http block)
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js -m conntrack --ctstate INVALID,NEW -j DROP ame (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Malcolmnixon]

@copilot Why do you insist on having Subsystem tests reach up and run the program. The Cli subsystem tests should ONLY use stuff under the CLI. The CLI is supposed to provide services that "could be" used to make an application, but they ABSOLUTELY SHOULD NOT run the application. If something is driving you to think that the CLI subsystem tests absolutely have to run the program then something is bad with the requirements - if a CLI subsystem requirement is making claims about the system or application then it's wrong. If there's missing tests at the system or program level that make you think you have to cover the gap here, then those missing tests elsewhere are the problem.

I need to know why you insist on calling the Program class (an entity outside the CLI subsystem) and we need to fix that problem.

[Copilot]

@copilot Why do you insist on having Subsystem tests reach up and run the program. The Cli subsystem tests should ONLY use stuff under the CLI. The CLI is supposed to provide services that "could be" ...

The root cause was over-reaching subsystem requirements that made system-level claims, which drove tests to call Program. Fixed in commits 79794da/79db560:

• CliTests.cs: all Program.Run() calls removed — tests now use only Context (e.g. Context.Create, context.WriteLine, context.WriteError, context.ExitCode).
• cli.yaml: requirement titles rewritten to describe what the CLI subsystem provides (e.g. "The CLI shall recognize the --version flag") instead of system behaviors; 6 missing requirements added for --validate, --sarif, --report, --report-depth, --heading, --results.
• sarif.yaml: removed SarifMark-Sarif-Required (Program-level concern with a wrong-level test calling Program.Main); moved to sarifmark.yaml as SarifMark-System-SarifRequired with IntegrationTest_MissingSarifParameter_ShowsError.
• cli.md: updated Interfaces section to document all parsed-flag properties the subsystem exposes.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Malcolmnixon]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Applied in commit f41e0df:

• Cli_EnforceFlagWithIssues_ReturnsError split into Cli_EnforceFlag_SetsEnforceFlag and Cli_WriteError_SetsExitCodeToOne
• Cli_UnknownArgument_ShowsError renamed to Cli_UnknownArgument_ThrowsArgumentException
• cli.yaml updated with split requirements and corrected test refs

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.