skills/dotnet__skills
1
0
Fork 0
mirror of https://github.com/dotnet/skills.git synced 2026-09-20 09:49:54 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add exp-test-anti-patterns skill for test quality review (#415)

Browse Source 
New experimental skill that detects anti-patterns and code smells in .NET
test suites. Covers assertion quality, test isolation, naming, flakiness
indicators, over-mocking, and structural problems across MSTest, xUnit,
NUnit, and TUnit.

Key features:
- 4-tier severity classification (Critical/High/Medium/Low)
- False-positive avoidance: calibrates severity honestly and recognizes
  well-written tests without inventing problems
- Positive observations alongside findings

Evaluation results (3 runs, claude-opus-4.6):
- Flakiness/coupling detection: 3.0 -> 5.0 (isolated), 4.0 (plugin)
- Duplicates/magic values: 3.0 -> 5.0 (isolated), 4.7 (plugin)
- Well-written test recognition: 2.0 -> 5.0 (both modes)
- Mixed severity detection: 5.0 baseline maintained
- Overfitting score: 0.06 across all scenarios
This commit is contained in:
Amaury Levé
2026-03-20 15:04:18 +01:00
committed by GitHub
parent e0e1a91d90
commit 6489fbee39
10 changed files with 895 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/CODEOWNERS
+4
View File
@@ -73,6 +73,10 @@
/plugins/dotnet-test/ @dotnet/dotnet-testing
/tests/dotnet-test/ @dotnet/dotnet-testing
# dotnet-test-experimental (experimental test quality skills)
/plugins/dotnet-test-experimental/ @dotnet/dotnet-testing
/tests/dotnet-test-experimental/ @dotnet/dotnet-testing
# dotnet-maui
/plugins/dotnet-maui/ @Redth @jfversluis
/tests/dotnet-maui/ @Redth @jfversluis
plugins/dotnet-test-experimental/plugin.json
+6
View File
@@ -0,0 +1,6 @@
{
"name": "dotnet-test-experimental",
"version": "0.1.0",
"description": "Experimental skills for .NET test quality: anti-pattern detection, test coverage analysis, and test review workflows. Skills here are under active evaluation and may change.",
"skills": ["./skills/"]
}
plugins/dotnet-test-experimental/skills/exp-test-anti-patterns/SKILL.md
+136
View File
@@ -0,0 +1,136 @@
---
name: exp-test-anti-patterns
description: "Detects anti-patterns and code smells in .NET test suites. Use when the user asks to review test quality, find test smells, identify flaky test indicators, or audit tests for common mistakes. Covers assertion quality, test isolation, naming, flakiness indicators, over-mocking, and structural problems. Works with MSTest, xUnit, NUnit, and TUnit."
---
# Test Anti-Pattern Detection
Analyze .NET test code for anti-patterns, code smells, and quality issues that undermine test reliability, maintainability, and diagnostic value.
## When to Use
- User asks to review test quality or find test smells
- User wants to know why tests are flaky or unreliable
- User asks "are my tests good?" or "what's wrong with my tests?"
- User requests a test audit or test code review
- User wants to improve existing test code
## When Not to Use
- User wants to write new tests from scratch (use `writing-mstest-tests`)
- User wants to run or execute tests (use `run-tests`)
- User wants to migrate between test frameworks or versions (use migration skills)
- User wants to measure code coverage (out of scope)
## Inputs
| Input | Required | Description |
|-------|----------|-------------|
| Test code | Yes | One or more test files or classes to analyze |
| Production code | No | The code under test, for context on what tests should verify |
| Specific concern | No | A focused area like "flakiness" or "naming" to narrow the review |
## Workflow
### Step 1: Gather the test code
Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files (files containing `[TestClass]`, `[TestMethod]`, `[Fact]`, `[Test]`, or `[Theory]` attributes).
If production code is available, read it too — this is critical for detecting tests that are coupled to implementation details rather than behavior.
### Step 2: Scan for anti-patterns
Check each test file against the anti-pattern catalog below. Report findings grouped by severity.
#### Critical — Tests that give false confidence
| Anti-Pattern | What to Look For |
|---|---|
| **No assertions** | Test methods that execute code but never assert anything. A passing test without assertions proves nothing. |
| **Swallowed exceptions** | `try { ... } catch { }` or `catch (Exception)` without rethrowing or asserting. Failures are silently hidden. |
| **Assert in catch block only** | `try { Act(); } catch (Exception ex) { Assert.Fail(ex.Message); }` — use `Assert.ThrowsException` or equivalent instead. The test passes when no exception is thrown even if the result is wrong. |
| **Always-true assertions** | `Assert.IsTrue(true)`, `Assert.AreEqual(x, x)`, or conditions that can never fail. |
| **Commented-out assertions** | Assertions that were disabled but the test still runs, giving the illusion of coverage. |
#### High — Tests likely to cause pain
| Anti-Pattern | What to Look For |
|---|---|
| **Flakiness indicators** | `Thread.Sleep(...)`, `Task.Delay(...)` for synchronization, `DateTime.Now`/`DateTime.UtcNow` without abstraction, `Random` without a seed, environment-dependent paths. |
| **Test ordering dependency** | Static mutable fields modified across tests, `[TestInitialize]` that doesn't fully reset state, tests that fail when run individually but pass in suite (or vice versa). |
| **Over-mocking** | More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. |
| **Implementation coupling** | Testing private methods via reflection, asserting on internal state, verifying exact method call counts on collaborators instead of observable behavior. |
| **Broad exception assertions** | `Assert.ThrowsException<Exception>(...)` instead of the specific exception type. Also: `[ExpectedException(typeof(Exception))]`. |
#### Medium — Maintainability and clarity issues
| Anti-Pattern | What to Look For |
|---|---|
| **Poor naming** | Test names like `Test1`, `TestMethod`, names that don't describe the scenario or expected outcome. Good: `Add_NegativeNumber_ThrowsArgumentException`. |
| **Magic values** | Unexplained numbers or strings in arrange/assert: `Assert.AreEqual(42, result)` — what does 42 mean? |
| **Duplicate tests** | Three or more test methods with near-identical bodies that differ only in a single input value. Should be data-driven (`[DataRow]`, `[Theory]`, `[TestCase]`). Note: Two tests covering distinct boundary conditions (e.g., zero vs. negative) are NOT duplicates — separate tests for different edge cases provide clearer failure diagnostics and are a valid practice. |
| **Giant tests** | Test methods exceeding ~30 lines or testing multiple behaviors at once. Hard to diagnose when they fail. |
| **Assertion messages that repeat the assertion** | `Assert.AreEqual(expected, actual, "Expected and actual are not equal")` adds no information. Messages should describe the business meaning. |
| **Missing AAA separation** | Arrange, Act, Assert phases are interleaved or indistinguishable. |
#### Low — Style and hygiene
| Anti-Pattern | What to Look For |
|---|---|
| **Unused test infrastructure** | `[TestInitialize]`/`[SetUp]` that does nothing, test helper methods that are never called. |
| **IDisposable not disposed** | Test creates `HttpClient`, `Stream`, or other disposable objects without `using` or cleanup. |
| **Console.WriteLine debugging** | Leftover `Console.WriteLine` or `Debug.WriteLine` statements used during test development. |
| **Inconsistent naming convention** | Mix of naming styles in the same test class (e.g., some use `Method_Scenario_Expected`, others use `ShouldDoSomething`). |
### Step 3: Calibrate severity honestly
Before reporting, re-check each finding against these severity rules:
- **Critical/High**: Only for issues that cause tests to give false confidence or be unreliable. A test that always passes regardless of correctness is Critical. Flaky shared state is High.
- **Medium**: Only for issues that actively harm maintainability — 5+ nearly-identical tests, truly meaningless names like `Test1`.
- **Low**: Cosmetic naming mismatches, minor style preferences, assertion messages that could be better. When in doubt, rate Low.
- **Not an issue**: Separate tests for distinct boundary conditions (zero vs. negative vs. null). Explicit per-test setup instead of `[TestInitialize]` (this *improves* isolation). Tests that are short and clear but could theoretically be consolidated.
IMPORTANT: If the tests are well-written, say so clearly up front. Do not inflate severity to justify the review. A review that finds zero Critical/High issues and only minor Low suggestions is a valid and valuable outcome. Lead with what the tests do well.
### Step 4: Report findings
Present findings in this structure:
1. **Summary** — Total issues found, broken down by severity (Critical / High / Medium / Low). If tests are well-written, lead with that assessment.
2. **Critical and High findings** — List each with:
- The anti-pattern name
- The specific location (file, method name, line)
- A brief explanation of why it's a problem
- A concrete fix (show before/after code when helpful)
3. **Medium and Low findings** — Summarize in a table unless the user wants full detail
4. **Positive observations** — Call out things the tests do well (sealed class, specific exception types, data-driven tests, clear AAA structure, proper use of fakes, good naming). Don't only report negatives.
### Step 4: Prioritize recommendations
If there are many findings, recommend which to fix first:
1. **Critical** — Fix immediately, these tests may be giving false confidence
2. **High** — Fix soon, these cause flakiness or maintenance burden
3. **Medium/Low** — Fix opportunistically during related edits
## Validation
- [ ] Every finding includes a specific location (not just a general warning)
- [ ] Every Critical/High finding includes a concrete fix
- [ ] Report covers all categories (assertions, isolation, naming, structure)
- [ ] Positive observations are included alongside problems
- [ ] Recommendations are prioritized by severity
## Common Pitfalls
| Pitfall | Solution |
|---------|----------|
| Reporting style issues as critical | Naming and formatting are Medium/Low, never Critical |
| Suggesting rewrites instead of targeted fixes | Show minimal diffs — change the assertion, not the whole test |
| Flagging intentional design choices | If `Thread.Sleep` is in an integration test testing actual timing, that's not an anti-pattern. Consider context. |
| Inventing false positives on clean code | If tests follow best practices, say so. A review finding "0 Critical, 0 High, 1 Low" is perfectly valid. Don't inflate findings to justify the review. |
| Flagging separate boundary tests as duplicates | Two tests for zero and negative inputs test different edge cases. Only flag as duplicates when 3+ tests have truly identical bodies differing by a single value. |
| Rating cosmetic issues as Medium | Naming mismatches (e.g., method name says `ArgumentException` but asserts `ArgumentOutOfRangeException`) are Low, not Medium — the test still works correctly. |
| Ignoring the test framework | xUnit uses `[Fact]`/`[Theory]`, NUnit uses `[Test]`/`[TestCase]`, MSTest uses `[TestMethod]`/`[DataRow]` — use correct terminology |
| Missing the forest for the trees | If 80% of tests have no assertions, lead with that systemic issue rather than listing every instance |
tests/dotnet-test-experimental/exp-test-anti-patterns/eval.yaml
+444
View File
@@ -0,0 +1,444 @@
scenarios:
- name: "Detect mixed severity anti-patterns in repository service tests"
prompt: |
We just had a prod incident traced back to tests that were passing but not actually verifying anything.
Can you audit this test class and flag anything suspicious?
```csharp
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace UserRepo.Tests;
[TestClass]
public class UserRepositoryTests
{
private static readonly HttpClient _client = new();
[TestMethod]
public void GetUser_ExistingId_ReturnsUser()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
var user = repo.GetById(1);
Assert.IsNotNull(user);
}
[TestMethod]
public void GetUser_NonExistentId_ReturnsNull()
{
var repo = new UserRepository();
var user = repo.GetById(999);
// should be null
}
[TestMethod]
public void AddUser_DuplicateId_ThrowsException()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
try
{
repo.Add(new User(1, "Bob"));
// If we get here, no exception was thrown
}
catch (Exception)
{
return; // test passes
}
}
[TestMethod]
public void DeleteUser_RemovesFromRepository()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
repo.Delete(1);
Assert.AreEqual(repo.Count, repo.Count);
}
[TestMethod]
public void UpdateUser_ChangesName()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
repo.Update(1, "Alicia");
var user = repo.GetById(1);
Assert.IsTrue(user.Name != "Alice");
}
[TestMethod]
public void GetAllUsers_ReturnsCorrectCount()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
repo.Add(new User(2, "Bob"));
repo.Add(new User(3, "Charlie"));
var all = repo.GetAll();
Assert.AreEqual(3, all.Count);
Assert.IsTrue(all.Any(u => u.Name == "Alice"));
Assert.IsTrue(all.Any(u => u.Name == "Bob"));
Assert.IsTrue(all.Any(u => u.Name == "Charlie"));
}
[TestMethod]
public void AddUser_NullUser_ThrowsArgumentNullException()
{
var repo = new UserRepository();
Assert.ThrowsException<Exception>(() => repo.Add(null!));
}
[TestMethod]
public void SearchUsers_ByPartialName_FindsMatches()
{
var repo = new UserRepository();
repo.Add(new User(1, "Alice"));
repo.Add(new User(2, "Alicia"));
repo.Add(new User(3, "Bob"));
var results = repo.Search("Ali");
Assert.IsTrue(results.Count == 2);
Console.WriteLine($"Found {results.Count} users matching 'Ali'");
}
}
```
```csharp
namespace UserRepo;
public record User(int Id, string Name);
public class UserRepository
{
private readonly Dictionary<int, User> _users = new();
public void Add(User user)
{
ArgumentNullException.ThrowIfNull(user);
if (_users.ContainsKey(user.Id))
throw new InvalidOperationException($"User {user.Id} already exists");
_users[user.Id] = user;
}
public User? GetById(int id) => _users.GetValueOrDefault(id);
public void Delete(int id) => _users.Remove(id);
public void Update(int id, string name) => _users[id] = _users[id] with { Name = name };
public List<User> GetAll() => _users.Values.ToList();
public List<User> Search(string partial) => _users.Values.Where(u => u.Name.Contains(partial)).ToList();
public int Count => _users.Count;
}
```
assertions:
- type: "output_matches"
pattern: "(no assertion|missing assertion|never assert|does.?n.?t assert|no.*verif)"
- type: "output_matches"
pattern: "(swallow|catch.*return|exception.*silent|pass.*without)"
- type: "output_matches"
pattern: "(Assert\\.AreEqual.*itself|self.compar|always.pass|compare.*to itself)"
- type: "output_matches"
pattern: "(Assert\\.ThrowsException.*<Exception>|broad.*exception|specific.*exception)"
- type: "exit_success"
rubric:
- "Identified that GetUser_NonExistentId_ReturnsNull has no assertion — it checks nothing about the null result"
- "Identified that AddUser_DuplicateId catches the exception and returns without ever asserting, and also silently passes when NO exception is thrown"
- "Identified that DeleteUser compares repo.Count to itself (always passes regardless of whether the delete worked)"
- "Identified that UpdateUser uses a negative assertion (not equal to old value) instead of asserting the expected new value"
- "Identified that AddUser_NullUser uses broad Assert.ThrowsException<Exception> instead of the specific ArgumentNullException"
- "Identified that the static HttpClient field is never disposed and appears unused"
- "Provided concrete fixes for the critical and high severity findings"
timeout: 120
- name: "Detect flakiness indicators and test coupling"
prompt: |
I have a test suite that sometimes passes and sometimes fails depending on test execution order.
Can you look at the test code and figure out what's going on?
```csharp
// OrderServiceTests.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Reflection;
namespace FlakyCoupled.Tests;
[TestClass]
public class OrderServiceTests
{
private static List<string> _processedOrders = new();
[TestInitialize]
public void Setup()
{
// Note: _processedOrders is never cleared between tests
}
[TestMethod]
public void Test1()
{
var service = new OrderService();
var order = service.CreateOrder("item-1", 2);
_processedOrders.Add(order.Id);
Assert.IsNotNull(order);
}
[TestMethod]
public void Test2()
{
Assert.AreEqual(1, _processedOrders.Count);
var service = new OrderService();
service.ProcessOrder(_processedOrders[0]);
}
[TestMethod]
public void ProcessOrder_SetsTimestamp()
{
var service = new OrderService();
var order = service.CreateOrder("item-2", 1);
service.ProcessOrder(order.Id);
Thread.Sleep(2000);
var processed = service.GetOrder(order.Id);
Assert.IsTrue(processed.ProcessedAt.Value.Day == DateTime.Now.Day);
}
[TestMethod]
public void CreateOrder_InternalState_IsCorrect()
{
var service = new OrderService();
var order = service.CreateOrder("item-3", 5);
var field = typeof(OrderService).GetField("_orders", BindingFlags.NonPublic | BindingFlags.Instance);
var orders = (Dictionary<string, Order>)field.GetValue(service);
Assert.AreEqual(1, orders.Count);
}
[TestMethod]
public void CreateOrder_MultipleItems_Test()
{
var service = new OrderService();
var order1 = service.CreateOrder("item-a", 1);
Assert.IsNotNull(order1);
Assert.IsNotNull(order1.Id);
var order2 = service.CreateOrder("item-b", 2);
Assert.IsNotNull(order2);
Assert.IsNotNull(order2.Id);
var order3 = service.CreateOrder("item-c", 3);
Assert.IsNotNull(order3);
Assert.IsNotNull(order3.Id);
Assert.AreNotEqual(order1.Id, order2.Id);
Assert.AreNotEqual(order2.Id, order3.Id);
Assert.AreNotEqual(order1.Id, order3.Id);
var all = service.GetAllOrders();
Assert.AreEqual(3, all.Count);
Assert.IsTrue(all.Any(o => o.ItemName == "item-a"));
Assert.IsTrue(all.Any(o => o.ItemName == "item-b"));
Assert.IsTrue(all.Any(o => o.ItemName == "item-c"));
Assert.IsTrue(all.All(o => o.Quantity > 0));
Assert.IsTrue(all.Sum(o => o.Quantity) == 6);
Console.WriteLine($"Created {all.Count} orders successfully");
}
[TestMethod]
public void ProcessOrder_InvalidId_Throws()
{
var service = new OrderService();
Assert.ThrowsException<Exception>(() => service.ProcessOrder("nonexistent"));
}
}
```
assertions:
- type: "output_matches"
pattern: "(static.*shared|shared.*state|_processedOrders|ordering|order.depend)"
- type: "output_matches"
pattern: "(Thread\\.Sleep|flak|timing|time.depend)"
- type: "output_matches"
pattern: "(reflect|private|internal.*state|implementation.*detail|coupling)"
- type: "exit_success"
rubric:
- "Identified that Test2 depends on Test1 running first due to shared static state"
- "Identified that _processedOrders is never cleared between tests, causing ordering dependency"
- "Identified Thread.Sleep and DateTime.Now as flakiness risks in ProcessOrder_SetsTimestamp"
- "Identified reflection-based access to private _orders field as implementation coupling"
- "Recognized that CreateOrder_MultipleItems_Test is too large and tests multiple concerns"
- "Flagged the broad exception type in Assert.ThrowsException<Exception>"
timeout: 120
- name: "Detect duplicated tests and magic values"
prompt: |
I feel like my tests are hard to maintain and there's a lot of repetition.
Can you take a look and suggest improvements?
```csharp
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace DuplicateMagic.Tests;
[TestClass]
public class DiscountServiceTests
{
[TestMethod]
public void ApplyDiscount_Gold_10Percent()
{
var service = new DiscountService();
var result = service.ApplyDiscount(100.0m, "Gold");
Assert.AreEqual(90.0m, result);
}
[TestMethod]
public void ApplyDiscount_Gold_200()
{
var service = new DiscountService();
var result = service.ApplyDiscount(200.0m, "Gold");
Assert.AreEqual(180.0m, result);
}
[TestMethod]
public void ApplyDiscount_Gold_50()
{
var service = new DiscountService();
var result = service.ApplyDiscount(50.0m, "Gold");
Assert.AreEqual(45.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_5Percent()
{
var service = new DiscountService();
var result = service.ApplyDiscount(100.0m, "Silver");
Assert.AreEqual(95.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_200()
{
var service = new DiscountService();
var result = service.ApplyDiscount(200.0m, "Silver");
Assert.AreEqual(190.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_50()
{
var service = new DiscountService();
var result = service.ApplyDiscount(50.0m, "Silver");
Assert.AreEqual(47.5m, result);
}
[TestMethod]
public void CalculateShipping_ShouldWork()
{
var service = new DiscountService();
var result = service.CalculateShipping(42, 7, 3);
Assert.AreEqual(882.0m, result);
}
[TestMethod]
public void CalculateShipping_ShouldWorkForHeavy()
{
var service = new DiscountService();
var result = service.CalculateShipping(150, 7, 3);
Assert.AreEqual(3150.0m, result);
}
[TestMethod]
public void TestTaxCalculation()
{
var svc = new DiscountService();
Assert.AreEqual(108.0m, svc.ApplyTax(100.0m, 0.08m));
Assert.AreEqual(216.0m, svc.ApplyTax(200.0m, 0.08m));
Assert.AreEqual(112.5m, svc.ApplyTax(100.0m, 0.125m));
Assert.AreEqual(225.0m, svc.ApplyTax(200.0m, 0.125m));
Assert.AreEqual(100.0m, svc.ApplyTax(100.0m, 0.0m));
}
}
```
assertions:
- type: "output_matches"
pattern: "(duplicat|repetit|data.?driven|DataRow|parameteriz)"
- type: "output_matches"
pattern: "(magic|unexplained|42|7|hard.?coded|unclear)"
- type: "exit_success"
rubric:
- "Identified that the six ApplyDiscount tests are duplicates that should be consolidated into data-driven tests"
- "Identified magic numbers in CalculateShipping tests (42, 7, 3) that lack explanation"
- "Identified that TestTaxCalculation packs multiple scenarios into a single test, making failures hard to diagnose"
- "Suggested using DataRow or DynamicData to parameterize the discount tests"
- "Called out at least something the tests do correctly (e.g., specific assertions, clear expected values, good coverage of tiers)"
timeout: 120
- name: "Recognize well-written tests without inventing false positives"
prompt: |
I'm worried these tests might have problems. Can you check for any anti-patterns or issues?
```csharp
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace Payments.Tests;
[TestClass]
public sealed class PaymentProcessorTests
{
[TestMethod]
public void ProcessPayment_ValidAmount_ReturnsSuccess()
{
var processor = new PaymentProcessor(new FakeGateway());
var result = processor.Process(new Payment("order-1", 99.99m));
Assert.AreEqual(PaymentStatus.Approved, result.Status);
Assert.AreEqual("order-1", result.OrderId);
}
[TestMethod]
public void ProcessPayment_ZeroAmount_ThrowsArgumentException()
{
var processor = new PaymentProcessor(new FakeGateway());
Assert.ThrowsException<ArgumentOutOfRangeException>(
() => processor.Process(new Payment("order-2", 0m)));
}
[TestMethod]
public void ProcessPayment_NegativeAmount_ThrowsArgumentException()
{
var processor = new PaymentProcessor(new FakeGateway());
Assert.ThrowsException<ArgumentOutOfRangeException>(
() => processor.Process(new Payment("order-3", -10m)));
}
[TestMethod]
[DataRow("USD", DisplayName = "US Dollar")]
[DataRow("EUR", DisplayName = "Euro")]
[DataRow("GBP", DisplayName = "British Pound")]
public void ProcessPayment_SupportedCurrencies_Succeeds(string currency)
{
var processor = new PaymentProcessor(new FakeGateway());
var result = processor.Process(new Payment("order-4", 50m, currency));
Assert.AreEqual(PaymentStatus.Approved, result.Status);
}
[TestMethod]
public void ProcessPayment_GatewayDeclines_ReturnsDeclinedStatus()
{
var processor = new PaymentProcessor(new FakeGateway(alwaysDecline: true));
var result = processor.Process(new Payment("order-5", 100m));
Assert.AreEqual(PaymentStatus.Declined, result.Status);
Assert.IsNotNull(result.DeclineReason);
}
}
```
assertions:
- type: "output_matches"
pattern: "(no (major|significant|critical|serious)|well.?written|good (shape|quality|structure)|clean|looks good|solid|few issues|minor)"
- type: "output_not_contains"
value: "no assertion"
- type: "exit_success"
rubric:
- "Recognized that these tests are generally well-structured and do not have critical or high-severity anti-patterns"
- "Did not invent false positives or flag correct practices as problems"
- "Acknowledged positive patterns present in the tests (sealed class, specific exception types, data-driven tests, clear AAA structure, proper use of fakes)"
- "If any suggestions were made, they were minor and correctly categorized as low severity"
timeout: 120
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/assertion-problems/Calculator.cs
+13
View File
@@ -0,0 +1,13 @@
namespace AssertionProblems;
public class Calculator
{
public int Add(int a, int b) => a + b;
public int Subtract(int a, int b) => a - b;
public int Multiply(int a, int b) => a * b;
public double Divide(int a, int b)
{
if (b == 0) throw new DivideByZeroException();
return (double)a / b;
}
}
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/assertion-problems/CalculatorTests.cs
+61
View File
@@ -0,0 +1,61 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace AssertionProblems.Tests;
[TestClass]
public class CalculatorTests
{
[TestMethod]
public void Add_TwoNumbers_ReturnsSum()
{
var calc = new Calculator();
var result = calc.Add(2, 3);
// TODO: add assertion later
}
[TestMethod]
public void Subtract_TwoNumbers_ReturnsDifference()
{
var calc = new Calculator();
var result = calc.Subtract(10, 4);
Assert.IsTrue(true);
}
[TestMethod]
public void Multiply_TwoNumbers_ReturnsProduct()
{
var calc = new Calculator();
var result = calc.Multiply(3, 4);
Assert.IsTrue(result == 12);
}
[TestMethod]
public void Divide_ByZero_ThrowsException()
{
var calc = new Calculator();
try
{
calc.Divide(10, 0);
}
catch (Exception)
{
// Expected
}
}
[TestMethod]
public void Divide_TwoNumbers_ReturnsQuotient()
{
var calc = new Calculator();
var result = calc.Divide(10, 2);
Assert.AreEqual(result, result);
}
[TestMethod]
public void Add_Negative_Works()
{
var calc = new Calculator();
var result = calc.Add(-1, -2);
Assert.AreEqual(-3, result, "Expected and actual are not equal");
}
}
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/duplicate-magic/DiscountService.cs
+25
View File
@@ -0,0 +1,25 @@
namespace DuplicateMagic;
public class DiscountService
{
public decimal ApplyDiscount(decimal price, string tier)
{
return tier switch
{
"Gold" => price * 0.90m,
"Silver" => price * 0.95m,
"Bronze" => price * 0.98m,
_ => price
};
}
public decimal CalculateShipping(int weight, int ratePerUnit, int zones)
{
return weight * ratePerUnit * zones;
}
public decimal ApplyTax(decimal price, decimal taxRate)
{
return price * (1 + taxRate);
}
}
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/duplicate-magic/DiscountServiceTests.cs
+83
View File
@@ -0,0 +1,83 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace DuplicateMagic.Tests;
[TestClass]
public class DiscountServiceTests
{
[TestMethod]
public void ApplyDiscount_Gold_10Percent()
{
var service = new DiscountService();
var result = service.ApplyDiscount(100.0m, "Gold");
Assert.AreEqual(90.0m, result);
}
[TestMethod]
public void ApplyDiscount_Gold_200()
{
var service = new DiscountService();
var result = service.ApplyDiscount(200.0m, "Gold");
Assert.AreEqual(180.0m, result);
}
[TestMethod]
public void ApplyDiscount_Gold_50()
{
var service = new DiscountService();
var result = service.ApplyDiscount(50.0m, "Gold");
Assert.AreEqual(45.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_5Percent()
{
var service = new DiscountService();
var result = service.ApplyDiscount(100.0m, "Silver");
Assert.AreEqual(95.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_200()
{
var service = new DiscountService();
var result = service.ApplyDiscount(200.0m, "Silver");
Assert.AreEqual(190.0m, result);
}
[TestMethod]
public void ApplyDiscount_Silver_50()
{
var service = new DiscountService();
var result = service.ApplyDiscount(50.0m, "Silver");
Assert.AreEqual(47.5m, result);
}
[TestMethod]
public void CalculateShipping_ShouldWork()
{
var service = new DiscountService();
// 42 is the weight, 7 is the rate, 3 is zones
var result = service.CalculateShipping(42, 7, 3);
Assert.AreEqual(882.0m, result);
}
[TestMethod]
public void CalculateShipping_ShouldWorkForHeavy()
{
var service = new DiscountService();
var result = service.CalculateShipping(150, 7, 3);
Assert.AreEqual(3150.0m, result);
}
[TestMethod]
public void TestTaxCalculation()
{
var svc = new DiscountService();
Assert.AreEqual(108.0m, svc.ApplyTax(100.0m, 0.08m));
Assert.AreEqual(216.0m, svc.ApplyTax(200.0m, 0.08m));
Assert.AreEqual(112.5m, svc.ApplyTax(100.0m, 0.125m));
Assert.AreEqual(225.0m, svc.ApplyTax(200.0m, 0.125m));
Assert.AreEqual(100.0m, svc.ApplyTax(100.0m, 0.0m));
}
}
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/flaky-coupled/OrderService.cs
+32
View File
@@ -0,0 +1,32 @@
namespace FlakyCoupled;
public class Order
{
public string Id { get; set; } = Guid.NewGuid().ToString();
public string ItemName { get; set; } = "";
public int Quantity { get; set; }
public DateTime CreatedAt { get; set; } = DateTime.UtcNow;
public DateTime? ProcessedAt { get; set; }
}
public class OrderService
{
private readonly Dictionary<string, Order> _orders = new();
public Order CreateOrder(string itemName, int quantity)
{
var order = new Order { ItemName = itemName, Quantity = quantity };
_orders[order.Id] = order;
return order;
}
public void ProcessOrder(string orderId)
{
if (!_orders.ContainsKey(orderId))
throw new InvalidOperationException($"Order {orderId} not found");
_orders[orderId].ProcessedAt = DateTime.UtcNow;
}
public Order GetOrder(string orderId) => _orders[orderId];
public List<Order> GetAllOrders() => _orders.Values.ToList();
}
tests/dotnet-test-experimental/exp-test-anti-patterns/fixtures/flaky-coupled/OrderServiceTests.cs
+91
View File
@@ -0,0 +1,91 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Reflection;
namespace FlakyCoupled.Tests;
[TestClass]
public class OrderServiceTests
{
private static List<string> _processedOrders = new();
[TestInitialize]
public void Setup()
{
// Note: _processedOrders is never cleared between tests
}
[TestMethod]
public void Test1()
{
var service = new OrderService();
var order = service.CreateOrder("item-1", 2);
_processedOrders.Add(order.Id);
Assert.IsNotNull(order);
}
[TestMethod]
public void Test2()
{
Assert.AreEqual(1, _processedOrders.Count);
var service = new OrderService();
service.ProcessOrder(_processedOrders[0]);
}
[TestMethod]
public void ProcessOrder_SetsTimestamp()
{
var service = new OrderService();
var order = service.CreateOrder("item-2", 1);
service.ProcessOrder(order.Id);
Thread.Sleep(2000);
var processed = service.GetOrder(order.Id);
Assert.IsTrue(processed.ProcessedAt.Value.Day == DateTime.Now.Day);
}
[TestMethod]
public void CreateOrder_InternalState_IsCorrect()
{
var service = new OrderService();
var order = service.CreateOrder("item-3", 5);
// Use reflection to verify internal state
var field = typeof(OrderService).GetField("_orders", BindingFlags.NonPublic | BindingFlags.Instance);
var orders = (Dictionary<string, Order>)field.GetValue(service);
Assert.AreEqual(1, orders.Count);
}
[TestMethod]
public void CreateOrder_MultipleItems_Test()
{
var service = new OrderService();
var order1 = service.CreateOrder("item-a", 1);
Assert.IsNotNull(order1);
Assert.IsNotNull(order1.Id);
var order2 = service.CreateOrder("item-b", 2);
Assert.IsNotNull(order2);
Assert.IsNotNull(order2.Id);
var order3 = service.CreateOrder("item-c", 3);
Assert.IsNotNull(order3);
Assert.IsNotNull(order3.Id);
Assert.AreNotEqual(order1.Id, order2.Id);
Assert.AreNotEqual(order2.Id, order3.Id);
Assert.AreNotEqual(order1.Id, order3.Id);
var all = service.GetAllOrders();
Assert.AreEqual(3, all.Count);
Assert.IsTrue(all.Any(o => o.ItemName == "item-a"));
Assert.IsTrue(all.Any(o => o.ItemName == "item-b"));
Assert.IsTrue(all.Any(o => o.ItemName == "item-c"));
Assert.IsTrue(all.All(o => o.Quantity > 0));
Assert.IsTrue(all.Sum(o => o.Quantity) == 6);
Console.WriteLine($"Created {all.Count} orders successfully");
}
[TestMethod]
public void ProcessOrder_InvalidId_Throws()
{
var service = new OrderService();
Assert.ThrowsException<Exception>(() => service.ProcessOrder("nonexistent"));
}
}