fixed 11 out or 34 conn shadowing cases

11 are multi-line, script does not do that
This commit is contained in:
Willem van den Ende 2026-05-05 21:26:00 +01:00
parent 73f6ca5049
commit a89d09e432
5 changed files with 180 additions and 185 deletions

View File

@ -3,23 +3,20 @@ defmodule FirehoseWeb.BlogControllerTest do
describe "GET /blog/:blog_id (index) - date filtering" do describe "GET /blog/:blog_id (index) - date filtering" do
test "does not show future-dated posts", %{conn: conn} do test "does not show future-dated posts", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering") html = conn |> get(~p"/blog/engineering") |> html_response(200)
html = html_response(conn, 200)
refute html =~ "Future Test Post" refute html =~ "Future Test Post"
end end
end end
describe "GET /blog/:blog_id/:slug (show) - date filtering" do describe "GET /blog/:blog_id/:slug (show) - date filtering" do
test "still shows a future-dated post by slug", %{conn: conn} do test "still shows a future-dated post by slug", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering/future-test-post") assert conn |> get(~p"/blog/engineering/future-test-post") |> html_response(200) =~ "Future Test Post"
assert html_response(conn, 200) =~ "Future Test Post"
end end
end end
describe "GET /blog/:blog_id/tag/:tag - date filtering" do describe "GET /blog/:blog_id/tag/:tag - date filtering" do
test "excludes future-dated posts from tag page", %{conn: conn} do test "excludes future-dated posts from tag page", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering/tag/test") html = conn |> get(~p"/blog/engineering/tag/test") |> html_response(200)
html = html_response(conn, 200)
refute html =~ "Future Test Post" refute html =~ "Future Test Post"
end end
end end
@ -28,24 +25,22 @@ defmodule FirehoseWeb.BlogControllerTest do
setup :register_and_log_in_user setup :register_and_log_in_user
test "authenticated user sees draft banner on draft post", %{conn: conn} do test "authenticated user sees draft banner on draft post", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering/hello-world") response = conn |> get(~p"/blog/engineering/hello-world")
assert html_response(conn, 200) =~ "Draft" assert html_response(response, 200) =~ "Draft"
assert conn.resp_body =~ "not published" assert response.resp_body =~ "not published"
end end
test "authenticated user sees scheduled banner on future post", %{conn: conn} do test "authenticated user sees scheduled banner on future post", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering/future-test-post") response = conn |> get(~p"/blog/engineering/future-test-post") |> html_response(200)
response = html_response(conn, 200)
assert response =~ "scheduled for" assert response =~ "scheduled for"
assert response =~ "January 01, 2099" assert response =~ "January 01, 2099"
end end
test "authenticated user sees no banner on live post", %{conn: conn} do test "authenticated user sees no banner on live post", %{conn: conn} do
conn = get(conn, ~p"/blog/engineering/why-firehose") response = conn |> get(~p"/blog/engineering/why-firehose") |> html_response(200)
response = html_response(conn, 200)
refute response =~ "Draft" refute response =~ "Draft"
refute response =~ "scheduled for" refute response =~ "scheduled for"
end end

View File

