125 lines
4.8 KiB
Markdown
125 lines
4.8 KiB
Markdown
---
|
|
name: tdd-test-refactoring
|
|
user-invocable: true
|
|
description: "Use during Green/Refactor phase to eliminate test duplication. Extracts shared setup into TEST_F fixtures, removes redundant expectations, and keeps tests minimal and focused. Run after tests pass but before committing."
|
|
---
|
|
|
|
# TDD Test Refactoring Patterns
|
|
|
|
This skill complements **tdd-minimalism** by focusing on **test code quality** during the Green and Refactor phases. After implementation is minimal and passing, apply these patterns to keep the test suite clean.
|
|
|
|
## Principle
|
|
|
|
Tests are code too — they accumulate duplication and cruft just like implementation. Each test should test **one distinct scenario** without repeating setup or expectations already covered by other tests.
|
|
|
|
## Refactoring Checklist
|
|
|
|
After tests pass (Green phase), review the test file for these patterns:
|
|
|
|
### 1. Extract Shared Setup into TEST_F
|
|
|
|
If multiple tests construct the same object(s) with the same configuration, extract the common setup into a fixture.
|
|
|
|
**Before:**
|
|
```cpp
|
|
TEST(VendingMachineTest, configured_machine_delivers_can) {
|
|
VendingMachine machine;
|
|
machine.Configure(Choice::Cola, Can::Coke);
|
|
EXPECT_EQ(machine.Deliver(Choice::Cola), Can::Coke);
|
|
}
|
|
|
|
TEST(VendingMachineTest, multiple_choices_deliver_different_cans) {
|
|
VendingMachine machine;
|
|
machine.Configure(Choice::Cola, Can::Coke);
|
|
machine.Configure(Choice::Fanta, Can::Orange);
|
|
EXPECT_EQ(machine.Deliver(Choice::Cola), Can::Coke);
|
|
EXPECT_EQ(machine.Deliver(Choice::Fanta), Can::Orange);
|
|
}
|
|
```
|
|
|
|
**After:**
|
|
```cpp
|
|
class VendingMachineTest : public ::testing::Test {
|
|
protected:
|
|
void SetUp() override {
|
|
machine.Configure(Choice::Cola, Can::Coke);
|
|
}
|
|
VendingMachine machine;
|
|
};
|
|
|
|
TEST_F(VendingMachineTest, configured_machine_delivers_can) {
|
|
EXPECT_EQ(machine.Deliver(Choice::Cola), Can::Coke);
|
|
}
|
|
|
|
TEST_F(VendingMachineTest, multiple_choices_deliver_different_cans) {
|
|
machine.Configure(Choice::Fanta, Can::Orange);
|
|
EXPECT_EQ(machine.Deliver(Choice::Cola), Can::Coke);
|
|
EXPECT_EQ(machine.Deliver(Choice::Fanta), Can::Orange);
|
|
}
|
|
```
|
|
|
|
**When to keep TEST instead of TEST_F:**
|
|
- The test needs a **different** or **empty** initial state (e.g., an unconfigured machine)
|
|
- The test's setup is entirely unique and not shared with any other test
|
|
|
|
### 2. Remove Redundant Expectations
|
|
|
|
If a test is a **strict superset** of another test (covers the same scenario plus more), the subset test may be redundant.
|
|
|
|
**Check:** Does the superset test already assert everything the subset test asserts?
|
|
|
|
| Scenario | Keep? |
|
|
|----------|-------|
|
|
| Superset test covers all assertions of subset | Consider removing subset if it adds no unique coverage |
|
|
| Superset test covers subset's assertions **plus** additional ones | Keep both if subset documents a specific edge case clearly |
|
|
| Tests cover different branches of the same logic | Keep both — they test different paths |
|
|
|
|
### 3. Name Tests by Scenario, Not by Order
|
|
|
|
Test names should describe **what scenario** they cover, not their sequence number.
|
|
|
|
| ❌ Bad | ✅ Good |
|
|
|--------|--------|
|
|
| `test1`, `test2`, `test3` | `choiceless_machine_deliver_nothing` |
|
|
| `first_test`, `second_test` | `configured_machine_delivers_can` |
|
|
| `test_feature_x` | `multiple_choices_deliver_different_cans` |
|
|
|
|
### 4. One Logical Assertion Per Test
|
|
|
|
Each test should verify **one behavior**. Avoid asserting unrelated things in the same test.
|
|
|
|
**When multiple assertions are OK:**
|
|
- They verify the **same result** from different angles (e.g., `.has_value()` and `== expected`)
|
|
- They verify a **post-condition** that naturally produces multiple observable effects
|
|
|
|
**When to split:**
|
|
- Assertions exercise **different code paths** or **different methods**
|
|
- One assertion failing hides whether the other would also fail
|
|
|
|
### 5. Keep Tests Independent
|
|
|
|
Tests must not depend on each other's side effects. Each test (or each fixture instance) starts fresh.
|
|
|
|
- ✅ Each `TEST_F` gets a **fresh** `SetUp()`
|
|
- ✅ No shared mutable state between tests
|
|
- ❌ No test ordering dependencies
|
|
|
|
## Post-Refactoring Verification
|
|
|
|
After applying test refactoring:
|
|
|
|
- [ ] Run the test suite — all tests still pass
|
|
- [ ] Run clang-format — all files comply
|
|
- [ ] No test is a complete subset of another (unless documenting a specific edge case)
|
|
- [ ] No duplicate setup code across tests using the same initial state
|
|
- [ ] Test names describe the scenario, not the sequence
|
|
|
|
## Relationship to tdd-minimalism
|
|
|
|
| Skill | Focus | When |
|
|
|-------|-------|------|
|
|
| tdd-minimalism | Implementation code — eliminate overengineering | Before writing implementation |
|
|
| tdd-test-refactoring | Test code — eliminate duplication and cruft | After tests pass, before commit |
|
|
|
|
Run **tdd-minimalism** before implementation. Run **tdd-test-refactoring** after tests pass but before committing.
|