---
name: test-quality-inspector
description: "Test quality inspection framework for reviewing test coverage, identifying gaps, and ensuring comprehensive validation"
user-invocable: false
version: "1.0.0"
category: agent-reference
effort: medium
---
# Example Test Inspection Report
## Scenario: User Registration Feature
### Engineer's Test Suite
```python
# test_user_registration.py
def test_user_creation():
"""Test user creation"""
user = create_user("test@example.com", "password123")
assert user
def test_login():
"""Test login"""
user = create_user("test@example.com", "password123")
result = login("test@example.com", "password123")
assert result
def test_duplicate_email():
"""Test duplicate email"""
create_user("test@example.com", "password123")
user2 = create_user("test@example.com", "password456")
assert user2
```
---
## QA Inspection Report
### Test 1: test_user_creation()
**Claimed Intent:** Test user creation
**Actually Tests:** Object instantiation only
#### Issues Found
🔴 **CRITICAL: Worthless Assertion**
```python
assert user # Line 5
```
- Would pass with corrupt data
- Would pass with wrong email
- Would pass with invalid password hash
- Would pass if database save failed
**Mental Debug:** User object with garbage data would pass this test.
🟠 **HIGH: Missing Validation Tests**
- No test for invalid email format
- No test for weak password
- No test for duplicate email (separate test exists but broken)
- No test for SQL injection in email
🟠 **HIGH: No Persistence Verification**
- Test doesn't verify user is saved to database
- Test doesn't verify user can be retrieved
- Transaction might be rolled back
🟡 **MEDIUM: Vague Test Name**
- "test_user_creation" doesn't describe what should happen
- Better: "test_user_creation_with_valid_data_persists_to_database"
#### Suggested Improvements
```python
def test_user_creation_with_valid_data_persists_to_database():
"""Creating a user with valid email and password should:
1. Create user object with correct attributes
2. Save to database
3. Hash password (not store plaintext)
4. Set default role and active status
"""
email = "test@example.com"
password = "SecurePass123!"
user = create_user(email, password)
# Verify user object
assert user.id is not None, "User should have an ID after creation"
assert user.email == email, "Email should match input"
assert user.is_active is True, "New users should be active by default"
assert user.role == "user", "New users should have 'user' role"
assert user.created_at is not None, "Created timestamp should be set"
# Verify password is hashed, not plaintext
assert user.password_hash != password, "Password should be hashed"
assert len(user.password_hash) > 50, "Password hash should be substantial"
# Verify persistence
retrieved_user = User.get_by_email(email)
assert retrieved_user is not None, "User should be retrievable from database"
assert retrieved_user.id == user.id, "Retrieved user should match created user"
def test_user_creation_with_invalid_email_format_raises_validation_error():
"""Creating a user with malformed email should raise ValidationError"""
invalid_emails = [
"not-an-email",
"@example.com",
"test@",
"test space@example.com",
"",
]
for invalid_email in invalid_emails:
with pytest.raises(ValidationError) as exc:
create_user(invalid_email, "password123")
assert "email" in str(exc.value).lower()
assert "invalid" in str(exc.value).lower()
def test_user_creation_with_weak_password_raises_validation_error():
"""Creating a user with weak password should raise ValidationError"""
weak_passwords = [
"123", # Too short
"password", # No numbers
"12345678", # No letters
"", # Empty
]
for weak_password in weak_passwords:
with pytest.raises(ValidationError) as exc:
create_user("test@example.com", weak_password)
assert "password" in str(exc.value).lower()
```
**Risk Level:** 🔴 CRITICAL
**Action:** ❌ BLOCK - Core functionality not tested
**Estimated Fix Time:** 30 minutes
---
### Test 2: test_login()
**Claimed Intent:** Test login
**Actually Tests:** Function call completes
#### Issues Found
🔴 **CRITICAL: Worthless Assertion**
```python
assert result # Line 11
```
- Passes with any truthy value
- Doesn't verify session/token
- Doesn't verify user authentication state
🔴 **CRITICAL: Missing Negative Tests**
- No test for wrong password
- No test for non-existent user
- No test for locked account
- No test for expired credentials
🟠 **HIGH: No Session Verification**
- Doesn't verify authentication token
- Doesn't verify session expiry
- Doesn't verify user context in session
🟡 **MEDIUM: Test Depends on Previous Test**
- Creates user in this test
- Should use fixture or setup
- Tests should be independent
#### Suggested Improvements
```python
@pytest.fixture
def registered_user():
"""Fixture providing a registered user for login tests"""
user = create_user("test@example.com", "SecurePass123!")
yield user
# Cleanup if needed
User.delete(user.id)
def test_login_with_valid_credentials_returns_authenticated_session(registered_user):
"""Logging in with correct email and password should:
1. Return authentication token/session
2. Set authenticated state
3. Include user context
4. Set appropriate expiry
"""
session = login(registered_user.email, "SecurePass123!")
assert session is not None, "Login should return session"
assert session.is_authenticated is True, "Session should be authenticated"
assert session.user_id == registered_user.id, "Session should contain user ID"
assert session.token is not None, "Session should have authentication token"
assert session.expires_at > datetime.now(), "Session should have future expiry"
assert (session.expires_at - datetime.now()).seconds >= 3600, "Session should last at least 1 hour"
def test_login_with_wrong_password_raises_authentication_error(registered_user):
"""Logging in with incorrect password should raise AuthenticationError"""
with pytest.raises(AuthenticationError) as exc:
login(registered_user.email, "WrongPassword")
assert "Invalid credentials" in str(exc.value)
assert "password" in str(exc.value).lower()
def test_login_with_nonexistent_email_raises_authentication_error():
"""Logging in with non-existent email should raise AuthenticationError"""
with pytest.raises(AuthenticationError) as exc:
login("doesnotexist@example.com", "password")
assert "Invalid credentials" in str(exc.value)
# Note: Don't reveal if email exists (security)
def test_login_with_locked_account_raises_account_locked_error(registered_user):
"""Logging in to locked account should raise AccountLockedError"""
lock_account(registered_user.id)
with pytest.raises(AccountLockedError) as exc:
login(registered_user.email, "SecurePass123!")
assert registered_user.email in str(exc.value)
def test_login_with_empty_password_raises_validation_error(registered_user):
"""Logging in with empty password should raise ValidationError"""
with pytest.raises(ValidationError) as exc:
login(registered_user.email, "")
assert "password" in str(exc.value).lower()
assert "required" in str(exc.value).lower()
```
**Risk Level:** 🔴 CRITICAL
**Action:** ❌ BLOCK - Authentication not actually tested
**Estimated Fix Time:** 45 minutes
---
### Test 3: test_duplicate_email()
**Claimed Intent:** Test duplicate email handling
**Actually Tests:** Second user creation succeeds (WRONG!)
#### Issues Found
🔴 **CRITICAL: Test is Backwards**
```python
user2 = create_user("test@example.com", "password456")
assert user2 # Line 17
```
- This test expects duplicate creation to SUCCEED
- It should expect it to FAIL with an error
- Test passes when it should fail
- **This is testing the opposite of what's needed**
🔴 **CRITICAL: False Confidence**
- Production bug: duplicate emails are allowed
- Test claims to verify duplicate prevention
- Test actually verifies duplicates work
- QA might approve thinking it's covered
🟡 **MEDIUM: Same Email Issue as Other Tests**
- If this fixed to expect error, needs all improvements from Test 1
#### Suggested Fix
```python
def test_create_user_with_duplicate_email_raises_integrity_error():
"""Creating a user with an email that already exists should:
1. Raise IntegrityError or ValidationError
2. Not create duplicate user in database
3. Preserve existing user data
"""
email = "test@example.com"
# Create first user
user1 = create_user(email, "FirstPassword123!")
initial_count = User.count()
# Attempt to create duplicate
with pytest.raises((IntegrityError, ValidationError)) as exc:
create_user(email, "SecondPassword456!")
assert "email" in str(exc.value).lower()
assert "duplicate" in str(exc.value).lower() or "exists" in str(exc.value).lower()
# Verify no new user created
assert User.count() == initial_count, "User count should not increase"
# Verify original user unchanged
original_user = User.get_by_email(email)
assert original_user.id == user1.id, "Original user should be intact"
assert original_user.verify_password("FirstPassword123!"), "Original password should work"
assert not original_user.verify_password("SecondPassword456!"), "New password should not work"
```
**Risk Level:** 🔴 CRITICAL
**Action:** ❌ BLOCK - Test verifies opposite of requirement
**Estimated Fix Time:** 20 minutes
---
## Summary Report
### Overall Assessment
**Test Suite Quality:** 🔴 FAILING
**Critical Issues:** 3
- Test 1: Doesn't actually test user creation
- Test 2: Doesn't actually test authentication
- Test 3: Tests opposite of requirement
**Total Tests:** 3
**Effective Tests:** 0
**Coverage:** High (claims)
**Protection:** None (reality)
### Risk Assessment
**Production Risk:** 🔴 EXTREME
Current test suite provides **zero protection** against:
- Data corruption in user creation
- Authentication bypass
- Duplicate email registration
- Password security issues
- Database integrity issues
**Confidence Level:** 0% - Tests passing means nothing
### Required Actions
#### Immediate (Block Merge)
1. Rewrite all three tests with proper assertions
2. Add negative test cases (12+ tests needed)
3. Verify tests catch intentional bugs
4. Add fixture for test user management
#### Follow-up (Required for completion)
1. Add edge case tests (15+ additional tests)
2. Add integration tests for full registration flow
3. Add security tests (SQL injection, XSS, etc.)
4. Add performance tests for registration endpoint
### Estimated Timeline
- Fix critical issues: 2-3 hours
- Complete test suite: 1 day
- Review and iteration: 0.5 days
**Total:** 1.5-2 days for proper test coverage
### Recommendation
❌ **BLOCK MERGE**
Do not approve this PR. Tests provide false confidence and mask critical bugs.
**Evidence:**
- All tests would pass with completely broken functionality
- Duplicate email test verifies the opposite of requirements
- No actual behavior is verified
**Next Steps:**
1. Engineer rewrites tests following examples above
2. QA re-inspects rewritten tests
3. QA verifies tests catch intentional bugs
4. Only then approve merge
---
## Lessons for Engineer
### What Went Wrong
1. **Wrote tests after code** - Led to tests that just confirm code runs
2. **Weak assertions** - "assert x" proves nothing
3. **No mental debugging** - Didn't verify tests catch bugs
4. **No negative testing** - Only tested happy path
5. **Misunderstood duplicate test** - Test verified opposite
### How to Improve
1. **Write tests first** (TDD) - Prevents these issues
2. **Specific assertions** - Verify exact values
3. **Mental debugging** - Break code, ensure test fails
4. **Test failures explicitly** - Every success needs failure test
5. **Read test name carefully** - Test what you claim to test
### TDD Would Have Prevented This
If tests were written first:
```python
# Write this FIRST (it will fail):
def test_user_creation_with_valid_data_persists_to_database():
user = create_user("test@example.com", "password")
assert user.email == "test@example.com" # Will fail until create_user works
...
# Then implement create_user to make it pass
```
See the Test-Driven Development skill for complete TDD workflow (available in the skill library for comprehensive TDD guidance).
---
## Sign-off
**QA Inspector:** [Your name]
**Date:** [Date]
**Status:** ❌ REJECTED
**Reason:** Tests provide zero protection, must be rewritten
**Re-inspection Required:** Yes
---
*This is what thorough test inspection looks like. Better to catch these issues now than in production.*