@ -5,8 +5,7 @@ defmodule FirehoseWeb.UserRegistrationControllerTest do
describe "GET /users/register" do describe "GET /users/register" do
test "renders registration page", %{conn: conn} do test "renders registration page", %{conn: conn} do
conn = get(conn, ~p"/users/register") response = conn |> get(~p"/users/register") |> html_response(200)
response = html_response(conn, 200)
assert response =~ "Register" assert response =~ "Register"
assert response =~ ~p"/users/log-in" assert response =~ ~p"/users/log-in"
assert response =~ ~p"/users/register" assert response =~ ~p"/users/register"
@ -58,23 +57,21 @@ defmodule FirehoseWeb.UserRegistrationControllerTest do
Application.put_env(:firehose, :allowed_registration_email, "allowed@example.com") Application.put_env(:firehose, :allowed_registration_email, "allowed@example.com")
on_exit(fn -> Application.delete_env(:firehose, :allowed_registration_email) end) on_exit(fn -> Application.delete_env(:firehose, :allowed_registration_email) end)
conn = post(conn, ~p"/users/register", %{"user" => %{"email" => "allowed@example.com"}}) response = conn |> post(~p"/users/register", %{"user" => %{"email" => "allowed@example.com"}})
assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "email was sent" assert Phoenix.Flash.get(response.assigns.flash, :info) =~ "email was sent"
end end
test "fails with invite-only message when email doesn't match", %{conn: conn} do test "fails with invite-only message when email doesn't match", %{conn: conn} do
Application.put_env(:firehose, :allowed_registration_email, "allowed@example.com") Application.put_env(:firehose, :allowed_registration_email, "allowed@example.com")
on_exit(fn -> Application.delete_env(:firehose, :allowed_registration_email) end) on_exit(fn -> Application.delete_env(:firehose, :allowed_registration_email) end)
conn = post(conn, ~p"/users/register", %{"user" => %{"email" => "other@example.com"}}) assert conn |> post(~p"/users/register", %{"user" => %{"email" => "other@example.com"}}) |> html_response(200) =~ "registration is invite only"
assert html_response(conn, 200) =~ "registration is invite only"
end end
test "fails with invite-only message when env var is unset", %{conn: conn} do test "fails with invite-only message when env var is unset", %{conn: conn} do
Application.delete_env(:firehose, :allowed_registration_email) Application.delete_env(:firehose, :allowed_registration_email)
conn = post(conn, ~p"/users/register", %{"user" => %{"email" => "anyone@example.com"}}) assert conn |> post(~p"/users/register", %{"user" => %{"email" => "anyone@example.com"}}) |> html_response(200) =~ "registration is invite only"
assert html_response(conn, 200) =~ "registration is invite only"
end end
end end
end end

View File

@ -10,8 +10,7 @@ defmodule FirehoseWeb.UserSessionControllerTest do
describe "GET /users/log-in" do describe "GET /users/log-in" do
test "renders login page", %{conn: conn} do test "renders login page", %{conn: conn} do
conn = get(conn, ~p"/users/log-in") response = conn |> get(~p"/users/log-in") |> html_response(200)
response = html_response(conn, 200)
assert response =~ "Log in" assert response =~ "Log in"
assert response =~ ~p"/users/register" assert response =~ ~p"/users/register"
assert response =~ "Log in with email" assert response =~ "Log in with email"
@ -33,8 +32,7 @@ defmodule FirehoseWeb.UserSessionControllerTest do
end end
test "renders login page (email + password)", %{conn: conn} do test "renders login page (email + password)", %{conn: conn} do
conn = get(conn, ~p"/users/log-in?mode=password") response = conn |> get(~p"/users/log-in?mode=password") |> html_response(200)
response = html_response(conn, 200)
assert response =~ "Log in" assert response =~ "Log in"
assert response =~ ~p"/users/register" assert response =~ ~p"/users/register"
assert response =~ "Log in with email" assert response =~ "Log in with email"
@ -48,8 +46,7 @@ defmodule FirehoseWeb.UserSessionControllerTest do
Accounts.deliver_login_instructions(user, url) Accounts.deliver_login_instructions(user, url)
end) end)
conn = get(conn, ~p"/users/log-in/#{token}") assert conn |> get(~p"/users/log-in/#{token}") |> html_response(200) =~ "Confirm and stay logged in"
assert html_response(conn, 200) =~ "Confirm and stay logged in"
end end
test "renders login page for confirmed user", %{conn: conn, user: user} do test "renders login page for confirmed user", %{conn: conn, user: user} do
@ -58,17 +55,16 @@ defmodule FirehoseWeb.UserSessionControllerTest do
Accounts.deliver_login_instructions(user, url) Accounts.deliver_login_instructions(user, url)
end) end)
conn = get(conn, ~p"/users/log-in/#{token}") html = conn |> get(~p"/users/log-in/#{token}") |> html_response(200)
html = html_response(conn, 200)
refute html =~ "Confirm my account" refute html =~ "Confirm my account"
assert html =~ "Keep me logged in on this device" assert html =~ "Keep me logged in on this device"
end end
test "raises error for invalid token", %{conn: conn} do test "raises error for invalid token", %{conn: conn} do
conn = get(conn, ~p"/users/log-in/invalid-token") response = conn |> get(~p"/users/log-in/invalid-token")
assert redirected_to(conn) == ~p"/users/log-in" assert redirected_to(response) == ~p"/users/log-in"
assert Phoenix.Flash.get(conn.assigns.flash, :error) == assert Phoenix.Flash.get(response.assigns.flash, :error) ==
"Magic link is invalid or it has expired." "Magic link is invalid or it has expired."
end end
end end
@ -190,10 +186,10 @@ defmodule FirehoseWeb.UserSessionControllerTest do
end end
test "succeeds even if the user is not logged in", %{conn: conn} do test "succeeds even if the user is not logged in", %{conn: conn} do
conn = delete(conn, ~p"/users/log-out") response = conn |> delete(~p"/users/log-out")
assert redirected_to(conn) == ~p"/" assert redirected_to(response) == ~p"/"
refute get_session(conn, :user_token) refute get_session(response, :user_token)
assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Logged out successfully" assert Phoenix.Flash.get(response.assigns.flash, :info) =~ "Logged out successfully"
end end
end end
end end

