Skip to content

Testing and Code Review Practices

Section Overview

Comprehensive testing strategies and code review processes that ensure reliability, maintain quality, and promote knowledge sharing across the development team.


Unit Testing Best Practices

Test Organization and Structure

Core Principle: Unit tests must be organized systematically to maintain clarity, scalability, and ease of maintenance, following a consistent project-wide structure.

Key Guidelines:

  • Mirror the production code structure in test organization
  • Group related tests logically by feature or module
  • Maintain clear separation between different types of tests
  • Follow consistent file naming patterns across the project

Why This Matters

Well-organized tests improve maintainability, make it easier to locate specific tests, and help ensure comprehensive coverage. Systematic organization reduces cognitive load during development and troubleshooting.

Standard Directory Structure:

src/
├── user/
│   ├── authentication.py
│   ├── profile.py
│   └── settings.py
tests/
├── unit/
│   ├── user/
│   │   ├── test_authentication.py
│   │   ├── test_profile.py
│   │   └── test_settings.py
├── integration/
├── e2e/
└── fixtures/

Test Naming Conventions

Pattern: test_[unit]_[scenario]_[expected_result]

Examples:

# tests/unit/user/test_authentication.py
def test_user_login_with_valid_credentials_should_succeed():
    user = create_test_user()
    result = authenticate_user(user.email, user.password)
    assert result.is_authenticated

def test_user_login_with_invalid_password_should_fail():
    user = create_test_user()
    result = authenticate_user(user.email, "wrong_password")
    assert not result.is_authenticated

def test_user_login_with_nonexistent_email_should_raise_error():
    with pytest.raises(UserNotFoundError):
        authenticate_user("nonexistent@example.com", "password")
// tests/unit/user/authentication.test.js
describe('User Authentication', () => {
  test('should authenticate successfully with valid credentials', () => {
    const user = createTestUser();
    const result = authenticateUser(user.email, user.password);
    expect(result.isAuthenticated).toBe(true);
  });

  test('should reject login with invalid password', () => {
    const user = createTestUser();
    const result = authenticateUser(user.email, 'wrongPassword');
    expect(result.isAuthenticated).toBe(false);
  });

  test('should throw error when user does not exist', () => {
    expect(() => {
      authenticateUser('nonexistent@example.com', 'password');
    }).toThrow(UserNotFoundError);
  });
});
// src/test/java/com/company/user/AuthenticationTest.java
public class AuthenticationTest {
    @Test
    public void shouldAuthenticateSuccessfullyWithValidCredentials() {
        User user = createTestUser();
        AuthResult result = authenticateUser(user.getEmail(), user.getPassword());
        assertTrue(result.isAuthenticated());
    }

    @Test
    public void shouldRejectLoginWithInvalidPassword() {
        User user = createTestUser();
        AuthResult result = authenticateUser(user.getEmail(), "wrongPassword");
        assertFalse(result.isAuthenticated());
    }

    @Test(expected = UserNotFoundException.class)
    public void shouldThrowExceptionWhenUserDoesNotExist() {
        authenticateUser("nonexistent@example.com", "password");
    }
}

Naming Guidelines

  • Use complete words, avoid abbreviations
  • Names should read as specifications
  • Include edge cases and conditions
  • Follow language-specific conventions (snake_case vs camelCase)

Mocking and Stubbing Strategies

Core Principle: Tests must isolate the unit under test from its dependencies through effective use of mocks, stubs, and test doubles.

When to Use Mocks:

Scenario Approach
External APIs Mock to avoid network calls
Database Operations Use in-memory or test instance
File System Mock file operations
Time-dependent Code Mock time functions
Third-party Services Stub expected responses

Implementation Examples:

# Using pytest-mock
def test_user_service_sends_welcome_email(mocker):
    # Arrange: Mock the email service
    mock_email = mocker.patch('services.EmailService')
    mock_email.send_welcome.return_value = True
    user_service = UserService(mock_email)

    # Act
    user = user_service.create_user("test@example.com", "password")

    # Assert
    mock_email.send_welcome.assert_called_once_with(
        "test@example.com",
        user_name=user.name
    )

