Skip to content

πŸ›‘οΈ Critical: Implement database protection system for testing#1

Merged
buger merged 3 commits into
mainfrom
fix/database-protection-system
Sep 14, 2025
Merged

πŸ›‘οΈ Critical: Implement database protection system for testing#1
buger merged 3 commits into
mainfrom
fix/database-protection-system

Conversation

@buger

@buger buger commented Sep 14, 2025

Copy link
Copy Markdown
Owner

Summary

  • Implements comprehensive database protection to prevent test runs from wiping the development database
  • Addresses critical issue where running tests accidentally destroyed all production data
  • Adds multiple layers of protection and clear documentation

Problem Statement

The Laravel test suite uses the RefreshDatabase trait which drops and recreates all database tables. When tests were run without proper environment configuration, they connected to the development database (learning_app) instead of the test database (learning_app_test), resulting in complete data loss.

This has happened multiple times and caused significant disruption to development work.

Solution Overview

Implemented a multi-layer protection system with the following safeguards:

1. βœ… Safe Test Runner Script (scripts/safe-test.sh)

  • Forces DB_DATABASE=learning_app_test explicitly
  • Validates database name isn't 'learning_app'
  • Creates test database if it doesn't exist
  • Shows configuration before running tests
  • Exits immediately if wrong database detected

2. βœ… TestCase.php Runtime Protection

  • Added database validation in setUp() method
  • Throws exception if tests attempt to run on non-test database
  • Provides clear error message with recovery instructions

3. βœ… phpunit.xml Configuration Hardening

  • Added force="true" to DB_DATABASE environment variable
  • Ensures test database is always used regardless of .env
  • Added warning comments about RefreshDatabase dangers

4. βœ… Makefile Integration

  • Updated test-unit and test-unit-coverage commands
  • Both now use safe-test.sh instead of direct artisan test
  • Added clear warnings about database safety

5. βœ… Comprehensive Documentation

  • Updated CLAUDE.md with safe testing instructions
  • Listed dangerous commands to avoid
  • Created TESTING_SAFETY.md with:
    • Quick reference card
    • Recovery procedures
    • Best practices
    • CI/CD examples

Testing Done

  • βœ… Verified safe-test.sh prevents connection to development database
  • βœ… Confirmed TestCase.php throws exception on wrong database
  • βœ… Tested Makefile commands work with new safety wrapper
  • βœ… Validated all documentation is accurate and helpful

Impact

  • Immediate: Prevents future database wipes during testing
  • Developer Experience: Clear error messages guide correct usage
  • Documentation: Recovery procedures if issues occur
  • CI/CD Ready: Includes examples for automated testing

Files Changed

  • scripts/safe-test.sh - New safety wrapper script
  • tests/TestCase.php - Added runtime database validation
  • phpunit.xml - Hardened configuration with force attributes
  • Makefile - Updated test commands to use safe wrapper
  • CLAUDE.md - Added safe testing instructions
  • TESTING_SAFETY.md - New comprehensive safety documentation

Checklist

  • Code follows project conventions
  • Tests pass locally using safe-test.sh
  • Documentation updated
  • No breaking changes
  • Security implications considered

Related Issues

  • Fixes critical database wipe issue reported multiple times
  • Implements requested safety measures for test execution

πŸ€– Generated with Claude Code

buger and others added 3 commits September 14, 2025 10:25
Critical fix to prevent development database from being wiped during testing.

## Problem
- Tests with RefreshDatabase trait were wiping the development database
- Running `php artisan test` without proper environment setup destroyed all data
- This happened multiple times causing significant data loss

## Solution - Multi-layer Protection System

### 1. Safe Test Runner Script (scripts/safe-test.sh)
- Forces DB_DATABASE=learning_app_test explicitly
- Validates database name isn't 'learning_app'
- Creates test database if it doesn't exist
- Shows configuration before running tests
- Exit immediately if wrong database detected

### 2. TestCase.php Runtime Protection
- Added database validation in setUp() method
- Throws exception if tests attempt to run on non-test database
- Provides clear error message with recovery instructions

### 3. phpunit.xml Configuration Hardening
- Added force="true" to DB_DATABASE environment variable
- Ensures test database is always used regardless of .env
- Added warning comments about RefreshDatabase dangers

### 4. Makefile Integration
- Updated test-unit and test-unit-coverage commands
- Both now use safe-test.sh instead of direct artisan test
- Added clear warnings about database safety

### 5. Documentation Updates
- Updated CLAUDE.md with safe testing instructions
- Listed dangerous commands to avoid
- Provided safe alternatives and examples
- Created TESTING_SAFETY.md with comprehensive guide
- Includes recovery procedures if database gets wiped

## Testing
- Verified safe-test.sh prevents connection to development database
- Confirmed TestCase.php throws exception on wrong database
- Tested Makefile commands work with new safety wrapper

## Impact
- Developers can now run tests without fear of data loss
- Clear documentation prevents future incidents
- Multiple failsafes ensure database protection
- Recovery procedures documented if issues occur

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
The CI pipeline uses 'homeschoolai_test' and 'homeschoolai_e2e' as database names,
but our protection was only allowing 'learning_app_test'. This commit updates
the TestCase.php to allow all valid test database names while still protecting
the development database from being wiped.
- Set max failures to 3 in CI (stop after 3 test failures)
- Disable retries in CI for faster feedback
- Add 10-minute timeout for e2e job
- Reduce individual test timeouts in CI (30s test, 5s expect, 5s action)
- Use BASE_URL environment variable properly
- Add simpler 'line' reporter for CI output
@buger buger merged commit d9cc352 into main Sep 14, 2025
4 checks passed
@buger buger deleted the fix/database-protection-system branch September 14, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant