firehose/context.md

Code Context

Files Retrieved

List with exact line ranges:

1. app/test/firehose_web/controllers/blog_test.exs (lines 1-128) - Comprehensive blog controller tests covering HTML and JSON API endpoints for engineering blog and release notes
2. app/lib/firehose_web/controllers/blog_controller.ex (lines 1-79) - Blog controller with pagination, 404 handling, and input validation
3. app/test/support/conn_case.ex (lines 1-38) - Test case template for connection tests
4. app/lib/firehose/blogs/engineering_blog.ex (lines 1-7) - Engineering blog module configuration
5. app/lib/firehose/blogs/release_notes.ex (lines 1-7) - Release notes blog module configuration

Key Code

Test Organization

# Current structure has 4 describe blocks:
describe "engineering blog (HTML)"     # 3 tests
describe "input validation"           # 5 tests (newly added in last commit)
describe "release notes blog (HTML)"  # 2 tests
describe "engineering blog (JSON API)" # 4 tests  
describe "release notes blog (JSON API)" # 3 tests

Input Validation Logic (blog_controller.ex, lines 68-76)

defp parse_page(nil), do: 1
defp parse_page(str) do
  case Integer.parse(str) do
    {page, ""} when page > 0 -> page
    _ -> 1
  end
end

Test Coverage Added in Last Commit

describe "input validation" do
  test "GET /blog/nonexistent returns 404", %{conn: conn} do
    conn = get(conn, "/blog/nonexistent")
    assert html_response(conn, 404)
  end

  test "GET /blog/engineering?page=abc falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=abc")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering?page=-1 falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=-1")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering?page=0 falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=0")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering/nonexistent-post returns 404", %{conn: conn} do
    assert_raise Blogex.NotFoundError, fn ->
      get(conn, "/blog/engineering/nonexistent-post")
    end
  end
end

Architecture

The application uses:

• Blogex library for blog functionality (engineering blog and release notes)
• Phoenix framework for web endpoints
• ConnCase test helper for connection testing
• Two blog types: Firehose.EngineeringBlog and Firehose.ReleaseNotes
• Pagination through blog.paginate(page) method
• 404 handling via Blogex.NotFoundError exception

Start Here

Which file to look at first and why:

Start with app/lib/firehose_web/controllers/blog_controller.ex

Why: This is the central controller that handles all blog requests. Understanding its structure (especially the parse_page/1 function and resolve_blog/2 plug) provides context for why the validation tests were added and how input handling works across both HTML and JSON endpoints.

Code Smells & Refactoring Suggestions

Smell 1: Repetitive Validation Tests

Issue: Four tests for page parameter validation (page=abc, -1, 0, and valid values) are highly repetitive with identical assertions.

Refactoring Suggestion: Use parameterized tests or test helpers:

# Test helper approach
test_page_fallback("page=abc", "abc")
test_page_fallback("page=-1", "-1") 
test_page_fallback("page=0", "0")

defp test_page_fallback(query_param, expected_page) do
  conn = get(conn, "/blog/engineering?#{query_param}")
  assert html_response(conn, 200) =~ "Engineering Blog"
end

Smell 2: Missing Negative Test Coverage

Issue: Tests don't verify what happens when invalid blog_id is provided (e.g., /blog/invalid-blog).

Refactoring Suggestion: Add test for unknown blog:

test "GET /blog/unknown returns 404", %{conn: conn} do
  conn = get(conn, "/blog/unknown")
  assert html_response(conn, 404)
end

Smell 3: Inconsistent Test Naming

Issue: Some tests use hyphenated slugs (v0-1-0), others use different formats. The naming doesn't clearly indicate what's being tested.

Refactoring Suggestion: Standardize naming:

# Instead of: "GET /blog/releases/v0-1-0 returns HTML post"
test "GET /blog/releases/:slug returns a release post", %{conn: conn} do

Smell 4: Redundant Layout Assertions

Issue: Multiple tests assert the same "firehose" string appears in response, testing layout presence.

Refactoring Suggestion: Create a shared test helper:

defp assert_has_app_layout(body), 
  do: assert body =~ "firehose"

# Then in tests: assert_has_app_layout(body)

Smell 5: Test Order Doesn't Follow Flow

Issue: Tests are grouped by endpoint but validation tests (which should be first for defensive programming) are in the middle.

Refactoring Suggestion: Reorder to follow natural request flow:

1. Input validation (404s, invalid params)
2. Success cases (index, show, tag)
3. Edge cases (pagination, RSS feeds)

Smell 6: No Test for Controller-Level Error Handling

Issue: The controller uses halt() in the resolve_blog plug, but there's no test verifying this behavior.

Refactoring Suggestion: Add test:

test "GET /blog/:blog_id with invalid blog halts request", %{conn: conn} do
  conn = get(conn, "/blog/invalid")
  assert conn.halted
end

Smell 7: Mixed Response Types Without Clear Separation

Issue: HTML tests use html_response/2, JSON tests use json_response/2, but there's no helper to verify content type before parsing.

Refactoring Suggestion: Create response helpers:

defp assert_html(conn, status), do: assert html_response(conn, status) != ""
defp assert_json(conn, status), do: assert json_response(conn, status) != %{}

Smell 8: No Test for Concurrent Requests or Edge Cases

Issue: Missing tests for:

• Empty page parameter (?page=)
• Very large page numbers
• Special characters in slug/tag parameters

Refactoring Suggestion: Add edge case tests to validation describe block.

Overall Recommendations

1. Extract test helpers to reduce duplication (especially for page validation)
2. Standardize test naming conventions across all blog types
3. Add positive test for valid page numbers (currently missing)
4. Consider property-based testing for input validation scenarios
5. Add performance tests if pagination is used heavily
6. Create integration tests that verify end-to-end flows