
test-quality-inspector
by bobmatnyc
Remote terminal control system with AI assistance - Control tmux and iTerm2 sessions via REST API with LLM-powered intelligent assistance
SKILL.md
name: test-quality-inspector description: Test quality inspection framework for reviewing test coverage, identifying gaps, and ensuring comprehensive validation. Use when reviewing test suites, performing QA inspections, or establishing testing quality standards.
Example Test Inspection Report
Scenario: User Registration Feature
Engineer's Test Suite
# 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
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
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
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
@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
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
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)
- Rewrite all three tests with proper assertions
- Add negative test cases (12+ tests needed)
- Verify tests catch intentional bugs
- Add fixture for test user management
Follow-up (Required for completion)
- Add edge case tests (15+ additional tests)
- Add integration tests for full registration flow
- Add security tests (SQL injection, XSS, etc.)
- 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:
- Engineer rewrites tests following examples above
- QA re-inspects rewritten tests
- QA verifies tests catch intentional bugs
- Only then approve merge
Lessons for Engineer
What Went Wrong
- Wrote tests after code - Led to tests that just confirm code runs
- Weak assertions - "assert x" proves nothing
- No mental debugging - Didn't verify tests catch bugs
- No negative testing - Only tested happy path
- Misunderstood duplicate test - Test verified opposite
How to Improve
- Write tests first (TDD) - Prevents these issues
- Specific assertions - Verify exact values
- Mental debugging - Break code, ensure test fails
- Test failures explicitly - Every success needs failure test
- Read test name carefully - Test what you claim to test
TDD Would Have Prevented This
If tests were written first:
# 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.
Score
Total Score
Based on repository quality metrics
SKILL.mdファイルが含まれている
ライセンスが設定されている
100文字以上の説明がある
GitHub Stars 100以上
3ヶ月以内に更新
10回以上フォークされている
オープンIssueが50未満
プログラミング言語が設定されている
1つ以上のタグが設定されている
Reviews
Reviews coming soon
