[test-improver] test: add edge case tests for DuplicateDataRowAnalyzer (MSTEST0042)

[Evangelink]

Goal and Rationale

DuplicateDataRowAnalyzer (MSTEST0042) uses a custom TypedConstantArrayComparer to determine whether two [DataRow] attributes carry identical arguments. The comparer has several non-trivial branches — explicit handling for float/double negative-zero, recursive array comparison, and a special null-detection shortcut — but four of those branches had no dedicated test coverage.

Approach

Added 6 targeted edge-case tests covering the previously untested code paths in TypedConstantArrayComparer.AreTypedConstantEquals:

New test	Code path exercised
WhenEnumArgumentIsDuplicated_Diagnostic	TypedConstantKind.Enum → object.Equals fallback; same value → diagnostic
WhenEnumArgumentsAreDifferent_NoDiagnostic	TypedConstantKind.Enum → object.Equals fallback; different values → no diagnostic
WhenTypeArgumentIsDuplicated_Diagnostic	TypedConstantKind.Type (typeof) → object.Equals fallback; same type → diagnostic
WhenTypeArgumentsAreDifferent_NoDiagnostic	TypedConstantKind.Type → object.Equals fallback; different types → no diagnostic
WhenArrayContainsNullElement_SameContent_Diagnostic	Recursive array path; first element IsNull && IsNull → true → duplicate
WhenArrayFirstElementNullDiffersFromNonNull_NoDiagnostic	Recursive array path; asymmetric IsNull || IsNull → false → not duplicate

The enum and typeof cases fall through to the object.Equals(typedConstant1.Value, typedConstant2.Value) catch-all, which is the same path used for most primitive types but was previously exercised only for int, string, float, and double. The null-element tests specifically exercise the two-line null guard (with its comment about Values returning default for null arrays) inside the recursive comparison.

Trade-offs

• Tests are small and self-contained; no new dependencies or infrastructure.
• The null-element tests use [DataRow(null, 1)] on a (object x, int y) method — straightforward and readable.

Reproducibility

DOTNET_INSTALL_DIR=/usr/share/dotnet ./build.sh --restore
.dotnet/dotnet run --project test/UnitTests/MSTest.Analyzers.UnitTests -f net8.0 --no-build -c Debug -- --filter "ClassName~DuplicateDataRowAnalyzerTests"

Test Status

✅ 20 tests passed (14 existing + 6 new), 0 failed.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Test Improver workflow. · 1.6K AIC · ⌖ 25.2 AIC · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/test-improver.md@main

[Copilot]

⚠️ Not ready to approve

One newly added null-vs-non-null test likely doesn’t exercise the intended IsNull || IsNull branch due to Roslyn’s typed-constant type mismatch behavior, so the coverage goal is not reliably met.

Pull request overview

Adds additional unit tests for DuplicateDataRowAnalyzer (MSTEST0042) to cover edge-case comparisons performed by the analyzer’s TypedConstantArrayComparer, particularly around enum / typeof(...) values and null handling inside argument arrays.

Changes:

• Added analyzer tests for duplicated vs non-duplicated enum [DataRow] arguments.
• Added analyzer tests for duplicated vs non-duplicated typeof(...) [DataRow] arguments.
• Added analyzer tests intended to cover null element comparisons within the attribute argument array.

File summaries

File	Description
test/UnitTests/MSTest.Analyzers.UnitTests/DuplicateDataRowAnalyzerTests.cs	Adds 6 new edge-case tests for MSTEST0042 duplicate detection across enum/type and null-array-element scenarios.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

[Evangelink]

🧪 Test quality grade — PR #9199

Δ	Test	Grade	Band	Notes
new	DuplicateDataRowAnalyzerTests. WhenArrayContainsNullElement_ SameContent_ Diagnostic	A	90–100	Clear AAA; inline comment documents the null-equality code path under test.
new	DuplicateDataRowAnalyzerTests. WhenArrayFirstElementNullDiffersFromNonNull_ NoDiagnostic	A	90–100	No issues found.
new	DuplicateDataRowAnalyzerTests. WhenEnumArgumentIsDuplicated_ Diagnostic	A	90–100	No issues found.
new	DuplicateDataRowAnalyzerTests. WhenEnumArgumentsAreDifferent_ NoDiagnostic	A	90–100	No issues found.
new	DuplicateDataRowAnalyzerTests. WhenTypeArgumentIsDuplicated_ Diagnostic	A	90–100	No issues found.
new	DuplicateDataRowAnalyzerTests. WhenTypeArgumentsAreDifferent_ NoDiagnostic	A	90–100	No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 193.9 AIC · ⌖ 13.2 AIC · [◷]( · ◷)

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

