fix: Address CodeRabbit PR review comments

ocean · ocean · commit a54933bd7989 · 2026-01-01T14:10:25.000+11:00
- Fix struct update pattern in security_test.exs (use map update after
  pattern match assertion)
- Replace unreachable :deallocated clause with :halt in cursor test
- Fix unused variable warnings (_state and _i)
- Improve test cleanup to properly stop repo before file removal
- Add -journal file cleanup to prevent stale files
diff --git a/test/ecto_migration_test.exs b/test/ecto_migration_test.exs
@@ -14,12 +14,21 @@ defmodule Ecto.Adapters.LibSql.MigrationTest do
   setup do
     # Start a fresh repo for each test with a unique database file.
     test_db = "z_ecto_libsql_test-migrations_#{:erlang.unique_integer([:positive])}.db"
-    {:ok, _} = start_supervised({TestRepo, database: test_db})
+    {:ok, pid} = start_supervised({TestRepo, database: test_db})
 
     on_exit(fn ->
+      # Stop the repo before cleaning up files.
+      if Process.alive?(pid) do
+        stop_supervised(TestRepo)
+      end
+
+      # Small delay to ensure file handles are released.
+      Process.sleep(10)
+
       File.rm(test_db)
       File.rm(test_db <> "-shm")
       File.rm(test_db <> "-wal")
+      File.rm(test_db <> "-journal")
     end)
 
     # Foreign keys are disabled by default in SQLite - tests that need them will enable them explicitly.
diff --git a/test/fuzz_test.exs b/test/fuzz_test.exs
@@ -38,6 +38,7 @@ defmodule EctoLibSql.FuzzTest do
       File.rm(db_path)
       File.rm(db_path <> "-shm")
       File.rm(db_path <> "-wal")
+      File.rm(db_path <> "-journal")
     end)
 
     {:ok, state: state, db_path: db_path}
diff --git a/test/security_test.exs b/test/security_test.exs
@@ -38,7 +38,8 @@ defmodule EctoLibSql.SecurityTest do
 
       # Try to use connection A's transaction on connection B by forcing trx_id
       # This tests that transactions are properly scoped to their connection
-      state_b_fake = %EctoLibSql.State{state_b | trx_id: trx_id_a}
+      %EctoLibSql.State{} = state_b
+      state_b_fake = %{state_b | trx_id: trx_id_a}
 
       case EctoLibSql.handle_execute(
              "SELECT 1",
@@ -234,7 +235,7 @@ defmodule EctoLibSql.SecurityTest do
         {:cont, _result, _state} ->
           flunk("Connection B should not access Connection A's cursor")
 
-        {:deallocated, _result, _state} ->
+        {:halt, _result, _state} ->
           flunk("Connection B should not access Connection A's cursor")
       end
 
@@ -309,7 +310,7 @@ defmodule EctoLibSql.SecurityTest do
         )
 
       for i <- 1..100 do
-        {:ok, _, _, state} =
+        {:ok, _, _, _state} =
           EctoLibSql.handle_execute(
             "INSERT INTO concurrent_test (value) VALUES (?)",
             ["value_#{i}"],
@@ -337,7 +338,7 @@ defmodule EctoLibSql.SecurityTest do
 
       # Try to fetch concurrently from multiple processes
       tasks =
-        for i <- 1..5 do
+        for _i <- 1..5 do
           Task.async(fn ->
             EctoLibSql.handle_fetch(
               %EctoLibSql.Query{statement: "SELECT * FROM concurrent_test"},
