Add IntegrationTest_* and Cli_* tests; update reqstream YAML for test-linkage compliance#46
Conversation
Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/5fe50d02-c30f-45d3-a137-438007a8d9b6 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…or test-linkage compliance - Add 7 new IntegrationTest_* methods to IntegrationTests.cs covering: ReviewPlanGeneration, ReviewReportGeneration, Enforce, IndexScan, WorkingDirectoryOverride, Elaborate, Lint - Add 13 new Cli_* methods to CliTests.cs covering: ResultsFlag, LogFlag, ErrorOutput, InvalidArgs, ExitCode, DefinitionFlag, PlanFlag, ReportFlag, EnforceFlag, DirFlag, ElaborateFlag, LintFlag, IndexFlag - Update docs/reqstream/review-mark/review-mark.yaml: system requirements now use only IntegrationTest_* tests and add children: links to subsystem IDs - Update docs/reqstream/review-mark/cli/cli.yaml: subsystem requirements now use only Cli_* tests; add new requirements for all CLI flags - Update docs/reqstream/review-mark/configuration/configuration.yaml: subsystem requirements use only Configuration_* tests - Update docs/reqstream/review-mark/indexing/indexing.yaml: subsystem requirements use only Indexing_* tests - Update docs/reqstream/review-mark/self-test/self-test.yaml: subsystem requirements use only SelfTest_* tests - Update docs/reqstream/review-mark/configuration/review-mark-configuration.yaml: remove Program_* test from ReviewMark-Config-Loading - Update docs/reqstream/review-mark/indexing/review-index.yaml: remove ReviewMarkConfiguration_* tests from ReviewMark-EvidenceSource-None - Update .reviewmark.yaml: add Purpose review; update ReviewMark-Architecture, ReviewMark-Design, ReviewMark-AllRequirements; remove user_guide and TestDirectory.cs from ReviewMark-Program review Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…eqs, reorder review-sets Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds new system-level (IntegrationTest_*) and CLI-subsystem (Cli_*) tests and restructures ReqStream/ReviewMark YAML so requirements link to tests at the correct hierarchy level (system → integration tests, subsystem → subsystem tests, unit → unit tests).
Changes:
- Added 7 new
IntegrationTest_*tests covering major CLI flows (plan/report/enforce/index/dir/elaborate/lint). - Added 13 new
Cli_*subsystem tests covering key CLI flags and behaviors. - Updated ReqStream requirements YAML and
.reviewmark.yamlreview-set organization to enforce test-linkage hierarchy and consistent review scoping.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.ReviewMark.Tests/IntegrationTests.cs | Adds new end-to-end integration tests for core CLI flows. |
| test/DemaConsulting.ReviewMark.Tests/Cli/CliTests.cs | Adds CLI subsystem tests for flags, file outputs, and error handling. |
| docs/reqstream/review-mark/self-test/self-test.yaml | Adjusts subsystem requirement test linkages to SelfTest_* tests only. |
| docs/reqstream/review-mark/review-mark.yaml | Updates system requirements to link only to IntegrationTest_* and adds children links. |
| docs/reqstream/review-mark/indexing/review-index.yaml | Removes mis-scoped configuration tests from indexing unit requirement. |
| docs/reqstream/review-mark/indexing/indexing.yaml | Updates indexing subsystem requirements to link only to Indexing_* tests and adds children. |
| docs/reqstream/review-mark/configuration/review-mark-configuration.yaml | Removes a mis-scoped Program_* test from configuration unit requirement. |
| docs/reqstream/review-mark/configuration/configuration.yaml | Updates configuration subsystem requirement test linkages and children. |
| docs/reqstream/review-mark/cli/cli.yaml | Updates CLI subsystem requirement test linkages to Cli_* tests and trims unit-test references. |
| .reviewmark.yaml | Reorganizes review-sets, adds Purpose, and aligns hierarchy/scoping with updated standards. |
| .github/standards/software-items.md | Adds naming guidance for hierarchy collisions. |
| .github/standards/reviewmark-usage.md | Updates usage examples and documents review-set design principles/hierarchy. |
| .github/standards/reqstream-usage.md | Documents test-linkage hierarchy and adds children field guidance. |
| .github/standards/csharp-testing.md | Updates AAA comment style and adds hierarchical test naming standards. |
| .github/agents/repo-consistency.agent.md | Converts template placeholders to {...} format and fixes indentation. |
| .github/agents/quality.agent.md | Converts template placeholders to {...} format and updates checklist wording. |
| .github/agents/implementation.agent.md | Converts template placeholders to {...} format and fixes indentation. |
| .github/agents/developer.agent.md | Converts template placeholders to {...} format and fixes indentation. |
| .github/agents/code-review.agent.md | Updates elaborate syntax example and converts placeholders to {...} format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot The DemaConsulting.ReviewMark.Tests.Cli.CliTests.Cli_LogFlag_WritesOutputToFile test is failing on Windows because a file is still in use. I think the test isn't disposing of the context before accessing the file. Please fix this and any other tests with the same issue. |
…putToFile Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/faf4bc52-4fa8-491e-a753-863b18c1e6a6 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in ebc7068. The test was using Changed to |
|
@copilot apply changes based on the comments in this thread |
…th tests, traceability tests Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/bb1deea5-108a-46ac-9875-9199ef6993e9 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…pth assertions Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/bb1deea5-108a-46ac-9875-9199ef6993e9 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
All review thread feedback applied in 677d91a and 752acd9:
All 204 tests pass across net8.0, net9.0, net10.0. |
Adds new system-level (
IntegrationTest_*) and CLI-subsystem (Cli_*) tests and restructures ReqStream/ReviewMark YAML so requirements link to tests at the correct hierarchy level (system → integration tests, subsystem → subsystem tests, unit → unit tests).Description
IntegrationTest_*tests toIntegrationTests.cscovering major CLI flows (ReviewPlanGeneration, ReviewReportGeneration, Enforce, IndexScan, WorkingDirectoryOverride, Elaborate, Lint)Cli_*subsystem tests toCliTests.cscovering key CLI flags and behaviors (ResultsFlag, LogFlag, ErrorOutput, InvalidArgs, ExitCode, DefinitionFlag, PlanFlag, ReportFlag, IndexFlag, EnforceFlag, DirFlag, ElaborateFlag, LintFlag, PlanDepthFlag, ReportDepthFlag)Configuration_LoadConfig_ReportGenerationSucceedsandConfiguration_LoadConfig_ElaborationSucceedstests toConfigurationTests.csto correctly cover report generation and elaboration requirementsIndexing_SafePathCombine_WithTraversalInputs_Throwstest toIndexingTests.csto validate path traversal rejection at the subsystem levelreview-mark.yaml(system): link only toIntegrationTest_*tests; addedchildren:links to subsystem requirementscli.yaml(subsystem): linkPlanDepthrequirement toCli_PlanDepthFlag_SetsHeadingDepthandReportDepthrequirement toCli_ReportDepthFlag_SetsHeadingDepth; maintainedchildren:links to unit requirementsconfiguration.yaml(subsystem): linkedReportGenerationrequirement toConfiguration_LoadConfig_ReportGenerationSucceedsandElaborationrequirement toConfiguration_LoadConfig_ElaborationSucceedsindexing.yaml(subsystem): addedIndexing_SafePathCombine_WithTraversalInputs_ThrowstoSafePathCombinerequirementself-test.yaml(subsystem): link only toSelfTest_*tests; addedchildren:linksreview-mark-configuration.yaml: removedProgram_*test that doesn't belong at unit levelreview-index.yaml: removedReviewMarkConfiguration_*tests fromReviewMark-EvidenceSource-None.reviewmark.yaml: addedPurposereview; updated Architecture/Design/AllRequirements/subsystem/unit review paths to match new standard patternsCli_LogFlag_WritesOutputToFile: switched to block-scopedusingso theContext(and itsStreamWriterlog file handle) is disposed before the file is read or deleted, preventing a "file in use" failure on WindowsCli_ErrorOutput_WritesToStderr: rewrote to invokeProgram.Mainvia reflection and assert stderr containsError:and the unknown argument namePath.GetTempFileName()+Path.ChangeExtensiontemp-file leaks inCliTests.cs(7 occurrences) andIntegrationTests.cs(4 occurrences) by usingPath.GetRandomFileName()/Guid.NewGuid()patterns that never pre-create a file on diskType of Change
Pre-Submission Checklist
Build and Test
dotnet build --configuration Releasedotnet test --configuration ReleaseCode Quality
dotnet format --verify-no-changesQuality Checks
cspell "**/*.{md,cs}"markdownlint "**/*.md"yamllint .Testing
Documentation
Additional Notes
All 204 tests pass across net8.0, net9.0, and net10.0. Platform requirements (
platform-requirements.yaml) retainReviewMark_*self-validation tests with source filters — this is intentional as those are the platform qualification mechanism, not regular integration tests.