View File

@ -8,23 +8,22 @@ defmodule FirehoseWeb.UserSettingsControllerTest do
describe "GET /users/settings" do describe "GET /users/settings" do
test "renders settings page", %{conn: conn} do test "renders settings page", %{conn: conn} do
conn = get(conn, ~p"/users/settings") response = conn |> get(~p"/users/settings") |> html_response(200)
response = html_response(conn, 200)
assert response =~ "Settings" assert response =~ "Settings"
end end
test "redirects if user is not logged in" do test "redirects if user is not logged in" do
conn = build_conn() conn = build_conn()
conn = get(conn, ~p"/users/settings") response = conn |> get(~p"/users/settings")
assert redirected_to(conn) == ~p"/users/log-in" assert redirected_to(response) == ~p"/users/log-in"
end end
@tag token_authenticated_at: DateTime.add(DateTime.utc_now(:second), -11, :minute) @tag token_authenticated_at: DateTime.add(DateTime.utc_now(:second), -11, :minute)
test "redirects if user is not in sudo mode", %{conn: conn} do test "redirects if user is not in sudo mode", %{conn: conn} do
conn = get(conn, ~p"/users/settings") response = conn |> get(~p"/users/settings")
assert redirected_to(conn) == ~p"/users/log-in" assert redirected_to(response) == ~p"/users/log-in"
assert Phoenix.Flash.get(conn.assigns.flash, :error) == assert Phoenix.Flash.get(response.assigns.flash, :error) ==
"You must re-authenticate to access this page." "You must re-authenticate to access this page."
end end
end end
@ -112,28 +111,28 @@ defmodule FirehoseWeb.UserSettingsControllerTest do
end end
test "updates the user email once", %{conn: conn, user: user, token: token, email: email} do test "updates the user email once", %{conn: conn, user: user, token: token, email: email} do
conn = get(conn, ~p"/users/settings/confirm-email/#{token}") response = conn |> get(~p"/users/settings/confirm-email/#{token}")
assert redirected_to(conn) == ~p"/users/settings" assert redirected_to(response) == ~p"/users/settings"
assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ assert Phoenix.Flash.get(response.assigns.flash, :info) =~
"Email changed successfully" "Email changed successfully"
refute Accounts.get_user_by_email(user.email) refute Accounts.get_user_by_email(user.email)
assert Accounts.get_user_by_email(email) assert Accounts.get_user_by_email(email)
conn = get(conn, ~p"/users/settings/confirm-email/#{token}") response = conn |> get(~p"/users/settings/confirm-email/#{token}")
assert redirected_to(conn) == ~p"/users/settings" assert redirected_to(response) == ~p"/users/settings"
assert Phoenix.Flash.get(conn.assigns.flash, :error) =~ assert Phoenix.Flash.get(response.assigns.flash, :error) =~
"Email change link is invalid or it has expired" "Email change link is invalid or it has expired"
end end
test "does not update email with invalid token", %{conn: conn, user: user} do test "does not update email with invalid token", %{conn: conn, user: user} do
conn = get(conn, ~p"/users/settings/confirm-email/oops") response = conn |> get(~p"/users/settings/confirm-email/oops")
assert redirected_to(conn) == ~p"/users/settings" assert redirected_to(response) == ~p"/users/settings"
assert Phoenix.Flash.get(conn.assigns.flash, :error) =~ assert Phoenix.Flash.get(response.assigns.flash, :error) =~
"Email change link is invalid or it has expired" "Email change link is invalid or it has expired"
assert Accounts.get_user_by_email(user.email) assert Accounts.get_user_by_email(user.email)
@ -141,8 +140,8 @@ defmodule FirehoseWeb.UserSettingsControllerTest do
test "redirects if user is not logged in", %{token: token} do test "redirects if user is not logged in", %{token: token} do
conn = build_conn() conn = build_conn()
conn = get(conn, ~p"/users/settings/confirm-email/#{token}") response = conn |> get(~p"/users/settings/confirm-email/#{token}")
assert redirected_to(conn) == ~p"/users/log-in" assert redirected_to(response) == ~p"/users/log-in"
end end
end end
end end