def test_payment_processing_handles_timeout(mocker):
    # Mock gateway to simulate timeout
    mock_gateway = mocker.Mock()
    mock_gateway.process.side_effect = TimeoutError("Timeout")
    payment_service = PaymentService(mock_gateway)

    # Verify proper error handling
    with pytest.raises(PaymentProcessingError) as exc:
        payment_service.charge(amount=100, card="1234")

    assert "Timeout" in str(exc.value)
// Using Jest
test('should send welcome email on user creation', async () => {
  const mockEmailService = {
    sendWelcome: jest.fn().mockResolvedValue(true)
  };
  const userService = new UserService(mockEmailService);

  const user = await userService.createUser(
    'test@example.com',
    'password'
  );

  expect(mockEmailService.sendWelcome).toHaveBeenCalledWith(
    'test@example.com',
    expect.objectContaining({ userName: user.name })
  );
});

test('should handle payment gateway timeout', async () => {
  const mockGateway = {
    process: jest.fn().mockRejectedValue(
      new Error('Gateway timeout')
    )
  };
  const paymentService = new PaymentService(mockGateway);

  await expect(
    paymentService.charge({ amount: 100, card: '1234' })
  ).rejects.toThrow('Gateway timeout');
});
// Using Mockito
@Test
public void shouldSendWelcomeEmailOnUserCreation() {
    EmailService mockEmail = mock(EmailService.class);
    when(mockEmail.sendWelcome(anyString())).thenReturn(true);
    UserService userService = new UserService(mockEmail);

    User user = userService.createUser("test@example.com", "password");

    verify(mockEmail).sendWelcome(
        eq("test@example.com"),
        eq(user.getName())
    );
}

@Test(expected = PaymentProcessingException.class)
public void shouldHandlePaymentGatewayTimeout() {
    PaymentGateway mockGateway = mock(PaymentGateway.class);
    when(mockGateway.process(any())).thenThrow(
        new TimeoutException("Gateway timeout")
    );
    PaymentService paymentService = new PaymentService(mockGateway);

    paymentService.charge(100, "1234");
}

Common Mocking Pitfalls

  • Over-mocking everything reduces test value
  • Mocking implementation details creates brittle tests
  • Mocks that don't reflect real behavior mask bugs
  • Excessive setup complexity indicates poor design

Integration and End-to-End Testing

Test Suite Organization

Hierarchical Structure:

tests/
├── unit/                    # Fast, isolated tests
│   ├── services/
│   ├── models/
│   └── utils/
├── integration/             # Tests with real dependencies
│   ├── auth/
│   │   ├── login/
│   │   ├── registration/
│   │   └── password_reset/
│   ├── payments/
│   │   ├── processing/
│   │   └── refunds/
│   └── api/
├── e2e/                     # Complete user workflows
│   ├── workflows/
│   └── scenarios/
└── fixtures/                # Shared test data

Testing with Realistic Environments

Environment Setup:

# docker-compose.test.yml
version: '3.8'

services:
  app:
    build:
      context: .
      dockerfile: Dockerfile.test
    environment:
      - NODE_ENV=test
      - DB_HOST=test-db
      - REDIS_HOST=test-redis
    depends_on:
      test-db:
        condition: service_healthy
      test-redis:
        condition: service_healthy

  test-db:
    image: postgres:14
    environment:
      - POSTGRES_DB=testdb
      - POSTGRES_USER=test
      - POSTGRES_PASSWORD=testpass
    healthcheck:
      test: ["CMD-SHELL", "pg_isready -U test"]
      interval: 5s
      timeout: 5s
      retries: 5
    tmpfs:
      - /var/lib/postgresql/data

  test-redis:
    image: redis:7-alpine
    healthcheck:
      test: ["CMD", "redis-cli", "ping"]
      interval: 5s
      timeout: 5s
      retries: 5
