Testing Anti-Patterns

Rule: Test real behavior, not mock behavior. Never pollute production code with test-only methods.

When to use this skill

• When writing new test files or test cases in any testing framework (Jest, Vitest, pytest, RSpec, Go testing, JUnit)
• When modifying existing tests that use mocks, stubs, spies, or test doubles
• When considering whether to add a method or property to a production class that would only be used in tests
• When test setup involves creating mock objects or configuring mock behavior
• When deciding between testing with real dependencies versus mocked dependencies
• When tests are failing and you need to determine if the issue is in the code or in how mocks are configured
• When implementing fixtures, test data builders, or test utilities
• During test code reviews to ensure testing best practices are followed
• When test files contain assertions that check for mock existence (e.g., expect(screen.getByTestId('component-mock')))
• When refactoring production code and tests need to be updated accordingly
• When choosing isolation strategies for unit tests versus integration tests

Core Principles

1. Mocks are isolation tools, not test subjects - Assert on real behavior, not mock existence
2. Production code stays pure - Test utilities handle test-specific needs
3. Understand before mocking - Know what side effects you're removing
4. Complete mocks or none - Partial mocks create silent failures
5. TDD prevents anti-patterns - Write failing tests first to avoid testing mocks

Anti-Pattern 1: Testing Mock Behavior

Violation:

// ❌ BAD: Asserting on mock existence
test('renders sidebar', () => {
  render(<Page />);
  expect(screen.getByTestId('sidebar-mock')).toBeInTheDocument();
});

Why wrong:

• Verifies mock works, not component behavior
• Test passes with mock present regardless of real functionality
• Provides zero confidence about production behavior

Fix:

// ✅ GOOD: Test real component
test('renders sidebar', () => {
  render(<Page />);  // Use real sidebar
  expect(screen.getByRole('navigation')).toBeInTheDocument();
});

// OR if isolation required: Test Page's behavior, not mock presence
test('renders page with sidebar slot', () => {
  render(<Page />);
  expect(screen.getByTestId('sidebar-container')).toBeInTheDocument();
});

Detection rule:

IF assertion contains '*-mock' OR checks mock.toHaveBeenCalled():
  Ask: "Am I testing real behavior or mock existence?"
  IF testing mock existence → STOP, delete assertion or unmock

Anti-Pattern 2: Test-Only Methods in Production Code

Violation:

// ❌ BAD: Method only called from tests
class Session {
  async destroy() {
    await this._workspaceManager?.destroyWorkspace(this.id);
  }
}

afterEach(() => session.destroy());

Why wrong:

