Skip to content

Commit a6e9e63

Browse files
authored
Only use async: true for ConnCase error tests on Postgres and no-Ecto apps (#6783)
Fixes a long-standing flaky test issue in generated SQLite applications where running `mix test` would intermittently fail with: ** (Exqlite.Error) Database busy Root Cause `phx.new` generated `error_html_test.exs` and `error_json_test.exs` with hardcoded `use ConnCase, async: true`. In apps with Ecto enabled, `ConnCase` includes a setup block that calls `DataCase.setup_sandbox(tags)`. When ExUnit runs an `async: true` test module concurrently with `async: false` test modules (such as tests generated by `phx.gen.auth`), `setup_sandbox` sees `tags[:async] == true` and checks out a non-shared secondary connection handle (`shared: false`) from the Repo pool. Because SQLite uses single-file database locking, having two active database connections opening sandbox transactions concurrently causes file lock contention whenever a synchronous test inserts records (e.g. `user_fixture()`). Depending on ExUnit test seeding and process scheduling, the secondary connection checkout fails with `(Exqlite.Error) Database busy`. Background When `--database sqlite3` support was added in 5143696 (#4268, April 2021), error controller test templates retained hardcoded `use ConnCase, async: true`. Later, when `phx.gen.auth` introduced support for `async: true` in 3d609e0 (#5689, January 2024), it added a helper to conditionally append `async: true` ONLY for PostgreSQL adapters while leaving non-Postgres adapters as `async: false`. However, `phx.new` was not updated at the time to use the same logic for its generated error test templates. Fix 1. Add `test_case_options/1` in `Phx.New.Generator` matching the behavior of `phx.gen.auth`: - `Ecto.Adapters.Postgres` -> `", async: true"` - `nil` (no Ecto) -> `", async: true"` - non-Postgres adapters (SQLite, MySQL, TDS) -> `""` 2. Update `error_html_test.exs.eex` and `error_json_test.exs.eex` templates to use `<%= @test_case_options %>`. 3. Added test coverage across single app and umbrella project generators for Postgres, `--no-ecto`, SQLite3, MySQL, and MSSQL.
1 parent 1abb125 commit a6e9e63

5 files changed

Lines changed: 84 additions & 11 deletions

File tree

installer/lib/phx_new/generator.ex

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ defmodule Phx.New.Generator do
326326
elixir_install_bin_path: from_elixir_install && elixir_install_bin_path(),
327327
inside_docker_env?: inside_docker_env?,
328328
agents_md: agents_md,
329-
config_regex_E: Version.match?(System.version(), "~> 1.19.3 or ~> 1.20") && "E" || ""
329+
config_regex_E: Version.match?(System.version(), "~> 1.19.3 or ~> 1.20") && "E" || "",
330+
test_case_options: test_case_options(adapter_module)
330331
]
331332

332333
%{project | binding: binding}
@@ -410,6 +411,10 @@ defmodule Phx.New.Generator do
410411
Mix.raise("Unknown database #{inspect(db)}")
411412
end
412413

414+
defp test_case_options(Ecto.Adapters.Postgres), do: ", async: true"
415+
defp test_case_options(nil), do: ", async: true"
416+
defp test_case_options(adapter) when is_atom(adapter), do: ""
417+
413418
defp get_web_adapter("cowboy"),
414419
do:
415420
{:plug_cowboy, "~> 2.7", Phoenix.Endpoint.Cowboy2Adapter,

installer/templates/phx_test/controllers/error_html_test.exs.eex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
defmodule <%= @web_namespace %>.ErrorHTMLTest do
2-
use <%= @web_namespace %>.ConnCase, async: true
2+
use <%= @web_namespace %>.ConnCase<%= @test_case_options %>
33

44
# Bring render_to_string/4 for testing custom views
55
import Phoenix.Template, only: [render_to_string: 4]

installer/templates/phx_test/controllers/error_json_test.exs.eex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
defmodule <%= @web_namespace %>.ErrorJSONTest do
2-
use <%= @web_namespace %>.ConnCase, async: true
2+
use <%= @web_namespace %>.ConnCase<%= @test_case_options %>
33

44
test "renders 404" do
55
assert <%= @web_namespace %>.ErrorJSON.render("404.json", %{}) == %{errors: %{detail: "Not Found"}}

installer/test/phx_new_test.exs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ defmodule Mix.Tasks.Phx.NewTest do
8989
end)
9090

9191
assert_file("phx_blog/test/phx_blog_web/controllers/page_controller_test.exs")
92-
assert_file("phx_blog/test/phx_blog_web/controllers/error_html_test.exs")
93-
assert_file("phx_blog/test/phx_blog_web/controllers/error_json_test.exs")
92+
assert_file("phx_blog/test/phx_blog_web/controllers/error_html_test.exs", "async: true")
93+
assert_file("phx_blog/test/phx_blog_web/controllers/error_json_test.exs", "async: true")
9494
assert_file("phx_blog/test/support/conn_case.ex")
9595
assert_file("phx_blog/test/test_helper.exs")
9696

@@ -557,6 +557,9 @@ defmodule Mix.Tasks.Phx.NewTest do
557557
assert file =~ "inputs: [\"*.{heex,ex,exs}\", \"{config,lib,test}/**/*.{heex,ex,exs}\"]"
558558
refute file =~ "subdirectories:"
559559
end)
560+
561+
assert_file("phx_blog/test/phx_blog_web/controllers/error_html_test.exs", "async: true")
562+
assert_file("phx_blog/test/phx_blog_web/controllers/error_json_test.exs", "async: true")
560563
end)
561564
end
562565