# tests/fixtures/data_manager.py
class TestDataManager:
    def __init__(self, database):
        self.db = database
        self.created_records = []

    def seed_base_data(self):
        """Load foundational test data"""
        self.db.load_fixtures('base_data.json')

    def create_test_user(self, **kwargs):
        """Create a test user with sensible defaults"""
        defaults = {
            'email': f'test-{uuid4()}@example.com',
            'username': f'testuser_{uuid4().hex[:8]}',
            'password': 'Test123!@#',
            'is_active': True
        }
        user_data = {**defaults, **kwargs}
        user = self.db.users.create(**user_data)
        self.created_records.append(('users', user.id))
        return user

    def create_test_order(self, user_id=None, **kwargs):
        """Create a test order"""
        if user_id is None:
            user = self.create_test_user()
            user_id = user.id

        defaults = {
            'user_id': user_id,
            'status': 'pending',
            'total_amount': 100.00
        }
        order_data = {**defaults, **kwargs}
        order = self.db.orders.create(**order_data)
        self.created_records.append(('orders', order.id))
        return order

    def cleanup(self):
        """Clean up all created test data"""
        for table, record_id in reversed(self.created_records):
            self.db[table].delete(record_id)
        self.created_records.clear()

Handling External Dependencies

Service Virtualization Approach:

Dependency Strategy Implementation
Payment Gateways Mock responses Custom mock class
Email Services Stub responses Test-specific implementation
Third-party APIs Record/replay VCR.py, nock
Databases Containerized Docker
Message Queues In-memory Fake implementation

Mock Payment Gateway Example:

# tests/mocks/payment_gateway.py
class MockPaymentGateway:
    """Simulates payment gateway for testing"""

    def __init__(self):
        self.transactions = []
        self.scenarios = {
            'success': {'status': 'approved', 'transaction_id': 'mock-123'},
            'insufficient_funds': {'status': 'declined', 'error': 'INS_FUND_001'},
            'card_declined': {'status': 'declined', 'error': 'CARD_DEC_001'},
            'service_error': {'status': 'error', 'error': 'SVC_ERR_001'}
        }

    def process_payment(self, amount, card):
        """Process payment with configurable scenarios"""
        if amount > 10000:
            response = self.scenarios['service_error']
        elif not self._is_valid_card(card):
            response = self.scenarios['card_declined']
        elif card.get('balance', float('inf')) < amount:
            response = self.scenarios['insufficient_funds']
        else:
            response = self.scenarios['success']

        self.transactions.append({
            'amount': amount,
            'card': card,
            'response': response,
            'timestamp': datetime.now()
        })

        return response

    def _is_valid_card(self, card):
        required_fields = ['number', 'expiry', 'cvv']
        return all(field in card for field in required_fields)

    def get_transaction_history(self):
        return self.transactions

    def reset(self):
        self.transactions.clear()

Retry Pattern:

from functools import wraps
import time

def retry(max_attempts=3, delay=1, backoff=2, exceptions=(Exception,)):
    """Retry decorator with exponential backoff"""
    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            attempt = 0
            current_delay = delay

            while attempt < max_attempts:
                try:
                    return func(*args, **kwargs)
                except exceptions as e:
                    attempt += 1
                    if attempt == max_attempts:
                        raise
                    time.sleep(current_delay)
                    current_delay *= backoff

            return None
        return wrapper
    return decorator

# Usage
@retry(max_attempts=3, delay=1, exceptions=(ConnectionError, TimeoutError))
def test_external_service():
    service = ExternalService()
    response = service.make_request()
    assert response.status_code == 200

Testing External Dependencies

  • Use containerized versions of external services
  • Mock services should match real API contracts
  • Record and replay real responses (VCR pattern)
  • Test error scenarios: timeouts, rate limits, service unavailable
  • Document all external service requirements

Test Coverage Guidelines

Coverage Standards

Minimum Coverage by Component:

Component Type Minimum Target
Critical Business Logic 95% 100%
API Endpoints 90% 95%
Service Layer 85% 90%
UI Components 80% 90%
Utility Functions 70% 85%
Configuration 60% 75%

Require 100% Coverage For:

  • Financial calculations and transactions
  • Authentication and authorization logic
  • Data validation and sanitization
  • Payment processing
  • User data handling
  • Security-critical functions
  • Regulatory compliance code

Coverage Configuration

