Add comprehensive tests for home, search, and style_guide apps

[nickmoreton]

Summary

Add response tests for home, search, and style_guide apps to ensure all frontend and admin endpoints return 200 OK.

Changes

• home app (6 tests): frontend, admin edit/delete/copy/move/history pages
• search app (4 tests): search page with various query scenarios
• style_guide app (1 test): frontend page

Testing

All 11 tests pass:

make test

🤖 Generated with Claude Code

[claude]

PR Review: Add comprehensive tests for home, search, and style_guide apps

Overall Assessment

This is a solid PR that adds valuable test coverage and CI/CD infrastructure. The test improvements and GitHub Actions workflow are well-structured, though there are a few areas that could be enhanced.

✅ Strengths

Test Organization

• Good docstrings: All test classes and methods have clear docstrings explaining their purpose
• Descriptive naming: Test method names follow the pattern test_<feature>_<action>_returns_200, making intent clear
• Logical grouping: Tests are properly organized by app functionality

CI/CD Workflow

• Comprehensive pipeline: Includes both pre-commit checks and test execution
• Proper matrix setup: Uses appropriate Python and Node.js versions
• Build steps included: Frontend asset building and static file collection before tests

📋 Code Quality Issues

1. Test Setup Inefficiency (app/home/tests.py:10-15)

User.objects.create_user(
    username="testuser", password="12345", is_staff=True, is_superuser=True
)

Issue: .save() was removed but the line was split unnecessarily. The previous call to .save() was redundant since create_user() already saves.
Status: Fixed correctly ✅

2. Missing Test Coverage

The tests only verify 200 OK responses. Consider adding:

• Assertion tests: Check for specific content/elements in responses
• Permission tests: Verify non-staff users cannot access admin pages
• Edge cases: Test with invalid page IDs, test POST operations
• Search functionality: Test actual search results, pagination

Example enhancement for app/search/tests.py:

def test_search_with_results(self):
    """Test that search returns actual results when content exists."""
    # Create a page that can be searched
    home = HomePage.objects.first()
    # Add searchable content and verify results

3. Weak Password in Tests (app/home/tests.py:11)

password="12345"

Issue: While this is test code, using weak passwords can promote bad practices
Recommendation: Use a more realistic test password like "TestPass123!" to model good practices

4. Duplicate Client Login

All admin tests repeat:

self.client.login(username="testuser", password="12345")

Recommendation: Use a helper method or create a separate test class for admin tests with login in setUp()

🔒 Security Considerations

No Major Concerns ✅

• Test code properly isolated
• No secrets or credentials exposed
• Admin access properly gated behind authentication
• CI workflow uses standard, trusted actions

Minor note: The CI workflow doesn't specify permission scopes, but for this use case (read-only operations) it's acceptable.

⚡ Performance Considerations

CI Workflow Caching

Observation: The workflow doesn't cache dependencies
Impact: Slower CI runs, increased cost
Recommendation: Add caching for faster builds:

- name: Cache Python dependencies
  uses: actions/cache@v4
  with:
    path: ~/.cache/uv
    key: ${{ runner.os }}-uv-${{ hashFiles('**/uv.lock') }}

- name: Cache Node modules
  uses: actions/cache@v4
  with:
    path: ~/.npm
    key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}

🧪 Test Coverage Assessment

Current Coverage: Basic smoke tests (200 OK checks)
Missing Coverage:

• Content validation (assert specific text/elements)
• Form submissions (POST requests)
• Error conditions (404s, permission denied)
• Search result accuracy
• Admin action operations (actual delete, copy, move)

Recommendation: Follow up with a PR to add functional tests that verify behavior beyond HTTP status codes.

📝 Suggestions

1. Environment Variable Duplication

The test job repeats DJANGO_SETTINGS_MODULE: app.settings.dev in two steps. Consider using job-level env:

test:
  name: Run tests
  runs-on: ubuntu-latest
  env:
    DJANGO_SETTINGS_MODULE: app.settings.dev
  steps:
    # ... steps here

2. Test Fixtures

Consider creating fixtures for commonly used test data:

# app/home/fixtures/test_data.json
# Then in tests:
fixtures = ['test_data']

3. Python Version Alignment

CI uses Python 3.10, but pyproject.toml specifies >=3.10. Consider testing against multiple Python versions (3.10, 3.11, 3.12) or pinning to one specific version.

✨ Best Practices Observed

• ✅ Proper use of Django TestCase
• ✅ setUp method for test initialization
• ✅ Consistent test naming conventions
• ✅ Separation of concerns (frontend vs admin tests)
• ✅ CI workflow triggers on main branch and PRs

🎯 Recommendation

Approve with suggestions for follow-up improvements

This PR provides valuable infrastructure and basic test coverage. The code quality is good, and there are no blocking issues. The suggested enhancements (caching, expanded test coverage, refactoring) can be addressed in future PRs.

Priority for follow-up:

1. Add CI caching for performance
2. Expand test coverage beyond smoke tests
3. Refactor duplicate login code
4. Add permission tests for admin pages

---

Review generated by Claude Code