Skip to content

Sync TemplateDotNetLibrary agent/standards files and align repo to updated standards#137

Merged
Malcolmnixon merged 24 commits into
mainfrom
copilot/copy-updated-github-files
Apr 4, 2026
Merged

Sync TemplateDotNetLibrary agent/standards files and align repo to updated standards#137
Malcolmnixon merged 24 commits into
mainfrom
copilot/copy-updated-github-files

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 4, 2026

Pull Request

Description

Syncs this repo's agent/standards documentation with the updated TemplateDotNetLibrary baseline, then applies the new standards across requirements, design documentation, tests, and ReviewMark review-sets.

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
  • 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

Requirement ID contraction fixes:

  • Renamed -Ser--Serializer- and -Jun--JUnit- in req YAMLs and design docs
  • Renamed -Mdl--Model- in req YAMLs and design docs
  • Renamed -Run--Runtime- in platform-requirements.yaml

Requirements hierarchy restructure:

  • Established clear system/subsystem/unit hierarchy matching the codebase architecture
  • System (TestResultsLibrary) with platform/runtime requirements, three top-level model units (TestOutcome, TestResult, TestResults), and an IO subsystem with five units (Serializer, SerializerHelpers, TrxSerializer, JUnitSerializer)
  • Added system-level requirements (TestResultsLibrary-Architecture-*) with children links to subsystem/unit requirements
  • Added IO subsystem requirements with children links to IO unit requirements
  • Added serializer-helpers.yaml unit requirements file for the SerializerHelpers unit; added corresponding TestResults-IO-SerializerHelpers ReviewMark review-set
  • TestResults-SerializerHelpers-Utf8Encoding requirement evidenced by dedicated tests TrxSerializer_Serialize_IncludesXmlDeclarationWithUtf8Encoding and JUnitSerializer_Serialize_IncludesXmlDeclarationWithUtf8Encoding

Design documentation updates:

  • Updated test-results-library.md system design with In-Memory Model and IO Subsystem sections
  • Created new test-results.md (TestResults unit design) and io/serializer-helpers.md (SerializerHelpers unit design)
  • Updated introduction.md, io/io.md, io/serializer.md, and test-outcome.md to reflect the correct hierarchy

Test linkage fixes:

  • Fixed SerializerTests.cs Act comments (changed "identify" → "deserialize" to match actual calls)
  • Added setter tests for TestResult output/error properties to properly validate capture capability
  • Added TrxSerializer_Serialize_IncludesXmlDeclarationWithUtf8Encoding and JUnitSerializer_Serialize_IncludesXmlDeclarationWithUtf8Encoding tests to assert the serialized XML output contains a UTF-8 encoding declaration
  • Updated platform-requirements.yaml to link platform/runtime requirements to all five system-level TestResultsLibrary_* tests (FormatAgnosticModel, Serialize TRX/JUnit, Deserialize TRX/JUnit) with appropriate source filter prefixes (windows@, ubuntu@, macos@, net8.0@, net9.0@, net481@, net10.0@) instead of unit-level serializer tests

ReviewMark updates:

  • Added Purpose review-set; removed SerializerTests.cs from system-level TestResults-Architecture review-set
  • Added TestResults-IO-SerializerHelpers review-set covering serializer-helpers.yaml, serializer-helpers.md, and SerializerHelpers.cs
  • Commented out MSTest OTS requirement pending proper self-validation tests

Linting fixes:

  • Fixed yamllint comments-indentation errors in 4 OTS YAML files (mstest.yaml, sonarscanner.yaml, pandoctool.yaml, weasyprinttool.yaml) by removing commented-out requirements template blocks from inside list items (TODO comments retained)
  • Fixed cspell errors by changing British English spellings to American English: organisedorganized (×3 in introduction.md), initialisedinitialized (test-results.md), behaviourbehavior (test-results-library.yaml and TestResultsLibraryTests.cs)

Copilot AI and others added 3 commits April 4, 2026 11:30
- Change 1: Update all AAA comments in test files to use ':' separator
  instead of ' - ' and add descriptions to bare Act/Assert/combined comments
- Change 2: Update test-outcome.yaml and test-result.yaml to use
  unit-specific test references instead of cross-unit links