# .coveragerc
[run]
source = src/
omit =
    */tests/*
    */migrations/*
    */venv/*

[report]
precision = 2
exclude_lines =
    pragma: no cover
    def __repr__
    raise AssertionError
    raise NotImplementedError
    if __name__ == .__main__.:

[html]
directory = coverage_report
{
  "jest": {
    "collectCoverageFrom": [
      "src/**/*.{js,jsx,ts,tsx}",
      "!src/**/*.d.ts",
      "!src/**/*.test.{js,jsx,ts,tsx}"
    ],
    "coverageThreshold": {
      "global": {
        "branches": 80,
        "functions": 80,
        "lines": 80,
        "statements": 80
      },
      "./src/critical/": {
        "branches": 100,
        "functions": 100,
        "lines": 100,
        "statements": 100
      }
    },
    "coverageReporters": ["text", "html", "lcov"]
  }
}
<!-- pom.xml -->
<plugin>
    <groupId>org.jacoco</groupId>
    <artifactId>jacoco-maven-plugin</artifactId>
    <version>0.8.8</version>
    <executions>
        <execution>
            <goals>
                <goal>prepare-agent</goal>
            </goals>
        </execution>
        <execution>
            <id>report</id>
            <phase>test</phase>
            <goals>
                <goal>report</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Coverage Reporting in CI/CD

name: Test Coverage

on:
  push:
    branches: [ main, develop ]
  pull_request:
    branches: [ main, develop ]

jobs:
  coverage:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3

      - name: Setup Node.js
        uses: actions/setup-node@v3
        with:
          node-version: '18'
          cache: 'npm'

      - name: Install dependencies
        run: npm ci

      - name: Run tests with coverage
        run: npm run test:coverage

      - name: Upload to Codecov
        uses: codecov/codecov-action@v3
        with:
          files: ./coverage/lcov.info
          fail_ci_if_error: true

      - name: Archive coverage reports
        uses: actions/upload-artifact@v3
        with:
          name: coverage-report
          path: coverage/
          retention-days: 30

Coverage Best Practices

  • Focus on meaningful coverage, not just percentages
  • Test behavior, not implementation details
  • Identify and prioritize critical paths
  • Review coverage reports regularly
  • Track coverage trends over time

Code Review Process

Pull Request Template

## Description

What changed and why?

---

## Type of Change

- [ ] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Configuration change
- [ ] Code refactoring

---

## Testing

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] All tests passing
- [ ] Manual testing completed

---

## Code Quality Checklist

- [ ] Code follows style guidelines
- [ ] Self-review completed
- [ ] Comments added for complex logic
- [ ] No debugging code left in
- [ ] No sensitive data exposed

---

## Security Considerations

- [ ] Input validation implemented
- [ ] Authentication/authorization checked
- [ ] Security implications considered
- [ ] No hardcoded credentials
- [ ] Sensitive data handling reviewed

---

## Performance Impact

- [ ] Performance implications assessed
- [ ] Database queries optimized
- [ ] No obvious performance issues

---

## Documentation

- [ ] README updated (if needed)
- [ ] API documentation updated (if needed)
- [ ] Code comments added
- [ ] CHANGELOG updated

---

## Related Issues

Closes #123, Relates to #456

---

## Additional Notes

Any other information for reviewers?

Review Classifications

Mandatory Reviews (2+ Approvals Required):

Critical Changes

  • Financial and payment processing logic
  • Authentication and authorization changes
  • Database schema modifications
  • API contract changes
  • Security-related code
  • Infrastructure configuration
  • Dependency version updates

Standard Reviews (1 Approval Required):

Normal Changes

  • Business logic modifications
  • New features
  • Bug fixes
  • Performance improvements
  • Code refactoring

Optional Reviews:

  • Documentation-only updates
  • Test additions without code changes
  • Minor UI copy changes
  • Development tooling updates

Review Checklists

Architecture and Design:

- [ ] Appropriate design patterns used (not over-engineered)
- [ ] SOLID principles followed
- [ ] Clear separation of concerns
- [ ] Minimal coupling between components
- [ ] Design scales with future requirements
- [ ] Code is maintainable and readable

