chore: solution review & modernization — AI config, CI coverage, test coverage - #133
Conversation
Co-authored-by: wforney <79032+wforney@users.noreply.github.com>
…y, avoid CA1030) Co-authored-by: wforney <79032+wforney@users.noreply.github.com>
…DataTestMethod) Co-authored-by: wforney <79032+wforney@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Modernization pass across the SharedCode .NET solution to (1) keep Copilot prompt/config documentation aligned with current practices, (2) improve CI observability via code coverage collection, and (3) expand unit-test coverage for core utilities.
Changes:
- Added/updated Copilot prompt + instruction docs and workflow validation for required prompt files.
- Updated CI and build props to capture coverage artifacts and re-enable
SourceRevisionIdpopulation. - Added new MSTest coverage for several previously untested
SharedCode.Corepublic members and adjusted test-project analyzer settings.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
SharedCode.Data.Tests/.editorconfig |
Suppresses CA1515 at the test-project level. |
SharedCode.Core.Tests/.editorconfig |
Suppresses CA1515 at the test-project level. |
SharedCode.Core.Tests/AssemblyExtensionsTests.cs |
Adds unit tests for AssemblyExtensions.GetAttribute<T>(). |
SharedCode.Core.Tests/EventHandlerExtensionsTests.cs |
Adds unit tests for EventHandlerExtensions.Raise(...) overloads. |
SharedCode.Core.Tests/ExtensionsTests.cs |
Adds unit tests for various Extensions helpers (e.g., IsBetween, In, ChangeType, GetPropertyValue). |
SharedCode.Core.Tests/FunctionExtensionsTests.cs |
Adds unit tests for FunctionExtensions.Memoize(...). |
SharedCode.Core.Tests/PropertySupportTests.cs |
Adds unit tests for PropertySupport.ExtractPropertyName<T>(). |
SharedCode.Core.Tests/TypeExtensionsTests.cs |
Adds unit tests for TypeExtensions helpers (e.g., GetDisplayName, IsNullable). |
Directory.Packages.props |
Updates AwesomeAssertions and analyzer package versions. |
Directory.Build.props |
Re-enables SourceRevisionId capture and allows git-less builds via IgnoreExitCode=True. |
.vscode/mcp.json |
Switches GitHub MCP server to npx and adds additional MCP servers. |
.github/workflows/maintain-copilot-instructions.yml |
Extends validation to include required .github/prompts/* files and updates issue wording. |
.github/workflows/dotnet.yml |
Collects XPlat code coverage during tests and uploads Cobertura reports as an artifact. |
.github/prompts/maintain-copilot-instructions.prompt.md |
Updates maintenance checklist to include new/updated prompts and MCP guidance. |
.github/prompts/improve-coverage.prompt.md |
New prompt describing a process for identifying and filling coverage gaps. |
.github/prompts/create-module.prompt.md |
Updates module scaffolding template to include net8.0 target. |
.github/prompts/add-test-class.prompt.md |
New prompt for MSTest scaffolding; contains a couple template issues noted in review comments. |
.github/copilot-instructions.md |
Updates module table and adds a C# 13/.NET 10 features section; expands testing conventions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… CA1515 guidance Co-authored-by: wforney <79032+wforney@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (3)
SharedCode.Core.Tests/ExtensionsTests.cs:35
- This parameterized test uses
[DataRow]but is marked[TestMethod]. To match the new testing guidance added in this PR (use[DataTestMethod]for[DataRow]tests), switch this to[DataTestMethod].
[TestMethod]
[DataRow(0, 1, 10)]
[DataRow(11, 1, 10)]
public void IsBetween_ValueOutOfRange_ReturnsFalse(int value, int low, int high)
SharedCode.Core.Tests/ExtensionsTests.cs:15
- These tests are parameterized with
[DataRow], but the attribute is[TestMethod]. In this PR you addedadd-test-class.prompt.mdwhich instructs using[DataTestMethod]+[DataRow]for parameterized tests, so this new test should follow that convention.
This issue also appears on line 32 of the same file.
[TestMethod]
[DataRow(5, 1, 10)]
[DataRow(1, 1, 10)]
[DataRow(10, 1, 10)]
public void IsBetween_ValueInRange_ReturnsTrue(int value, int low, int high)
.github/copilot-instructions.md:169
- Testing docs currently say assertions are done with AwesomeAssertions, but the new
add-test-class.prompt.mdand the new tests added in this PR use MSTestAssert.*. This inconsistency will lead to mixed guidance for contributors; please align the instructions to reflect the intended assertion style (or explicitly document that both are acceptable).
- **Framework**: MSTest (`[TestClass]`, `[TestMethod]`, `[DataRow]`, `[DataTestMethod]`)
- **Assertions**: AwesomeAssertions
- **Pattern**: Arrange / Act / Assert with blank lines separating each block
Broad modernization pass: updates Copilot AI configuration to reflect current patterns, wires up code coverage in CI, and adds test coverage for previously untested public members.
AI / Copilot system
copilot-instructions.md— addsSharedCode.Data.Teststo the module table; adds a C# 13 / .NET 10 Modern Features section covering collection expressions,params ReadOnlySpan<T>, primary constructors,fieldkeyword,allows ref struct; expands testing conventions for both test projectscreate-module.prompt.md— target frameworks template now includesnet8.0add-test-class.prompt.md(new) — MSTest scaffolding guide with single-scenario,[DataRow], andAssert.ThrowsExactlyexception templatesimprove-coverage.prompt.md(new) — coverage-gap identification guide with per-member checklist and known gap tablemaintain-copilot-instructions.yml— also validates required prompt files exist; opens an issue if any are missing.vscode/mcp.json— switches GitHub MCP from Docker tonpx; addssequential-thinkinganddotnet-skillsserversBuild / CI
dotnet.yml— adds--collect:"XPlat Code Coverage"+ Cobertura artifact uploadDirectory.Build.props— re-enables theSourceRevisionId<Output>element that was commented out, preventing source revision from ever being set;IgnoreExitCode=Truefor git-less buildsDirectory.Packages.props—AwesomeAssertions9.1.0→9.2.0editorconfig
.editorconfigfiles:CA1515suppressed at project level (MSTest requirespublictest classes, making per-class[SuppressMessage]boilerplate unnecessary)Test coverage (+38 methods across 6 new files)
AssemblyExtensionsTestsGetAttribute<T>— found, not found, null guardEventHandlerExtensionsTestsRaiseoverloads — null handler, value-wrapped,EventArgsExtensionsTestsIsBetween,In,IfNotNull,IsNull,IsNotNull,ChangeType<T>,GetPropertyValue<T>FunctionExtensionsTestsMemoize— cache miss, hit, multiple keys, null guardPropertySupportTestsExtractPropertyName— valid expression, null expressionTypeExtensionsTestsGetDisplayName,IsNullable,IsBoolean,IsString,BaseType,IsSubclassOfTypeByName