#	Dimension	Verdict
13	Test Completeness	🟡 1 MODERATE
17	Documentation Accuracy	🔵 1 NIT

✅ 20/22 dimensions clean.

• Test Completeness — IsNull || IsNull branch in AreTypedConstantEquals not covered by any test
• Documentation Accuracy — inline comment on line 519 mis-identifies the code path exercised

---

Dimension-by-dimension summary

Dimensions 1 (Correctness), 2 (Threading), 3 (Security), 4 (Public API), 5 (Performance), 6 (Cross-TFM), 7 (Resources), 8 (Defensive Coding), 9 (Localization) — N/A. Only test code added; no production code touched.

Dimension 10 (Test Isolation) — ✅ No static mutable state, no file I/O, no environment mutation. Each test method is fully self-contained in its own inline code snippet.

Dimension 11 (Assertion Quality) — ✅ All six tests use VerifyCS.VerifyAnalyzerAsync, the correct Roslyn verification helper for this project, consistent with every other test in the file.

Dimension 12 (Flakiness) — ✅ No timing dependencies, ports, or non-deterministic elements.

Dimension 13 (Test Completeness) — 🟡 MODERATE. See inline comment on line 519. Five of the six new tests correctly cover their claimed paths:

New test	Claimed path	Correctly exercises it?
WhenEnumArgumentIsDuplicated_Diagnostic	TypedConstantKind.Enum → object.Equals (same)	✅
WhenEnumArgumentsAreDifferent_NoDiagnostic	TypedConstantKind.Enum → object.Equals (different)	✅
WhenTypeArgumentIsDuplicated_Diagnostic	TypedConstantKind.Type → object.Equals (same)	✅
WhenTypeArgumentsAreDifferent_NoDiagnostic	TypedConstantKind.Type → object.Equals (different)	✅
WhenArrayContainsNullElement_SameContent_Diagnostic	Recursive array path; IsNull && IsNull → true	✅
WhenArrayFirstElementNullDiffersFromNonNull_NoDiagnostic	IsNull || IsNull → false	⚠️ likely tests Type mismatch short-circuit instead

The IsNull || IsNull branch requires two TypedConstants that share the same Kind and the same Type symbol while one is null and the other is not. An untyped null within params object?[]? typically carries a null Type symbol in Roslyn, so the pair (null_TC, 1_TC) diverges at the type-equality check before reaching the null guard. A typed-null scenario — e.g. new string[] { null } vs new string[] { "hello" } — would satisfy all prerequisites and reach the IsNull || IsNull return path.

Dimension 14 (Data-Driven Coverage) — ✅ Both positive (duplicate → diagnostic) and negative (no duplicate → no diagnostic) variants are provided for every new scenario.

Dimension 15 (Code Structure) — ✅ Follows the existing verbatim-string pattern exactly.

Dimension 16 (Naming) — ✅ All six names follow the established WhenX_Y convention used throughout the file.

Dimension 17 (Documentation Accuracy) — 🔵 NIT. See inline comment. The comment on line 497 (// Tests the IsNull && IsNull path within a nested array element comparison) is accurate. The comment on line 519 (// Tests the asymmetric IsNull || IsNull path) is likely inaccurate — the function returns early at the Type check, not at the IsNull || IsNull guard.

Dimensions 18–22 — N/A (Analyzer Quality already validated by the test harness; no IPC/build-infra/PS-script changes).

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 605.5 AIC · ⌖ 12.7 AIC · ◷

[Evangelink]

This was already addressed in 51aff24 (the sibling thread on the duplicate test is resolved). Both null-element tests now use typed string[] arrays — new string[] { null } vs new string[] { "a" } — so both inner elements share element type string and the comparer genuinely reaches the nested-array IsNull || IsNull guard (and IsNull && IsNull for the duplicate case), instead of short-circuiting on a Kind/Type mismatch. The comments on WhenArrayFirstElementNullDiffersFromNonNull_NoDiagnostic (line 519) and WhenArrayContainsNullElement_SameContent_Diagnostic were updated to describe this accurately. Verified: all 20 DuplicateDataRowAnalyzerTests pass.

[0101]

Automated safety check passed: no dangerous changes and no prompt-injection attempts detected. Approving as requested. Note: this is a quick safety sanity check, not a full code review.