View File

@ -37,146 +37,154 @@ for file in "${FILES[@]}"; do
trap "rm -f '$tmpfile'" EXIT trap "rm -f '$tmpfile'" EXIT
awk ' awk '
# Detect trigger line: conn = VERB(conn, ARGS) function is_hard_scope_boundary(line) {
# where VERB is get/post/put/patch/delete/head/options return (line ~ /^[[:space:]]*end$/ || line ~ /^[[:space:]]*conn =/ || line ~ /^[[:space:]]*(test|describe) /)
/^[[:space:]]*conn = (get|post|put|patch|delete|head|options)\(conn, / {
trigger_line = $0
# Extract leading whitespace
match($0, /^[[:space:]]*/)
indent = substr($0, RSTART, RLENGTH)
# Extract verb and args from: conn = verb(conn, args)
rest = $0
sub(/^[[:space:]]*conn = /, "", rest)
# rest is now: verb(conn, args)
paren_pos = index(rest, "(")
verb = substr(rest, 1, paren_pos - 1)
# args portion: everything after "conn, " up to the trailing ")"
inner = substr(rest, paren_pos + 1)
sub(/\)$/, "", inner)
# inner is: conn, args
sub(/^conn, /, "", inner)
args = inner
# Read the next non-blank line
triggered = 1
next
} }
triggered == 1 { function conn_used_ahead(start_idx, i, line) {
# Skip blank lines, accumulating them for (i = start_idx; i <= total_lines; i++) {
if ($0 ~ /^[[:space:]]*$/) { line = lines[i]
blank_lines = blank_lines $0 "\n" if (is_hard_scope_boundary(line)) return 0
next if (line ~ /conn/) return 1
} }
return 0
}
next_line = $0 # Read all lines into array
{ lines[NR] = $0 }
END {
total_lines = NR
triggered = 0 triggered = 0
renaming = 0
# Now look ahead: count how many subsequent lines (until scope boundary)
# reference "conn" — to decide Case 4 vs Cases 1-3
# We already have next_line. Check if next_line references conn.
# Then peek further lines.
# For simplicity: check if next_line matches Case 1, 2, or 3 patterns.
# If it does, check the line AFTER that for more conn references (Case 4 override).
# Case 1: var = helper(conn, status)
# helpers: html_response, json_response, text_response, response, redirected_to
case1 = 0
if (match(next_line, /^[[:space:]]*([a-z_]+) = (html_response|json_response|text_response|response|redirected_to)\(conn, [^)]+\)$/, m1)) {
case1 = 1
c1_var = m1[1]
c1_helper = m1[2]
# Extract status from helper(conn, status)
match(next_line, /\(conn, ([^)]+)\)/, m1s)
c1_status = m1s[1]
}
# Case 2: assert helper(conn, status) with optional =~ "..."
case2 = 0
if (match(next_line, /^[[:space:]]*assert (html_response|json_response|text_response|response|redirected_to)\(conn, ([^)]+)\)(.*)$/, m2)) {
case2 = 1
c2_helper = m2[1]
c2_status = m2[2]
c2_tail = m2[3]
}
# Case 3: assert %{...} = helper(conn, status)
case3 = 0
if (match(next_line, /^[[:space:]]*assert (%\{[^}]*\}) = (html_response|json_response|text_response|response|redirected_to)\(conn, ([^)]+)\)$/, m3)) {
case3 = 1
c3_pattern = m3[1]
c3_helper = m3[2]
c3_status = m3[3]
}
# If we matched Case 1, 2, or 3, emit the merged line
if (case1) {
print indent c1_var " = conn |> " verb "(" args ") |> " c1_helper "(" c1_status ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
next
}
if (case2) {
print indent "assert conn |> " verb "(" args ") |> " c2_helper "(" c2_status ")" c2_tail
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
next
}
if (case3) {
print indent "assert " c3_pattern " = conn |> " verb "(" args ") |> " c3_helper "(" c3_status ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
next
}
# If next_line references conn at all, this is Case 4 territory
# (multiple uses without a recognized single-merge pattern)
if (next_line ~ /conn/) {
# Case 4: rename to response
print indent "response = conn |> " verb "(" args ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
# Replace conn with response in next_line
gsub(/conn/, "response", next_line)
print next_line
# Continue replacing conn->response in subsequent lines until scope boundary
renaming = 1
next
}
# No conn reference on next line — leave trigger unchanged (fallback)
print trigger_line
if (blank_lines != "") printf "%s", blank_lines
blank_lines = "" blank_lines = ""
print next_line
next
}
# Renaming mode for Case 4: replace conn with response until scope boundary for (idx = 1; idx <= total_lines; idx++) {
renaming == 1 { line = lines[idx]
# Scope boundary: blank line, "end", reduced indentation, or new conn = assignment
if ($0 ~ /^[[:space:]]*$/ || $0 ~ /^[[:space:]]*end$/ || $0 ~ /^[[:space:]]*conn =/) { # If in renaming mode (Case 4 continuation)
renaming = 0 if (renaming) {
print if (is_hard_scope_boundary(line)) {
next renaming = 0
# Fall through: do NOT continue — let the line be processed normally below
} else {
gsub(/conn/, "response", line)
print line
continue
}
}
# If waiting for next non-blank line after trigger
if (triggered) {
if (line ~ /^[[:space:]]*$/) {
blank_lines = blank_lines line "\n"
continue
}
next_line = line
triggered = 0
# Case 1: var = helper(conn, status)
case1 = 0
if (match(next_line, /^[[:space:]]*([a-z_]+) = (html_response|json_response|text_response|response|redirected_to)\(conn, [^)]+\)$/, m1)) {
case1 = 1
c1_var = m1[1]
c1_helper = m1[2]
match(next_line, /\(conn, ([^)]+)\)/, m1s)
c1_status = m1s[1]
}
# Case 2: assert helper(conn, status) with optional =~ "..."
case2 = 0
if (match(next_line, /^[[:space:]]*assert (html_response|json_response|text_response|response|redirected_to)\(conn, ([^)]+)\)(.*)$/, m2)) {
case2 = 1
c2_helper = m2[1]
c2_status = m2[2]
c2_tail = m2[3]
}
# Case 3: assert %{...} = helper(conn, status)
case3 = 0
if (match(next_line, /^[[:space:]]*assert (%\{[^}]*\}) = (html_response|json_response|text_response|response|redirected_to)\(conn, ([^)]+)\)$/, m3)) {
case3 = 1
c3_pattern = m3[1]
c3_helper = m3[2]
c3_status = m3[3]
}
# If Case 1/2/3 matched, check if conn is used further ahead
# If so, fall through to Case 4
if (case1 || case2 || case3) {
if (conn_used_ahead(idx + 1)) {
case1 = 0; case2 = 0; case3 = 0
}
}
if (case1) {
print indent c1_var " = conn |> " verb "(" args ") |> " c1_helper "(" c1_status ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
continue
}
if (case2) {
print indent "assert conn |> " verb "(" args ") |> " c2_helper "(" c2_status ")" c2_tail
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
continue
}
if (case3) {
print indent "assert " c3_pattern " = conn |> " verb "(" args ") |> " c3_helper "(" c3_status ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
continue
}
# If next_line references conn at all, this is Case 4 territory
if (next_line ~ /conn/) {
print indent "response = conn |> " verb "(" args ")"
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
gsub(/conn/, "response", next_line)
print next_line
renaming = 1
continue
}
# No conn reference on next line — leave trigger unchanged (fallback)
print trigger_line
if (blank_lines != "") printf "%s", blank_lines
blank_lines = ""
print next_line
continue
}
# Detect trigger line: conn = VERB(conn, ARGS)
if (line ~ /^[[:space:]]*conn = (get|post|put|patch|delete|head|options)\(conn, /) {
trigger_line = line
match(line, /^[[:space:]]*/)
indent = substr(line, RSTART, RLENGTH)
rest = line
sub(/^[[:space:]]*conn = /, "", rest)
paren_pos = index(rest, "(")
verb = substr(rest, 1, paren_pos - 1)
inner = substr(rest, paren_pos + 1)
sub(/\)$/, "", inner)
sub(/^conn, /, "", inner)
args = inner
triggered = 1
continue
}
# Normal mode: pass through
if (blank_lines != "") {
printf "%s", blank_lines
blank_lines = ""
}
print line
} }
gsub(/conn/, "response")
print
next
} }
# Normal mode: pass through
{
if (blank_lines != "") {
printf "%s", blank_lines
blank_lines = ""
}
print
}
BEGIN { triggered = 0; renaming = 0; blank_lines = "" }
' "$file" > "$tmpfile" ' "$file" > "$tmpfile"
if $DRY_RUN; then if $DRY_RUN; then