Code Quality:

- [ ] Logical organization and structure
- [ ] Appropriate modularization
- [ ] No code duplication
- [ ] Naming conventions followed
- [ ] Complex logic is documented
- [ ] Functions are reasonably sized

Error Handling:

- [ ] Proper exception handling
- [ ] Meaningful error messages
- [ ] Edge cases considered
- [ ] Graceful degradation implemented
- [ ] Error recovery is appropriate

Security:

- [ ] All user inputs validated
- [ ] SQL injection prevention in place
- [ ] XSS prevention implemented
- [ ] CSRF protection configured
- [ ] Sensitive data properly handled
- [ ] Authentication/authorization verified
- [ ] No hardcoded credentials

Feedback Guidelines

Structure for Feedback:

  1. Observation - What you found
  2. Impact - Why it matters
  3. Suggestion - How to improve

Example Feedback:

## Suggestion: Improve Error Handling

The current code catches all exceptions generically:

try:
    result = process_payment(order)
except Exception as e:
    return error_response()


This makes debugging difficult and masks the actual error from monitoring.

Consider handling specific exception types:

try:
    result = process_payment(order)
except PaymentGatewayError as e:
    logger.error(f"Payment gateway error: {e}")
    return error_response("Payment processing failed", status=503)
except InsufficientFundsError as e:
    logger.warning(f"Insufficient funds for order {order.id}")
    return error_response("Insufficient funds", status=402)
except Exception as e:
    logger.exception(f"Unexpected error: {e}")
    return error_response("Internal error", status=500)

Benefits: Better error visibility, appropriate HTTP status codes, easier debugging.

Code Review Best Practices

  • Read the code thoroughly, don't skim
  • Ask questions if something is unclear
  • Acknowledge good work, not just problems
  • Be respectful and professional
  • Focus on the code, not the person
  • Block PRs only for serious issues

Continuous Integration and Deployment

CI/CD Pipeline Configuration

GitHub Actions Example:

name: CI/CD Pipeline

on:
  push:
    branches: [ main, develop ]
  pull_request:
    branches: [ main, develop ]

jobs:
  test:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3

      - name: Setup Node.js
        uses: actions/setup-node@v3
        with:
          node-version: '18'
          cache: 'npm'

      - name: Install dependencies
        run: npm ci

      - name: Run linting
        run: npm run lint

      - name: Run type checking
        run: npm run type-check

      - name: Run unit tests
        run: npm run test:unit

      - name: Run integration tests
        run: npm run test:integration

      - name: Run coverage
        run: npm run test:coverage

      - name: Security scanning
        uses: snyk/actions/node@master
        env:
          SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}

  deploy-staging:
    needs: test
    if: github.ref == 'refs/heads/main'
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - name: Deploy to staging
        run: ./scripts/deploy.sh staging

  deploy-production:
    needs: deploy-staging
    if: github.ref == 'refs/heads/main'
    runs-on: ubuntu-latest
    environment: production
    steps:
      - uses: actions/checkout@v3
      - name: Deploy to production
        run: ./scripts/deploy.sh production

Jenkins Pipeline Example:

pipeline {
    agent any

    environment {
        NODE_ENV = 'test'
    }

    stages {
        stage('Build') {
            steps {
                sh 'npm ci'
                sh 'npm run build'
            }
        }

        stage('Test') {
            parallel {
                stage('Lint') {
                    steps {
                        sh 'npm run lint'
                    }
                }
                stage('Unit Tests') {
                    steps {
                        sh 'npm run test:unit'
                    }
                }
                stage('Integration Tests') {
                    steps {
                        sh 'npm run test:integration'
                    }
                }
            }
        }

        stage('Security Scan') {
            steps {
                sh 'npm audit'
                sh 'snyk test'
            }
        }

        stage('Deploy Staging') {
            when { branch 'main' }
            steps {
                sh './scripts/deploy.sh staging'
                sh 'npm run test:smoke'
            }
        }

        stage('Deploy Production') {
            when { branch 'main' }
            input {
                message "Deploy to production?"
                ok "Yes"
            }
            steps {
                sh './scripts/deploy.sh production'
            }
        }
    }

    post {
        always {
            junit '**/test-results/*.xml'
            publishHTML(target: [
                reportDir: 'coverage',
                reportFiles: 'index.html',
                reportName: 'Coverage Report'
            ])
        }
        failure {
            sh './scripts/rollback.sh'
        }
    }
}

