testing-anti-patterns

What this skill does

<skill_overview>
Tests must verify real behavior, not mock behavior; mocks are tools to isolate, not things to test.
</skill_overview>

<rigidity_level>
LOW FREEDOM - The 3 Iron Laws are absolute (never test mocks, never add test-only methods, never mock without understanding). Apply gate functions strictly.
</rigidity_level>

<quick_reference>
## The 3 Iron Laws

1. **NEVER test mock behavior** → Test real component behavior
2. **NEVER add test-only methods to production** → Use test utilities instead
3. **NEVER mock without understanding** → Know dependencies before mocking

## Gate Functions (Use Before Action)

**Before asserting on any mock:**
- Ask: "Am I testing real behavior or mock existence?"
- If mock existence → STOP, delete assertion

**Before adding method to production:**
- Ask: "Is this only used by tests?"
- If yes → STOP, put in test utilities

**Before mocking:**
- Ask: "What side effects does real method have?"
- Ask: "Does test depend on those side effects?"
- If depends → Mock lower level, not this method
</quick_reference>

<when_to_use>
- Writing new tests
- Adding mocks to tests
- Tempted to add method only tests will use
- Test failing and considering mocking something
- Unsure whether to mock a dependency
- Test setup becoming complex with mocks

**Critical moment:** Before you add a mock or test-only method, use this skill's gate functions.
</when_to_use>

<the_iron_laws>
## Law 1: Never Test Mock Behavior

**Anti-pattern:**
```rust
// ❌ BAD: Testing that mock exists
#[test]
fn test_processes_request() {
    let mock_service = MockApiService::new();
    let handler = RequestHandler::new(Box::new(mock_service));

    // Testing mock existence, not behavior
    assert!(handler.service().is_mock());
}
```

**Why wrong:** Verifies mock works, not that code works.

**Fix:**
```rust
// ✅ GOOD: Test real behavior
#[test]
fn test_processes_request() {
    let service = TestApiService::new();  // Real implementation or full fake
    let handler = RequestHandler::new(Box::new(service));

    let result = handler.process_request("data");
    assert_eq!(result.status, StatusCode::OK);
}
```

---

## Law 2: Never Add Test-Only Methods to Production

**Anti-pattern:**
```rust
// ❌ BAD: reset() only used in tests
pub struct Connection {
    pool: Arc<ConnectionPool>,
}

impl Connection {
    pub fn reset(&mut self) {  // Looks like production API!
        self.pool.clear_all();
    }
}

// In tests
#[test]
fn test_something() {
    let mut conn = Connection::new();
    conn.reset();  // Test-only method
}
```

**Why wrong:**
- Production code polluted with test-only methods
- Dangerous if accidentally called in production
- Confuses object lifecycle with entity lifecycle

**Fix:**
```rust
// ✅ GOOD: Test utilities handle cleanup
// Connection has no reset()

// In tests/test_utils.rs
pub fn cleanup_connection(conn: &Connection) {
    if let Some(pool) = conn.get_pool() {
        pool.clear_test_data();
    }
}

// In tests
#[test]
fn test_something() {
    let conn = Connection::new();
    cleanup_connection(&conn);
}
```

---

## Law 3: Never Mock Without Understanding

**Anti-pattern:**
```rust
// ❌ BAD: Mock breaks test logic
#[test]
fn test_detects_duplicate_server() {
    // Mock prevents config write that test depends on!
    let mut config_manager = MockConfigManager::new();
    config_manager.expect_add_server()
        .returning(|_| Ok(()));  // No actual config write!

    config_manager.add_server(&config).unwrap();
    config_manager.add_server(&config).unwrap();  // Should fail - but won't!
}
```

**Why wrong:** Mocked method had side effect test depended on (writing config).