- Change 3: Update .reviewmark.yaml to add README.md, Purpose review,
  rename TestResults-System to TestResults-Architecture, and remove
  Platform-Runtime review

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
@Malcolmnixon Malcolmnixon marked this pull request as ready for review April 4, 2026 12:22
Copilot AI review requested due to automatic review settings April 4, 2026 12:22
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

Syncs this repo’s agent/standards documentation with the updated TemplateDotNetLibrary baseline, then applies the new standards by updating test AAA comment formatting, requirements→tests linkage, and ReviewMark review-set structure.

Changes:

  • Updated AAA section comment format across MSTest files to the Arrange: / Act: / Assert: style.
  • Adjusted ReqStream unit-requirement test linkages to remove cross-unit references.
  • Restructured .reviewmark.yaml to add a Purpose review-set, revise system-level review-sets, and include README.md in needs-review.

Reviewed changes

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

Show a summary per file
File Description
test/DemaConsulting.TestResults.Tests/TestResultTests.cs AAA comment formatting updates only.
test/DemaConsulting.TestResults.Tests/TestResultsTests.cs AAA comment formatting updates only.
test/DemaConsulting.TestResults.Tests/TestOutcomeTests.cs AAA comment formatting updates only (combined Act+Assert wording updated).
test/DemaConsulting.TestResults.Tests/IO/TrxSerializerTests.cs AAA/section comment formatting updates only.
test/DemaConsulting.TestResults.Tests/IO/TrxExampleTests.cs AAA/section comment formatting updates only.
test/DemaConsulting.TestResults.Tests/IO/SerializerTests.cs AAA comment formatting updates; some new Act comments are now inaccurate vs the code under test.
test/DemaConsulting.TestResults.Tests/IO/JUnitSerializerTests.cs AAA/section comment formatting updates only.
test/DemaConsulting.TestResults.Tests/IO/IOTests.cs AAA comment formatting updates only.
docs/reqstream/test-results/test-result.yaml Updated unit requirement test links to same-unit tests (but now appears misaligned with requirement wording).
docs/reqstream/test-results/test-outcome.yaml Removed cross-unit test links per updated linkage hierarchy guidance.
.reviewmark.yaml Added README.md to needs-review; introduced Purpose review-set; updated system-level review-set composition.
.github/standards/reviewmark-usage.md Updated ReviewMark guidance content and formatting; adds hierarchical review-set principles and patterns.
.github/standards/reqstream-usage.md Added explicit “Test Linkage Hierarchy” guidance.
.github/standards/csharp-testing.md Updated AAA comment format guidance to colon separators and combined Act+Assert guidance.
.github/agents/quality.agent.md Updated quality checklist wording for review-set updates.

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

Comment thread test/DemaConsulting.TestResults.Tests/IO/SerializerTests.cs Outdated
Comment thread test/DemaConsulting.TestResults.Tests/IO/SerializerTests.cs Outdated
Comment thread test/DemaConsulting.TestResults.Tests/IO/SerializerTests.cs Outdated
Comment thread test/DemaConsulting.TestResults.Tests/IO/SerializerTests.cs Outdated
Comment thread docs/reqstream/test-results/test-result.yaml Outdated
Comment thread docs/reqstream/test-results-library/test-result.yaml Outdated
Comment thread .reviewmark.yaml Outdated
@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread

…tests, and architecture review-set

Agent-Logs-Url: https://github.com/demaconsulting/TestResults/sessions/4dc7b958-5e0d-430c-a03e-deaf08dba906

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

Copilot AI commented Apr 4, 2026

@copilot apply changes based on the comments in this thread

Applied all review changes in d5235d4:

  • SerializerTests.cs: Fixed 4 Act comments that incorrectly said "identify" (changed to "deserialize") to match the actual Serializer.Deserialize(...) calls being tested.
  • TestResultTests.cs: Added 4 new setter tests (TestResult_SystemOutput_Set_RetainsValue, TestResult_SystemError_Set_RetainsValue, TestResult_ErrorMessage_Set_RetainsValue, TestResult_ErrorStackTrace_Set_RetainsValue) that assign values and verify they are retained, properly validating the capture capability.
  • test-result.yaml: Updated TestResults-Mdl-TestOutput and TestResults-Mdl-ErrorInfo to include the new setter tests alongside the default-value tests.
  • .reviewmark.yaml: Removed SerializerTests.cs from TestResults-Architecture — it's a unit-level test file and doesn't belong in a system-level review under the hierarchical scope principle.

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

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.


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

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot The requirements hierarchy is weird.