Deployment Strategies

Blue-Green Deployment

Maintains two identical production environments for zero-downtime deployments and quick rollbacks.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: app-blue
spec:
  replicas: 3
  selector:
    matchLabels:
      app: myapp
      version: blue
  template:
    metadata:
      labels:
        app: myapp
        version: blue
    spec:
      containers:
      - name: myapp
        image: myapp:1.0
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: app-green
spec:
  replicas: 3
  selector:
    matchLabels:
      app: myapp
      version: green
  template:
    metadata:
      labels:
        app: myapp
        version: green
    spec:
      containers:
      - name: myapp
        image: myapp:2.0

Canary Deployment

Gradually rolls out changes to a small subset of users before full deployment.

def deploy_canary(new_version, initial_percent=5):
    """Canary deployment with monitoring"""
    try:
        deploy_version(new_version, percent=initial_percent)

        thresholds = {
            'error_rate': 1.0,
            'latency_p95': 500,
            'cpu_usage': 85,
            'memory_usage': 90
        }

        stages = [25, 50, 75, 100]

        for target_percent in stages:
            if monitor_deployment(thresholds):
                deploy_version(new_version, percent=target_percent)
            else:
                rollback_deployment()
                raise Exception("Deployment metrics exceeded thresholds")

        return True

    except Exception as e:
        logging.error(f"Canary deployment failed: {str(e)}")
        rollback_deployment()
        return False

Rollback and Disaster Recovery

Automated Rollback Triggers:

Trigger Threshold Action
Error rate Exceeds 1% Automatic rollback
Response time Exceeds 500ms Automatic rollback
CPU usage Exceeds 85% Alert and manual review
Memory usage Exceeds 90% Automatic rollback

Rollback Script:

#!/bin/bash

PREVIOUS_VERSION=$(cat ./previous_version)
DEPLOYMENT_NAME="myapp"
NAMESPACE="production"
MAX_RETRIES=3

check_health() {
    local endpoint=$1
    local retry_count=0

    while [ $retry_count -lt $MAX_RETRIES ]; do
        if curl -f "http://${endpoint}/health" > /dev/null 2>&1; then
            return 0
        fi
        retry_count=$((retry_count + 1))
        sleep 5
    done
    return 1
}

perform_rollback() {
    echo "Starting rollback to version ${PREVIOUS_VERSION}"
    incident_start=$(date +%s)

    kubectl rollout undo deployment/${DEPLOYMENT_NAME} -n ${NAMESPACE}

    if ! kubectl rollout status deployment/${DEPLOYMENT_NAME} -n ${NAMESPACE} --timeout=300s; then
        echo "Rollback failed to complete"
        exit 1
    fi

    if ! check_health "${DEPLOYMENT_NAME}.${NAMESPACE}"; then
        echo "Health check failed after rollback"
        exit 1
    fi

    incident_end=$(date +%s)
    recovery_time=$((incident_end - incident_start))

    echo "Rollback completed in ${recovery_time} seconds"
    update_monitoring_system
    generate_incident_report ${incident_start} ${incident_end}
}

perform_rollback

Disaster Recovery Procedures:

Critical Recovery Elements

  • Automated health checks on critical services
  • Automated alerts for system anomalies
  • Regular database backup verification
  • Define and document Recovery Time Objectives (RTO)
  • Implement automated recovery procedures
  • Regular disaster recovery drills
  • Regular database backups with transaction logs
  • Point-in-time recovery capabilities
  • Detailed incident response procedures
  • System dependencies and recovery order documentation

Deployment Checklist:

  • All tests passing
  • Code review completed and approved
  • Security scan completed
  • Database migrations reviewed (if applicable)
  • Rollback plan documented
  • On-call support scheduled
  • Monitoring and alerting configured
  • Backup verification completed

Last updated: October 2025