**Fix:**
```rust
// ✅ GOOD: Mock at correct level
#[test]
fn test_detects_duplicate_server() {
    // Mock the slow part, preserve behavior test needs
    let server_manager = MockServerManager::new();  // Just mock slow server startup
    let config_manager = ConfigManager::new_with_manager(server_manager);

    config_manager.add_server(&config).unwrap();  // Config written
    let result = config_manager.add_server(&config);  // Duplicate detected ✓
    assert!(result.is_err());
}
```
</the_iron_laws>

<gate_functions>
## Gate Function 1: Before Asserting on Mock

```
BEFORE any assertion that checks mock elements:

1. Ask: "Am I testing real component behavior or just mock existence?"

2. If testing mock existence:
   STOP - Delete the assertion or unmock the component

3. Test real behavior instead
```

**Examples of mock existence testing (all wrong):**
- `assert!(handler.service().is_mock())`
- `XCTAssertTrue(manager.delegate is MockDelegate)`
- `expect(component.database).toBe(mockDb)`

---

## Gate Function 2: Before Adding Method to Production

```
BEFORE adding any method to production class:

1. Ask: "Is this only used by tests?"

2. If yes:
   STOP - Don't add it
   Put it in test utilities instead

3. Ask: "Does this class own this resource's lifecycle?"

4. If no:
   STOP - Wrong class for this method
```

**Red flags:**
- Method named `reset()`, `clear()`, `cleanup()` in production class
- Method only has `#[cfg(test)]` callers
- Method added "for testing purposes"

---

## Gate Function 3: Before Mocking

```
BEFORE mocking any method:

STOP - Don't mock yet

1. Ask: "What side effects does the real method have?"
2. Ask: "Does this test depend on any of those side effects?"
3. Ask: "Do I fully understand what this test needs?"

If depends on side effects:
  → Mock at lower level (the actual slow/external operation)
  → OR use test doubles that preserve necessary behavior
  → NOT the high-level method the test depends on

If unsure what test depends on:
  → Run test with real implementation FIRST
  → Observe what actually needs to happen
  → THEN add minimal mocking at the right level
```

**Red flags:**
- "I'll mock this to be safe"
- "This might be slow, better mock it"
- Mocking without understanding dependency chain
</gate_functions>

<examples>
<example>
<scenario>Developer tests mock behavior instead of real behavior</scenario>

<code>
#[test]
fn test_user_service_initialized() {
    let mock_db = MockDatabase::new();
    let service = UserService::new(mock_db);

    // Testing that mock exists
    assert_eq!(service.database().connection_string(), "mock://test");
    assert!(service.database().is_test_mode());
}
</code>

<why_it_fails>
- Assertions check mock properties, not service behavior
- Test passes when mock is correct, fails when mock is wrong
- Tells you nothing about whether UserService works
- Would pass even if UserService.new() does nothing
- False confidence - mock works, but does service work?
</why_it_fails>

<correction>
**Apply Gate Function 1:**

"Am I testing real behavior or mock existence?"
→ Testing mock existence (connection_string(), is_test_mode() are mock properties)

**Fix:**

```rust
#[test]
fn test_user_service_creates_user() {
    let db = TestDatabase::new();  // Real test implementation
    let service = UserService::new(db);

    // Test real behavior
    let user = service.create_user("alice", "[email protected]").unwrap();
    assert_eq!(user.name, "alice");
    assert_eq!(user.email, "[email protected]");

    // Verify user was saved
    let retrieved = service.get_user(user.id).unwrap();
    assert_eq!(retrieved.name, "alice");
}
```

**What you gain:**
- Tests actual UserService behavior
- Validates create and retrieve work
- Would fail if service broken (even with working mock)
- Confidence service actually works
</correction>
</example>

<example>
<scenario>Developer adds test-only method to production class</scenario>

<code>
// Production code
pub struct Database {
    pool: ConnectionPool,
}

impl Database {
    pub fn new() -> Self { /* ... */ }

    // Added "for testing"
    pub fn reset(&mut self) {
        self.pool.clear();
        self.pool.reinitialize();
    }
}

// Tests
#[test]
fn test_user_creation() {
    let mut db = Database::new();
    // ... test logic ...
    db.r