The SYSTEM is this TestResults-Library (a thing that can read and write test results to different files.
The SYSTEM has PLATFORM requirements (windows, linux, mac, net8, net9, net10, netstandard2)
There are top-level UNITS (TestOutcome, TestResult, TestResults) for modeling
There's a SUBSYSTEM for I/O
The I/O Subsystem has four UNITS (JUnitSerializer, Serializer, SerializerHelpers, and TrxSerialize)

The problem is I'm not seeing this breakdown or hierarchical organization in the design documentation or requirements. Please examine the code, then adjust the system/subsystems/units to match this.

…t architecture

Agent-Logs-Url: https://github.com/demaconsulting/TestResults/sessions/80d2fc29-50db-4e9a-8f22-5435ea16dc7d

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

Copilot AI commented Apr 4, 2026

@copilot The requirements hierarchy is weird.

The SYSTEM is this TestResults-Library (a thing that can read and write test results to different files.
...

Restructured the requirements hierarchy in f16db8c to match the architecture you described:

platform-requirements.yaml — now covers the full SYSTEM platform/runtime support tier:

  • Platform Support: Windows, Linux, macOS (was already here)
  • Runtime Support: net8, net9, net10, netstandard2 (moved from test-results.yaml)

test-results.yaml — now contains only the TestResults CLASS unit requirements (TestResults-Mdl-Collection). The old file mixed system-level runtime requirements with unit-level model requirements; they're now separated with a header pointing to the appropriate sibling files.

io/io.yaml — was empty; now has two IO subsystem requirements (TestResults-IO-Identify, TestResults-IO-Deserialize) linked to the existing IOTests.cs subsystem tests, which already lived in the TestResults-IO review-set.

The resulting hierarchy in requirements now mirrors the architecture:

System: TestResults-Library
├── platform-requirements.yaml  (Platform: windows/linux/macos, Runtime: net8/net9/net10/netstandard2)
├── test-results.yaml           (UNIT: TestResults class)
├── test-outcome.yaml           (UNIT: TestOutcome)
├── test-result.yaml            (UNIT: TestResult)
└── io/
    ├── io.yaml                 (SUBSYSTEM: IO)
    ├── serializer.yaml         (UNIT: Serializer + SerializerHelpers internal helper)
    ├── trx-serializer.yaml     (UNIT: TrxSerializer)
    └── junit-serializer.yaml   (UNIT: JUnitSerializer)

@copilot The requirements hierarchy is weird.

The SYSTEM is this TestResults-Library (a thing that can read and write test results to different files.
...

Resolved the naming collision in c79a5fc. The SYSTEM is now consistently named TestResultsLibrary (test-results-library) in all design and requirements files, while the TestResults class remains the test-results unit within it:

Renamed folders:

  • docs/design/test-results/docs/design/test-results-library/
  • docs/reqstream/test-results/docs/reqstream/test-results-library/

Renamed system design file:

  • test-results.mdtest-results-library.md ("TestResults Library System Design")

New system-level requirements file:

  • docs/reqstream/test-results-library/test-results-library.yaml — contains the system's architectural requirement (TestResultsLibrary-Architecture-InMemoryModel) linked to the IO subsystem tests that prove the system's core promise

The hierarchy is now unambiguous:

docs/reqstream/test-results-library/
├── test-results-library.yaml   ← SYSTEM  (TestResults Library)
├── test-results.yaml           ← UNIT    (TestResults class)
├── test-outcome.yaml           ← UNIT    (TestOutcome)
├── test-result.yaml            ← UNIT    (TestResult)
├── platform-requirements.yaml  ← PLATFORM (windows/linux/macos, net8/net9/net10/netstandard2)
└── io/
    ├── io.yaml                 ← SUBSYSTEM (IO)
    ├── serializer.yaml         ← UNIT    (Serializer + SerializerHelpers)
    ├── trx-serializer.yaml     ← UNIT    (TrxSerializer)
    └── junit-serializer.yaml   ← UNIT    (JUnitSerializer)

Updated: requirements.yaml, definition.yaml, .reviewmark.yaml, and all cross-references in design docs. No source, test, or csproj files changed.

…ve naming collision

Agent-Logs-Url: https://github.com/demaconsulting/TestResults/sessions/80d2fc29-50db-4e9a-8f22-5435ea16dc7d

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

@copilot Please make sure to update the docs/design/introduction to reflect this new system/subsystem/unit breakdown.

Copilot AI and others added 2 commits April 4, 2026 13:42
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 4, 2026

@copilot Please make sure to update the docs/design/introduction to reflect this new system/subsystem/unit breakdown.

Updated all design documentation in 45fab81. Here's a summary of every file touched:

introduction.md — Fixed duplicate reference links (left over from previous session). Already had the correct system/subsystem/unit structure tree.

test-results-library.md (System design) — Replaced the generic "Software Items" pointer and the misplaced TestResults Class properties section with a proper "Subsystems and Units" section containing linked tables for the IO Subsystem and the three top-level units. Updated the Mermaid diagram to include SerializerHelpers in the IO Subsystem and rename Model LayerModel Units.

New test-results.md (TestResults unit design) — Created; contains the TestResults class property documentation extracted from the system design, plus a related requirements link. Added to definition.yaml and the TestResults-TestResults review set.

io/io.md (IO Subsystem design) — Added SerializerHelpers row to the units table. Fixed the TestResults class links to point to test-results.md (not test-results-library.md). Changed "Model layer" → "Model units".

New io/serializer-helpers.md (SerializerHelpers unit design) — Created; documents Utf8StringWriter (moved from serializer.md). Added to definition.yaml and the TestResults-IO-Serializer review set.

io/serializer.md (Serializer unit design) — Removed the Utf8StringWriter section, replaced with a brief reference to the SerializerHelpers unit.

test-outcome.md (TestOutcome unit design) — Fixed two "serialization layer" references → "IO subsystem".

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread

…erHelpers unit

Agent-Logs-Url: https://github.com/demaconsulting/TestResults/sessions/bcca5ce1-cfcc-4026-978f-e05991845ce8

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

Copilot AI commented Apr 4, 2026

@copilot apply changes based on the comments in this thread

Applied in c295d1e. Added docs/reqstream/test-results-library/io/serializer-helpers.yaml as a proper unit requirements file for SerializerHelpers, updated io/io.yaml to reference it, added it to requirements.yaml, and created a dedicated TestResults-IO-SerializerHelpers review-set in .reviewmark.yaml.

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

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

Comments suppressed due to low confidence (1)

docs/design/test-results-library/io/serializer.md:60

  • In the "SerializerHelpers Dependency" section, the text says Serializer.Deserialize() "delegates writing" and depends on SerializerHelpers for UTF-8 output. Deserialize() is a read path; the UTF-8 writer helper is used by TrxSerializer.Serialize() / JUnitSerializer.Serialize() (write path). Update this section to refer to the serialize path (and/or the serializers) to avoid documenting incorrect behavior.

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

Comment thread docs/reqstream/test-results-library/io/serializer-helpers.yaml Outdated
@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 4, 2026

@copilot apply changes based on the comments in this thread

Applied in 81e4f14. Added dedicated UTF-8 encoding tests for both TrxSerializer and JUnitSerializer, and updated serializer-helpers.yaml to reference them.

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

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

Comments suppressed due to low confidence (1)

docs/design/test-results-library/io/serializer.md:60

  • This section incorrectly states that Serializer.Deserialize() “delegates writing” and that it depends on SerializerHelpers for UTF-8 output encoding. In the implementation, Serializer.Deserialize() only delegates deserialization to TrxSerializer.Deserialize() / JUnitSerializer.Deserialize() and does not use Utf8StringWriter at all; the UTF-8 declaration concern applies to TrxSerializer.Serialize() / JUnitSerializer.Serialize(). Update this section to reflect the real dependency (or remove it from the Serializer doc and keep it in the serializer-specific docs / SerializerHelpers doc).

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

Comment thread test/DemaConsulting.TestResults.Tests/TestResultsLibraryTests.cs
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