@@ -686,6 +689,14 @@ defmodule Mix.Tasks.Phx.NewTest do
686689
"custom_path/test/support/data_case.ex",
687690
"Ecto.Adapters.SQL.Sandbox.start_owner"
688691
)
692+
693+
assert_file("custom_path/test/custom_path_web/controllers/error_html_test.exs", fn file ->
694+
refute file =~ "async: true"
695+
end)
696+
697+
assert_file("custom_path/test/custom_path_web/controllers/error_json_test.exs", fn file ->
698+
refute file =~ "async: true"
699+
end)
689700
end)
690701
end
691702

@@ -716,8 +727,18 @@ defmodule Mix.Tasks.Phx.NewTest do
716727
"Ecto.Adapters.SQL.Sandbox.start_owner"
717728
)
718729

719-
assert_file("custom_path/.gitignore", "*.db")
720-
assert_file("custom_path/.gitignore", "*.db-*")
730+
assert_file("custom_path/test/custom_path_web/controllers/error_html_test.exs", fn file ->
731+
refute file =~ "async: true"
732+
end)
733+
734+
assert_file("custom_path/test/custom_path_web/controllers/error_json_test.exs", fn file ->
735+
refute file =~ "async: true"
736+
end)
737+
738+
assert_file("custom_path/.gitignore", fn file ->
739+
assert file =~ "*.db"
740+
assert file =~ "*.db-*"
741+
end)
721742
end)
722743
end
723744

@@ -747,6 +768,14 @@ defmodule Mix.Tasks.Phx.NewTest do
747768
"custom_path/test/support/data_case.ex",
748769
"Ecto.Adapters.SQL.Sandbox.start_owner"
749770
)
771+
772+
assert_file("custom_path/test/custom_path_web/controllers/error_html_test.exs", fn file ->
773+
refute file =~ "async: true"
774+
end)
775+
776+
assert_file("custom_path/test/custom_path_web/controllers/error_json_test.exs", fn file ->
777+
refute file =~ "async: true"
778+
end)
750779
end)
751780
end
752781

installer/test/phx_new_umbrella_test.exs

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,17 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
150150
)
151151

152152
assert_file(web_path(@app, "test/#{@app}_web/controllers/page_controller_test.exs"))
153-
assert_file(web_path(@app, "test/#{@app}_web/controllers/error_html_test.exs"))
154-
assert_file(web_path(@app, "test/#{@app}_web/controllers/error_json_test.exs"))
153+
154+
assert_file(
155+
web_path(@app, "test/#{@app}_web/controllers/error_html_test.exs"),
156+
"async: true"
157+
)
158+
159+
assert_file(
160+
web_path(@app, "test/#{@app}_web/controllers/error_json_test.exs"),
161+
"async: true"
162+
)
163+
155164
assert_file(web_path(@app, "test/support/conn_case.ex"))
156165
assert_file(web_path(@app, "test/test_helper.exs"))
157166

@@ -625,6 +634,16 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
625634
assert_file(root_path(app, "config/runtime.exs"), [~r/url: database_url/])
626635

627636
assert_file(web_path(app, "test/support/conn_case.ex"), "DataCase.setup_sandbox(tags)")
637+
638+
assert_file(
639+
web_path(app, "test/custom_path_web/controllers/error_html_test.exs"),
640+
fn file -> refute file =~ "async: true" end
641+
)
642+
643+
assert_file(
644+
web_path(app, "test/custom_path_web/controllers/error_json_test.exs"),
645+
fn file -> refute file =~ "async: true" end
646+
)
628647
end)
629648
end
630649

@@ -652,6 +671,16 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
652671

653672
assert_file(web_path(app, "test/support/conn_case.ex"), "DataCase.setup_sandbox(tags)")
654673

674+
assert_file(
675+
web_path(app, "test/custom_path_web/controllers/error_html_test.exs"),
676+
fn file -> refute file =~ "async: true" end
677+
)
678+
679+
assert_file(
680+
web_path(app, "test/custom_path_web/controllers/error_json_test.exs"),
681+
fn file -> refute file =~ "async: true" end
682+
)
683+
655684
assert_file(root_path(app, ".gitignore"), "*.db")
656685
assert_file(root_path(app, ".gitignore"), "*.db-*")
657686
end)
@@ -679,6 +708,16 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
679708
assert_file(root_path(app, "config/runtime.exs"), [~r/url: database_url/])
680709

681710
assert_file(web_path(app, "test/support/conn_case.ex"), "DataCase.setup_sandbox(tags)")
711+
712+
assert_file(
713+
web_path(app, "test/custom_path_web/controllers/error_html_test.exs"),
714+
fn file -> refute file =~ "async: true" end
715+
)
716+
717+
assert_file(
718+
web_path(app, "test/custom_path_web/controllers/error_json_test.exs"),
719+
fn file -> refute file =~ "async: true" end
720+
)
682721
end)
683722
end
684723

@@ -822,8 +861,8 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
822861
assert_file("another/lib/another/endpoint.ex", ~r/defmodule Another.Endpoint do/)
823862

824863
assert_file("another/test/another/controllers/page_controller_test.exs")
825-
assert_file("another/test/another/controllers/error_html_test.exs")
826-
assert_file("another/test/another/controllers/error_json_test.exs")
864+
assert_file("another/test/another/controllers/error_html_test.exs", "async: true")
865+
assert_file("another/test/another/controllers/error_json_test.exs", "async: true")
827866
assert_file("another/test/support/conn_case.ex")
828867
assert_file("another/test/test_helper.exs")
829868

0 commit comments

Comments
 (0)