Skip to content

fix: validate missing child requirement references and fix test compilation errors#156

Merged
Malcolmnixon merged 5 commits intomainfrom
copilot/identify-review-sets-and-fix-issues
Apr 3, 2026
Merged

fix: validate missing child requirement references and fix test compilation errors#156
Malcolmnixon merged 5 commits intomainfrom
copilot/identify-review-sets-and-fix-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 3, 2026

Pull Request

Description

Performed formal reviews of all 12 review-sets defined in .reviewmark.yaml using the code-review agent. Found and fixed critical issues discovered during the reviews.

1. Build failure — invalid property reference in linter test (Critical)

  • Problem: A linter test referenced issue.Message which does not exist on LintIssue — the correct property is issue.Description. This caused a CS1061 compilation error across all target frameworks.
  • Problem: The test had no assertions (SonarAnalyzer S2699), was placed in the wrong folder (Tracing/) with a non-standard filename, and used an incorrect naming prefix.
  • Fix: Moved the test into test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoaderTests.cs as RequirementsLoader_Load_WithUnknownChildReference_ReportsError, following the ClassName_Method_Scenario_Result naming convention used across the Modeling test files and correct Modeling subsystem placement. Removed TestMissingChild.cs entirely.

2. Missing child requirement reference validation (High)

  • Problem: RequirementsLoader.ValidateCyclesFrom silently ignored child requirement IDs that don't exist in the requirements tree. A typo in a children: list would go undetected, causing silent failures in trace matrix generation.
  • Fix: Added validation in ValidateCyclesFrom to emit an error-level LintIssue when a child requirement ID is not found. Simplified the implementation to use a single TryGetValue call (eliminating a redundant ContainsKey + later TryGetValue double-lookup).

3. Pre-existing test used undefined child IDs (Medium)

  • Problem: Requirements_Load_RequirementWithChildren_ParsesChildrenCorrectly referenced child IDs AUTH-001 and AUTH-002 that were not defined as requirements in the test YAML, which now (correctly) fails validation.
  • Fix: Added the missing child requirement definitions to the test YAML.

4. Test naming convention corrected across RequirementsLoaderTests.cs (Code Quality)

  • Problem: All 35 tests in RequirementsLoaderTests.cs used the incorrect Linter_Lint_* / Linter_ProgramRun_* prefix, which references a non-existent subsystem and unit.
  • Fix: Renamed all tests to follow the correct ClassName_Method_Scenario_Result convention: Linter_Lint_*RequirementsLoader_Load_* and Linter_ProgramRun_WithLintFlag_RunsLinterProgram_Run_WithLintFlag_RunsLinter. Updated all corresponding test name references in docs/reqstream/modeling/requirements.yaml and docs/reqstream/program.yaml.

5. New requirement added

  • Added ReqStream-Lint-UnknownChildReference to docs/reqstream/modeling/requirements.yaml to formally capture the new validation behaviour with traceability to RequirementsLoader_Load_WithUnknownChildReference_ReportsError.

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.ReqStream --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:

  • Spell checker passes: cspell "**/*.{md,cs}"
  • Markdown linter passes: markdownlint "**/*.md"
  • YAML linter passes: yamllint .

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

  • Build: ✅ 0 warnings, 0 errors across net8.0/net9.0/net10.0
  • Tests: ✅ 214/214 passing across all target frameworks

Files changed:

  • src/DemaConsulting.ReqStream/Modeling/RequirementsLoader.cs — add unknown child reference validation; simplify ValidateCyclesFrom to single TryGetValue
  • test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoaderTests.cs — rename all tests to ClassName_Method_* convention; add RequirementsLoader_Load_WithUnknownChildReference_ReportsError with specific assertions
  • test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoadParsingTests.cs — fix test to include child requirement definitions
  • docs/reqstream/modeling/requirements.yaml — add ReqStream-Lint-UnknownChildReference requirement; update all test name references
  • docs/reqstream/program.yaml — update Program_Run_WithLintFlag_RunsLinter test name reference

Copilot AI and others added 2 commits April 3, 2026 01:15
Agent-Logs-Url: https://github.com/demaconsulting/ReqStream/sessions/7c60a53b-cbb6-499a-94b9-0787e3c5e691

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…ion errors

Agent-Logs-Url: https://github.com/demaconsulting/ReqStream/sessions/7c60a53b-cbb6-499a-94b9-0787e3c5e691

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…e TestMissingChild.cs

Agent-Logs-Url: https://github.com/demaconsulting/ReqStream/sessions/46a3be6f-2b81-42bb-9bd3-e1f94a679047

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…rence_ReportsError

Agent-Logs-Url: https://github.com/demaconsulting/ReqStream/sessions/0a2bb730-b79a-40a4-a673-fc9ba9b9b2f8

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
@Malcolmnixon Malcolmnixon marked this pull request as ready for review April 3, 2026 01:36
Copilot AI review requested due to automatic review settings April 3, 2026 01:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves requirements model validation by detecting and reporting references to non-existent child requirement IDs during load/lint, and updates unit tests and formal requirements to match the new behavior.

Changes:

  • Add error-level linting for unknown child requirement references during cycle validation.
  • Update/add unit tests to cover the new validation behavior and fix previously-invalid test YAML.
  • Add a formal requirement documenting the new lint behavior and linking it to the corresponding test.

Reviewed changes

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

File Description
src/DemaConsulting.ReqStream/Modeling/RequirementsLoader.cs Emits an error LintIssue when a requirement’s children: list references an ID not present in the loaded requirements.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoaderTests.cs Adds a regression test ensuring unknown child references produce a lint error.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoadParsingTests.cs Fixes YAML test data by defining the previously-referenced child requirement IDs.
docs/reqstream/modeling/requirements.yaml Adds ReqStream-Lint-UnknownChildReference requirement documenting the new validation behavior and mapping to the new test.

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

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread

…ValidateCyclesFrom, improve assertions

Agent-Logs-Url: https://github.com/demaconsulting/ReqStream/sessions/1ee16f98-852e-4dab-96fb-0f4d4543adea

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 3, 2026

@copilot apply changes based on the comments in this thread

Applied in 315c3f5: all Linter_Lint_* tests renamed to use correct ClassName_Method_* convention, ValidateCyclesFrom simplified to a single TryGetValue (eliminates the redundant ContainsKey + later TryGetValue), and the unknown-child test now asserts for "PARENT", "NONEXISTENT", and "unknown child" in the error output.

@Malcolmnixon Malcolmnixon merged commit d6853da into main Apr 3, 2026
15 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.

3 participants