• Pollutes production API with test-specific code
• Risk of accidental production calls
• Violates YAGNI (You Aren't Gonna Need It)
• Confuses class responsibilities

Fix:

// ✅ GOOD: Test utilities handle cleanup
// Session class has no destroy() method

// In test-utils.ts
export async function cleanupSession(session: Session) {
  const workspace = session.getWorkspaceInfo();
  if (workspace) {
    await workspaceManager.destroyWorkspace(workspace.id);
  }
}

// In tests
afterEach(() => cleanupSession(session));

Detection rule:

BEFORE adding method to production class:
  1. Search codebase: Is this method only called from test files?
  2. Ask: Does this class own this resource's lifecycle?
  
  IF only used in tests OR class doesn't own lifecycle:
    STOP - Create test utility function instead

Anti-Pattern 3: Mocking Without Understanding Dependencies

Violation:

// ❌ BAD: Mock removes side effect test depends on
test('detects duplicate server', () => {
  vi.mock('ToolCatalog', () => ({
    discoverAndCacheTools: vi.fn().mockResolvedValue(undefined)
  }));

  await addServer(config);
  await addServer(config);  // Should throw but won't - config never written!
});

Why wrong:

• Mocked method writes config that duplicate detection needs
• Over-mocking "to be safe" breaks test logic
• Test passes/fails for wrong reasons

Fix:

// ✅ GOOD: Mock only external/slow operations
test('detects duplicate server', () => {
  vi.mock('MCPServerManager'); // Mock slow server startup only

  await addServer(config);  // Config written ✓
  await addServer(config);  // Duplicate detected ✓
});

Decision process:

BEFORE mocking:
  1. List method's side effects (DB writes, file I/O, API calls, state changes)
  2. Identify what test actually needs (duplicate detection needs config write)
  3. Mock ONLY external/slow operations, preserve test dependencies
  
  IF unsure what test needs:
    Run with real implementation FIRST
    Observe required behavior
    THEN mock minimally at lowest level
  
  Red flags indicating wrong approach:
    - "Mock this to be safe"
    - "Might be slow, better mock"
    - Can't explain why mocking
    - Mock setup longer than test

Anti-Pattern 4: Incomplete Mock Data Structures

Violation:

// ❌ BAD: Only fields you think you need
const mockResponse = {
  status: 'success',
  data: { userId: '123', name: 'Alice' }
  // Missing: metadata field
};

// Later: Silent failure when code accesses response.metadata.requestId

Why wrong:

• Partial mocks hide structural assumptions
• Downstream code may depend on omitted fields
• Tests pass, production fails
• False confidence in test coverage

Fix:

// ✅ GOOD: Complete structure matching real API
const mockResponse = {
  status: 'success',
  data: { userId: '123', name: 'Alice' },
  metadata: { requestId: 'req-789', timestamp: 1234567890 }
};

Mandatory process:

BEFORE creating mock data:
  1. Check API documentation or real response examples
  2. Include ALL fields from actual structure
  3. Use realistic values (not null/undefined unless API returns them)
  4. Verify mock matches real schema completely
  
  IF uncertain about structure:
    - Examine real API response
    - Include all documented fields
    - Add comment linking to API docs

Anti-Pattern 5: Tests as Afterthought

Violation:

Implementation complete → No tests → "Ready for review"

Why wrong:

• Testing is part of implementation, not optional
• Violates TDD workflow
• Cannot claim completion without tests

Fix - TDD cycle:

1. Write failing test (RED)
2. Implement minimal code (GREEN)
3. Refactor
4. THEN claim complete

When Mocks Signal Deeper Issues

Warning signs:

• Mock setup > 50% of test code
• Mocking everything to make test pass
• Mocks missing methods real components have
• Test breaks when mock implementation changes
• Can't explain why each mock is needed

Question to ask: "Should this be an integration test with real components?"

Complex mocks often indicate integration tests would be simpler and more valuable.

How TDD Prevents These Anti-Patterns

TDD workflow naturally avoids anti-patterns:

1. Write test first → Forces clarity on what you're testing
2. Watch it fail → Confirms test verifies real behavior, not mocks
3. Minimal implementation → Prevents test-only methods
4. Real dependencies first → See actual needs before mocking

Key insight: If you're testing mock behavior, you violated TDD by adding mocks before seeing test fail against real code.

Detection Checklist

Before finalizing any test, verify:

• No assertions on mock existence (*-mock test IDs, toHaveBeenCalled without behavior verification)
• No methods in production classes only called from test files
• Understand what each mock removes (side effects, dependencies, behavior)
• Mock data structures complete (all fields from real API/response)
• Tests written before or during implementation (not after)
• Mock setup < 50% of test code (if more, consider integration test)
• Can explain necessity of each mock

Quick Reference

Red Flags - Stop and Reconsider

When you encounter these, stop and reassess your approach:

• Assertions check for *-mock test IDs
• Methods only called in test files
• Mock setup > 50% of test code
• Test fails when you remove mock
• Can't explain why mock is needed
• Mocking "just to be safe"
• Mock missing methods real component has

Summary

Mocks isolate code under test from external dependencies. They are not the subject of tests.

When uncertain:

1. Write test with real dependencies first
2. Identify what's slow or external (API calls, DB queries, file I/O)
3. Mock only that specific dependency at lowest level
4. Verify test still validates real behavior, not mock presence