elixir-patterns/smells/common-mistakes.md

# Common Mistakes in Elixir (What the Core Team Avoids)

Patterns that suggest "if you're doing X, you're doing it wrong" — extracted from studying the Elixir standard library and ExUnit source.

---

## 1. Using `Process.sleep` Instead of Message-Based Synchronization

**The mistake:**
```elixir
test "process sends response" do
  pid = spawn(fn -> send(test_pid, :done) end)
  Process.sleep(50)  # Hope 50ms is enough...
  assert_received :done
end
```

**Why it's wrong:** The test is a race condition. It might pass locally but fail on CI. The sleep is arbitrary.

**The fix:**
```elixir
test "process sends response" do
  pid = spawn(fn -> send(test_pid, :done) end)
  assert_receive :done, 1000  # Explicit timeout, proper wait
end
```

**Source:** Elixir's test suite has 39 `assert_receive`/`refute_receive` calls in `task_test.exs` alone, vs 0 `Process.sleep(N)` for synchronization.

### When to Apply This Rule

**Triggers:**
- `Process.sleep` followed by `assert_received` (note: `assert_received` checks mailbox NOW, doesn't wait)
- Tests with "magic number" sleeps (50, 100, 200ms)
- CI failures on tests that always pass locally

**Example — the smell:**
```elixir
test "worker processes job" do
  Worker.submit(job)
  Process.sleep(200)
  assert_received {:job_complete, ^job}
end
```

**Example — fixed:**
```elixir
test "worker processes job" do
  Worker.submit(job)
  assert_receive {:job_complete, ^job}, 5000
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- You're testing that something does NOT happen within a time window (use `refute_receive` with explicit timeout instead)
- The test requires a minimum elapsed time as part of the assertion (e.g., testing a cooldown)

**Example of acceptable use:**
```elixir
test "debouncer doesn't fire before window" do
  Debouncer.trigger(:action, 100)
  refute_receive :action, 50  # Verify it HASN'T fired after 50ms
  assert_receive :action, 200  # Then verify it DOES fire
end
```

**Why it's OK here:** The `refute_receive` with a timeout IS the assertion — we're verifying that the message hasn't arrived yet. The timing is the behavior under test.

---

## 2. Not Using `start_supervised` in Tests

**The mistake:**
```elixir
setup do
  {:ok, pid} = MyGenServer.start_link(name: :my_server)
  on_exit(fn -> GenServer.stop(pid) end)
  %{pid: pid}
end
```

**Why it's wrong:** If the process crashes during the test, `on_exit` will try to stop an already-dead process. If `start_link` links to the test process, a crash kills the test before cleanup.

**The fix:**
```elixir
setup do
  pid = start_supervised!(MyGenServer)
  %{pid: pid}
end
```

**Source:** `lib/ex_unit/lib/ex_unit/callbacks.ex:520-568` — `start_supervised` is designed specifically for this: guaranteed shutdown in reverse order, no leaked processes, no race conditions.

### When to Apply This Rule

**Triggers:**
- `GenServer.start_link` or `Supervisor.start_link` in test setup blocks
- Manual `on_exit` cleanup for processes
- Test failures that leave zombie processes (ports exhausted, name conflicts)

**Example — the smell:**
```elixir
setup do
  {:ok, cache} = MyApp.Cache.start_link(name: :test_cache)
  {:ok, db} = MyApp.DB.start_link(pool_size: 1)
  on_exit(fn ->
    GenServer.stop(cache)
    GenServer.stop(db)
  end)
  %{cache: cache, db: db}
end
```

**Example — fixed:**
```elixir
setup do
  cache = start_supervised!({MyApp.Cache, name: :test_cache})
  db = start_supervised!({MyApp.DB, pool_size: 1})
  %{cache: cache, db: db}
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Testing supervision tree behavior itself (restart strategies, shutdown order)
- The process must be linked to the test process to test crash propagation
- You need a process started OUTSIDE ExUnit's supervisor (e.g., testing distributed node behavior)

**Example of acceptable use:**
```elixir
test "crash propagates to caller" do
  Process.flag(:trap_exit, true)
  {:ok, pid} = CrashableWorker.start_link(self())
  send(pid, :crash)
  assert_receive {:EXIT, ^pid, :boom}
end
```

**Why it's OK here:** The test is verifying link/exit behavior. Using `start_supervised!` would put the process under ExUnit's supervisor, which would intercept the crash and defeat the purpose of the test.

---

## 3. Asserting Exact Equality on Concurrent/Shared Output

**The mistake:**
```elixir
# In an async test
test "logs error message" do
  assert capture_io(:stderr, fn -> Logger.error("oops") end) == "[error] oops\n"
end
```

**Why it's wrong:** With `async: true`, other tests may write to `:stderr` simultaneously. The captured output may include their messages.

**The fix:**
```elixir
test "logs error message" do
  assert capture_io(:stderr, fn -> Logger.error("oops") end) =~ "oops"
end
```

**Source:** `lib/ex_unit/lib/ex_unit/capture_io.ex` docs explicitly warn: "use `=~` instead of `==` for assertions on `:stderr` if your tests are async"

### When to Apply This Rule

**Triggers:**
- `== "..."` assertions on `capture_io(:stderr, ...)` in async tests
- Tests that fail intermittently with "extra" output in the captured string
- Assertions on Logger output that include timestamps or metadata

**Example — the smell:**
```elixir
use ExUnit.Case, async: true

test "warns on deprecation" do
  output = capture_io(:stderr, fn -> MyModule.deprecated_func() end)
  assert output == "[warning] deprecated_func/0 is deprecated\n"
end
```

**Example — fixed:**
```elixir
use ExUnit.Case, async: true

test "warns on deprecation" do
  output = capture_io(:stderr, fn -> MyModule.deprecated_func() end)
  assert output =~ "deprecated_func/0 is deprecated"
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Capturing `:stdio` (group leader), which is per-process and safe for async tests
- Running with `async: false` where no concurrent output interference is possible
- Testing exact formatted output in a synchronous, isolated context

**Example of acceptable use:**
```elixir
use ExUnit.Case, async: true

test "prints exact format to stdout" do
  # :stdio uses group leader — per-process, no interference
  assert capture_io(fn -> IO.puts("hello") end) == "hello\n"
end
```

**Why it's OK here:** Capturing `:stdio` (the default) uses the process group leader, which is isolated per test process. No other test can pollute it, even in async mode.

---

## 4. Registering Global Names in Async Tests

**The mistake:**
```elixir
defmodule MyTest do
  use ExUnit.Case, async: true

  test "starts a server" do
    {:ok, _} = GenServer.start_link(MyServer, [], name: :my_server)
    # Another test instance might try to register the same name!
  end
end
```

**Why it's wrong:** Registered names are global. Two concurrent test runs will collide on `:my_server`.

**The fix:**
```elixir
test "starts a server", %{test: test_name} do
  {:ok, _} = GenServer.start_link(MyServer, [], name: test_name)
  # test_name is unique per test
end
```

**Source:** `lib/elixir/test/elixir/registry_test.exs:29` — `name = :"#{config.test}_#{partitions}_#{inspect(keys)}"` — always derives unique names from test context.

### When to Apply This Rule

**Triggers:**
- Hardcoded atom names in `GenServer.start_link` inside async test modules
- `{:error, {:already_started, _}}` errors in test output
- Tests that pass alone but fail when the full suite runs

**Example — the smell:**
```elixir
use ExUnit.Case, async: true

test "cache stores and retrieves" do
  {:ok, _} = Cachex.start_link(name: :test_cache)
  Cachex.put(:test_cache, :key, "value")
  assert Cachex.get!(:test_cache, :key) == "value"
end
```

**Example — fixed:**
```elixir
use ExUnit.Case, async: true

test "cache stores and retrieves", %{test: test_name} do
  {:ok, _} = Cachex.start_link(name: test_name)
  Cachex.put(test_name, :key, "value")
  assert Cachex.get!(test_name, :key) == "value"
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- The test module uses `async: false` and properly cleans up the named process
- You're testing the name registration behavior itself
- Using `start_supervised!` which handles cleanup automatically (though names still can't collide across concurrent modules)

**Example of acceptable use:**
```elixir
use ExUnit.Case, async: false  # Explicitly synchronous

setup do
  pid = start_supervised!({MyServer, name: :singleton_server})
  %{pid: pid}
end

test "singleton server responds" do
  assert MyServer.ping(:singleton_server) == :pong
end
```

**Why it's OK here:** `async: false` guarantees no other test module runs concurrently. The named process won't collide, and `start_supervised!` ensures cleanup between tests.

---

## 5. Testing Private Functions Directly

**The mistake:**
```elixir
# Exposing private implementation for testing
@doc false
def __internal_transform__(data), do: ...

# In test:
test "internal transform works" do
  assert MyModule.__internal_transform__(%{}) == %{transformed: true}
end
```

**Why it's wrong:** Tests become coupled to implementation. You can't refactor without breaking tests. The public API is the contract.

**The fix:** Test through the public interface. If a private function is complex enough to need its own tests, it should probably be its own module.

**Source:** The Elixir test suite tests public APIs exclusively. `gen_server_test.exs` tests `GenServer.call/cast/stop` — never the internal `handle_*` callbacks directly.

### When to Apply This Rule

**Triggers:**
- Functions prefixed with `__` or marked `@doc false` being called in tests
- Test file imports or aliases internal modules not part of the public API
- Tests that break after internal refactoring even though external behavior is unchanged

**Example — the smell:**
```elixir
defmodule MyApp.Parser do
  def parse(input), do: input |> tokenize() |> build_ast()

  # Made public just for testing!
  @doc false
  def tokenize(input), do: ...
  @doc false
  def build_ast(tokens), do: ...
end

# Test:
test "tokenizer splits on commas" do
  assert MyApp.Parser.tokenize("a,b,c") == ["a", "b", "c"]
end
```

**Example — fixed:**
```elixir
# Test through the public API
test "parser handles comma-separated input" do
  assert MyApp.Parser.parse("a,b,c") == %AST{nodes: ["a", "b", "c"]}
end

# OR extract into its own module if tokenizing is genuinely reusable:
defmodule MyApp.Tokenizer do
  def tokenize(input), do: ...
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- The "private" function is actually a public utility that other modules depend on (it should be `@doc`'d)
- Property-based testing where you need to verify invariants on intermediate transformations
- Testing a complex algorithm step-by-step during development (remove these tests before merging)

**Example of acceptable use:**
```elixir
# Pure algorithmic module where each step has documented guarantees
defmodule MyApp.Compiler.Optimizer do
  @doc "Remove dead code branches. Used by Compiler pipeline."
  def eliminate_dead_code(ast), do: ...

  @doc "Inline constant expressions. Used by Compiler pipeline."
  def inline_constants(ast), do: ...
end
```

**Why it's OK here:** These aren't private functions exposed for testing — they're public steps in a compilation pipeline. Each has its own documented contract and is used by other modules.

---

## 6. Not Cleaning Up After Global State Changes

**The mistake:**
```elixir
test "changes log level" do
  Logger.configure(level: :error)
  # ... test stuff ...
  # Oops, forgot to restore! All subsequent tests have wrong log level.
end
```

**Why it's wrong:** Contaminates the test environment for all subsequent tests. Causes mysterious failures in unrelated tests.

**The fix:**
```elixir
test "changes log level" do
  Logger.configure(level: :error)
  # ... test stuff ...
after
  Logger.configure(level: :debug)
end

# Or better, use on_exit in setup:
setup do
  on_exit(fn -> Logger.configure(level: :debug) end)
end
```

**Source:** `lib/logger/test/logger_test.exs:12-17` — Every Logger config change has a corresponding `on_exit` restoration. `lib/logger/test/test_helper.exs:57-65` — `capture_log` uses `after` to always restore level.

### When to Apply This Rule

**Triggers:**
- `Application.put_env`, `System.put_env`, `Logger.configure` in tests without corresponding restoration
- Tests that pass in isolation but fail in the full suite
- "Works on my machine" but fails in CI (different test ordering)

**Example — the smell:**
```elixir
test "respects timezone setting" do
  System.put_env("TZ", "UTC")
  assert MyApp.current_time().zone == "UTC"
  # System env is now polluted for all remaining tests!
end
```

**Example — fixed:**
```elixir
test "respects timezone setting" do
  original = System.get_env("TZ")
  System.put_env("TZ", "UTC")
  on_exit(fn ->
    if original, do: System.put_env("TZ", original), else: System.delete_env("TZ")
  end)

  assert MyApp.current_time().zone == "UTC"
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- The state change is idempotent and matches the test suite's baseline (setting it to what it already is)
- You're in an integration test with a completely isolated environment (Docker container, separate VM)

**Example of acceptable use:**
```elixir
# Test helper that always restores to known baseline
setup do
  # This IS the baseline — safe to set without storing original
  Logger.configure(level: :warning)
  on_exit(fn -> Logger.configure(level: :warning) end)
end
```

**Why it's OK here:** The level being set IS the test suite's standard baseline. Even if cleanup "fails," the state is already correct. The `on_exit` is belt-and-suspenders.

---

## 7. Nested `describe` Blocks

**The mistake:**
```elixir
describe "users" do
  describe "admin users" do  # This won't compile!
    test "can delete" do
    end
  end
end
```

**Why it's wrong:** ExUnit explicitly prevents nested describe blocks. The framework raises: "cannot call describe inside another describe."

**The fix:** Use flat describe blocks with descriptive names, or prefix test names:
```elixir
describe "admin users - deletion" do
  test "can delete other users" do
  end
end
```

**Source:** `lib/ex_unit/lib/ex_unit/callbacks.ex:433-437` — `no_describe!` check prevents nesting.

### When to Apply This Rule

**Triggers:**
- Compile error: "cannot call describe inside another describe"
- Desire to organize tests hierarchically (coming from RSpec/Jest habits)
- Test file with many describe blocks that feel like they should be nested

**Example — the smell:**
```elixir
describe "API" do
  describe "v1" do      # Won't compile!
    describe "users" do  # Won't compile!
      test "list" do ... end
    end
  end
end
```

**Example — fixed:**
```elixir
describe "API v1 users - list" do
  test "returns paginated results" do ... end
  test "filters by role" do ... end
end

describe "API v1 users - create" do
  test "validates email" do ... end
  test "hashes password" do ... end
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- You're using a testing library that extends ExUnit with nested contexts (though this is rare and usually discouraged in Elixir culture)

**Example of acceptable use:**
```elixir
# Flat describes with shared setup via module attributes or helper functions
describe "admin users" do
  setup [:create_admin]

  test "can delete other users", %{admin: admin} do ... end
  test "can promote users", %{admin: admin} do ... end
end

describe "regular users" do
  setup [:create_user]

  test "cannot delete others", %{user: user} do ... end
end
```

**Why it's OK here:** Flat describe blocks with named setup functions achieve the same organization as nesting, while staying within ExUnit's design philosophy.

---

## 8. Using `assert` Where `assert_receive` Belongs

**The mistake:**
```elixir
test "message is sent" do
  send(self(), {:result, 42})
  assert {:result, 42} in Process.info(self(), :messages) |> elem(1)
end
```

**Why it's wrong:** Reinvents the wheel poorly. Doesn't wait for async messages. No pattern matching. Bad failure messages.

**The fix:**
```elixir
test "message is sent" do
  send(self(), {:result, 42})
  assert_received {:result, 42}
end
```

**Source:** ExUnit provides specialized assertion macros for messages precisely because generic `assert` is inadequate for mailbox testing.

### When to Apply This Rule

**Triggers:**
- `Process.info(self(), :messages)` in test code
- `:erlang.process_info` for mailbox inspection
- Manual mailbox polling loops in tests

**Example — the smell:**
```elixir
test "broadcast reaches subscriber" do
  PubSub.subscribe(:events)
  PubSub.broadcast(:events, :hello)
  Process.sleep(50)
  {_, messages} = Process.info(self(), :messages)
  assert Enum.any?(messages, fn msg -> msg == :hello end)
end
```

**Example — fixed:**
```elixir
test "broadcast reaches subscriber" do
  PubSub.subscribe(:events)
  PubSub.broadcast(:events, :hello)
  assert_receive :hello
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Testing mailbox ordering (need to inspect all messages at once)
- Asserting on message count rather than content
- Building test utilities that need raw mailbox access

**Example of acceptable use:**
```elixir
test "exactly 3 events are received" do
  trigger_events(3)
  # Wait for all to arrive
  assert_receive :event, 1000
  assert_receive :event, 100
  assert_receive :event, 100
  # Verify no extras
  refute_receive :event, 100
end
```

**Why it's OK here:** We need to assert both presence AND absence of messages, with ordering guarantees. The sequence of `assert_receive` + `refute_receive` is the idiomatic way to verify "exactly N messages."

---

## 9. Forgetting `Process.flag(:trap_exit, true)` When Testing Linked Processes

**The mistake:**
```elixir
test "handles process crash" do
  task = Task.async(fn -> raise "boom" end)
  # Test process crashes because it's linked to the task!
  assert catch_exit(Task.await(task))
end
```

**Why it's wrong:** Without trapping exits, the linked process crash kills the test process before `catch_exit` can catch anything.

**The fix:**
```elixir
test "handles process crash" do
  Process.flag(:trap_exit, true)
  task = Task.async(fn -> raise "boom" end)
  assert {{%RuntimeError{}, _}, _} = catch_exit(Task.await(task))
end
```

**Source:** `lib/elixir/test/elixir/task_test.exs:300,308,316,327` — Every test that expects a linked process to crash sets `:trap_exit` first.

### When to Apply This Rule

**Triggers:**
- Tests using `Task.async` where the task might crash
- Tests that verify process crash behavior
- Unexplained test crashes with no assertion failure message
- `** (EXIT from #PID<...>)` in test output

**Example — the smell:**
```elixir
test "supervisor restarts crashed child" do
  {:ok, sup} = Supervisor.start_link([{Worker, []}], strategy: :one_for_one)
  [{_, pid, _, _}] = Supervisor.which_children(sup)
  Process.exit(pid, :kill)
  # Test might crash here if linked!
  Process.sleep(100)
  [{_, new_pid, _, _}] = Supervisor.which_children(sup)
  assert new_pid != pid
end
```

**Example — fixed:**
```elixir
test "supervisor restarts crashed child" do
  Process.flag(:trap_exit, true)
  {:ok, sup} = Supervisor.start_link([{Worker, []}], strategy: :one_for_one)
  [{_, pid, _, _}] = Supervisor.which_children(sup)
  ref = Process.monitor(pid)
  Process.exit(pid, :kill)
  assert_receive {:DOWN, ^ref, :process, ^pid, :killed}

  [{_, new_pid, _, _}] = Supervisor.which_children(sup)
  assert new_pid != pid
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Using `start_supervised!` (ExUnit's test supervisor handles the linking)
- Testing that a crash DOES propagate (the test crashing IS the assertion — use `catch_exit` at the call site)
- The process is started with `start_link` under ExUnit's supervisor

**Example of acceptable use:**
```elixir
test "unlinked task doesn't crash caller" do
  # Task.async_stream with ordered: false doesn't link
  results = Task.async_stream([1, 2, 3], fn
    2 -> raise "boom"
    n -> n * 2
  end, on_timeout: :kill_task)
  |> Enum.to_list()

  assert {:exit, _} = Enum.find(results, &match?({:exit, _}, &1))
end
```

**Why it's OK here:** `Task.async_stream` handles the linking internally and converts crashes to tagged results. The test process isn't directly linked to the failing task.

---

## 10. Writing Flaky Tests with Timing Assumptions

**The mistake:**
```elixir
test "debounce fires after 100ms" do
  start_debounce(:my_action, 100)
  Process.sleep(110)  # "should be enough"
  assert_received :my_action
end
```

**Why it's wrong:** System load, GC pauses, or CI resource contention can make 110ms insufficient. The test will flake.

**The fix:** Use generous timeouts with `assert_receive`:
```elixir
test "debounce fires after delay" do
  start_debounce(:my_action, 100)
  assert_receive :my_action, 1000  # generous timeout, still fast on success
end
```

Or better, make the system under test notify completion:
```elixir
test "debounce fires callback" do
  start_debounce(fn -> send(self(), :fired) end, 100)
  assert_receive :fired, 1000
end
```

**Source:** Elixir's own `ExUnit.configure` allows setting `assert_receive_timeout` globally (default 100ms, CI uses 300ms via env var `ELIXIR_ASSERT_TIMEOUT`).

### When to Apply This Rule

**Triggers:**
- `Process.sleep(N)` where N is close to the expected delay (sleep 110ms for a 100ms timer)
- Tests tagged `@tag :flaky` or skipped in CI
- Tests with "TODO: increase timeout" comments

**Example — the smell:**
```elixir
test "rate limiter allows after window" do
  RateLimiter.hit(:endpoint)
  RateLimiter.hit(:endpoint)  # Should be rejected
  assert RateLimiter.hit(:endpoint) == {:error, :rate_limited}

  Process.sleep(1010)  # Wait for 1s window to expire
  assert RateLimiter.hit(:endpoint) == :ok
end
```

**Example — fixed:**
```elixir
test "rate limiter allows after window" do
  RateLimiter.hit(:endpoint)
  RateLimiter.hit(:endpoint)
  assert RateLimiter.hit(:endpoint) == {:error, :rate_limited}

  # Use the rate limiter's own notification mechanism
  assert_receive {:window_reset, :endpoint}, 5000
  assert RateLimiter.hit(:endpoint) == :ok
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Testing actual wall-clock behavior with very generous margins (10x the expected time)
- Performance benchmarks where timing IS the measurement
- The system under test has no notification mechanism and can't be modified

**Example of acceptable use:**
```elixir
test "connection timeout fires within expected range" do
  {time_us, {:error, :timeout}} = :timer.tc(fn ->
    HttpClient.get("http://10.255.255.1", timeout: 1000)
  end)

  # Generous bounds: should be 1000ms, accept 800-3000ms
  assert time_us > 800_000
  assert time_us < 3_000_000
end
```

**Why it's OK here:** The timing IS the assertion — we're verifying the timeout mechanism works correctly. The bounds are generous enough to handle system load without being so wide as to be meaningless.

---

## 11. Not Using `=~` for Regex/Partial Matching

**The mistake:**
```elixir
test "error message" do
  {:error, msg} = do_thing()
  assert msg == "failed to connect to localhost:5432 (connection refused)"
end
```

**Why it's wrong:** The message might include timestamps, PIDs, or other dynamic content. Any format change breaks the test.

**The fix:**
```elixir
test "error message" do
  {:error, msg} = do_thing()
  assert msg =~ "connection refused"
  # Or with regex:
  assert msg =~ ~r/failed to connect to .+:\d+/
end
```

**Source:** `lib/elixir/test/elixir/gen_server_test.exs:70-82` uses `~r"expected :name option to be one of the following:"` in `assert_raise` — testing the stable part, ignoring the dynamic rest.

### When to Apply This Rule

**Triggers:**
- `assert msg == "..."` where the string contains dynamic parts (PIDs, timestamps, paths)
- Tests that break after minor wording changes in error messages
- Assertions on formatted output from Logger or IO

**Example — the smell:**
```elixir
test "reports compile error" do
  assert_raise CompileError, "nofile:1: undefined function foo/0 (there is no such import)", fn ->
    Code.compile_string("foo()")
  end
end
```

**Example — fixed:**
```elixir
test "reports compile error" do
  assert_raise CompileError, ~r/undefined function foo\/0/, fn ->
    Code.compile_string("foo()")
  end
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- The exact output IS the contract (e.g., a formatter that must produce deterministic output)
- Testing serialization where byte-exact output matters
- The string is fully under your control with no dynamic parts

**Example of acceptable use:**
```elixir
test "JSON encoder produces exact output" do
  assert Jason.encode!(%{a: 1, b: 2}) == ~s({"a":1,"b":2})
end
```

**Why it's OK here:** The JSON encoder's exact output format IS the contract. Users depend on the precise serialization. A partial match would miss regressions in formatting.

---

## 12. Relying on Process.alive? Without Synchronization

**The mistake:**
```elixir
test "process stops" do
  GenServer.stop(pid)
  refute Process.alive?(pid)  # Race condition!
end
```

**Why it's wrong:** `GenServer.stop` is synchronous for the server's exit, but the `Process.alive?` check and :DOWN signal delivery have a timing gap.

**The fix:**
```elixir
test "process stops" do
  ref = Process.monitor(pid)
  GenServer.stop(pid)
  assert_receive {:DOWN, ^ref, :process, ^pid, :normal}
end
```

**Source:** `lib/elixir/test/elixir/supervisor_test.exs:289-293` — `assert_kill` helper always uses monitor + assert_receive, never `Process.alive?` polling.

### When to Apply This Rule

**Triggers:**
- `Process.alive?/1` in test assertions
- `refute Process.alive?(pid)` after killing/stopping a process
- Polling loops checking if a process has died

**Example — the smell:**
```elixir
test "worker shuts down gracefully" do
  pid = start_supervised!(Worker)
  Worker.shutdown(pid)
  Process.sleep(50)
  refute Process.alive?(pid)
end
```

**Example — fixed:**
```elixir
test "worker shuts down gracefully" do
  pid = start_supervised!(Worker)
  ref = Process.monitor(pid)
  Worker.shutdown(pid)
  assert_receive {:DOWN, ^ref, :process, ^pid, :normal}, 5000
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Checking that a process IS alive (existence check, not death check) — no race condition there
- In production code (not tests) where you need a non-blocking liveness check and can handle the race

**Example of acceptable use:**
```elixir
test "start_link creates a running process" do
  {:ok, pid} = MyServer.start_link([])
  assert Process.alive?(pid)  # Just started — guaranteed alive
end
```

**Why it's OK here:** We just created the process synchronously. It's guaranteed to be alive. There's no race condition when checking for liveness immediately after creation.

---

## 13. Using `== ""` for Empty Capture Assertions in Async Tests

**The mistake:**
```elixir
# In an async test
test "no output on success" do
  assert capture_io(:stderr, fn -> do_quiet_thing() end) == ""
end
```

**Why it's wrong:** Another async test might write to stderr during your capture window, making your capture non-empty.

**The fix:** Either make the test synchronous, or don't assert emptiness on shared devices:
```elixir
# Use :stdio (group leader) which is per-process and safe
test "no output on success" do
  assert capture_io(fn -> do_quiet_thing() end) == ""
end
```

**Source:** `lib/ex_unit/lib/ex_unit/capture_io.ex` docs: "avoid empty captures on `:stderr` with async tests"

### When to Apply This Rule

**Triggers:**
- `capture_io(:stderr, ...)` combined with `== ""` in async tests
- Flaky assertion failures showing unexpected stderr content
- Tests that pass alone but fail in the full async suite

**Example — the smell:**
```elixir
use ExUnit.Case, async: true

test "no warnings on valid input" do
  assert capture_io(:stderr, fn -> MyApp.process("valid") end) == ""
end
```

**Example — fixed:**
```elixir
use ExUnit.Case, async: true

test "no warnings on valid input" do
  # Option 1: Use capture_log instead (isolated per process)
  assert capture_log(fn -> MyApp.process("valid") end) == ""

  # Option 2: Use :stdio which is per-process
  assert capture_io(fn -> MyApp.process("valid") end) == ""
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- The test uses `async: false` (no concurrent interference)
- Capturing `:stdio` (default) which uses the per-process group leader
- The function being tested explicitly writes to a named IO device you control

**Example of acceptable use:**
```elixir
use ExUnit.Case, async: false

test "no stderr output in production mode" do
  Application.put_env(:my_app, :env, :prod)
  on_exit(fn -> Application.put_env(:my_app, :env, :test) end)
  assert capture_io(:stderr, fn -> MyApp.start() end) == ""
end
```

**Why it's OK here:** `async: false` means no other test is running concurrently. The stderr capture is isolated by test scheduling, not by the IO mechanism.

---

## 14. Overriding ExUnit Reserved Tags

**The mistake:**
```elixir
@tag test: "my_custom_value"  # Overwrites ExUnit's :test tag!
```

**Why it's wrong:** ExUnit reserves certain context keys (`:case`, `:file`, `:line`, `:test`, `:async`, `:registered`, `:describe`). Overriding them breaks ExUnit internals.

**The fix:** Use your own tag names that don't conflict:
```elixir
@tag test_type: "integration"
```

**Source:** `lib/ex_unit/lib/ex_unit/callbacks.ex` — ExUnit raises if you try to set reserved keys to different values in setup.

### When to Apply This Rule

**Triggers:**
- Runtime error: "ExUnit reserved field ... was set to a different value"
- Tags named `:test`, `:case`, `:file`, `:line`, `:async`, `:registered`, `:describe`
- Mysterious test failures after adding tags

**Example — the smell:**
```elixir
@tag async: false          # Doesn't actually change async behavior!
@tag file: "custom.exs"   # Breaks file reporting
@tag describe: "my group"  # Conflicts with describe blocks

test "my test" do
  # ...
end
```

**Example — fixed:**
```elixir
# Use the actual ExUnit mechanisms:
use ExUnit.Case, async: false  # This is how you control async

# For custom categorization, use non-reserved names:
@tag category: :integration
@tag speed: :slow
@tag feature: "auth"
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- You're writing ExUnit extensions or custom formatters that intentionally read/write these fields (rare, advanced use)

**Example of acceptable use:**
```elixir
# Custom ExUnit formatter that adds metadata
defmodule MyFormatter do
  use GenServer

  def handle_cast({:test_finished, %{tags: tags}}, state) do
    # Reading reserved tags in a formatter is fine
    IO.puts("#{tags.file}:#{tags.line} - #{tags.test}")
    {:noreply, state}
  end
end
```

**Why it's OK here:** The formatter is reading reserved tags (their intended purpose), not overriding them. ExUnit populates these fields for formatters and reporters to consume.

---

## 15. Complex Conditional Logic in Tests

**The mistake:**
```elixir
test "handles all cases" do
  for input <- [:a, :b, :c] do
    result = process(input)
    if input == :a do
      assert result == 1
    else
      if input == :b do
        assert result == 2
      else
        assert result == 3
      end
    end
  end
end
```

**Why it's wrong:** When this test fails, you don't know which case failed. The logic is harder to read than separate tests. Conditionals in tests suggest you're testing multiple behaviors in one test.

**The fix:** Separate tests for separate behaviors, or use parameterize:
```elixir
# Option 1: Separate tests
test "processes :a" do
  assert process(:a) == 1
end

test "processes :b" do
  assert process(:b) == 2
end

# Option 2: Parameterize (Elixir 1.18+)
use ExUnit.Case, parameterize: [
  %{input: :a, expected: 1},
  %{input: :b, expected: 2},
  %{input: :c, expected: 3}
]

test "processes input", %{input: input, expected: expected} do
  assert process(input) == expected
end
```

**Source:** ExUnit case.ex docs: "If you use parameterized tests and then find yourself adding conditionals in your tests to deal with different parameters, then parameterized tests may be the wrong solution."

### When to Apply This Rule

**Triggers:**
- `if`/`case`/`cond` inside test bodies
- `for` loops with assertions inside (unless trivially mapping input→output)
- Tests that assert different things based on runtime values

**Example — the smell:**
```elixir
test "validates all field types" do
  for {field, value, expected_error} <- [
    {:email, "bad", "invalid email"},
    {:age, -1, "must be positive"},
    {:name, "", "can't be blank"}
  ] do
    changeset = User.changeset(%User{}, %{field => value})
    if expected_error do
      assert errors_on(changeset)[field] == [expected_error]
    else
      assert changeset.valid?
    end
  end
end
```

**Example — fixed:**
```elixir
# Elixir 1.18+ parameterize
use ExUnit.Case, parameterize: [
  %{field: :email, value: "bad", error: "invalid email"},
  %{field: :age, value: -1, error: "must be positive"},
  %{field: :name, value: "", error: "can't be blank"}
]

test "validates #{inspect(field)}", %{field: field, value: value, error: error} do
  changeset = User.changeset(%User{}, %{field => value})
  assert errors_on(changeset)[field] == [error]
end
```

### Exceptions (When This Rule Doesn't Apply)

**It's OK when:**
- Testing a pure function over many inputs where the mapping is trivial and the loop is just convenience
- Property-based tests (StreamData) that generate inputs — conditionals in generators are fine
- The loop tests identical behavior, not different behaviors

**Example of acceptable use:**
```elixir
test "all ASCII digits parse to integers" do
  for char <- ?0..?9 do
    assert Integer.parse(<<char>>) == {char - ?0, ""}
  end
end
```

**Why it's OK here:** There's no conditional logic — every iteration tests the exact same behavior with a trivially predictable result. If one fails, the assertion message includes the specific value. The loop is purely for conciseness.

<!-- PATTERN_COMPLETE -->

---

## Logger.warn/2 is Deprecated

**Smell:** Using `Logger.warn/2` instead of `Logger.warning/2`

**Why it's wrong:** `Logger.warn/2` was deprecated in Elixir 1.11. The standard function is `Logger.warning/2`.

**Example — broken:**
```elixir
Logger.warn("Connection lost: #{reason}")
```

**Example — fixed:**
```elixir
Logger.warning("Connection lost: #{reason}")
```

**Source:** [Elixir 1.11 Changelog](https://hexdocs.pm/elixir/1.11/changelog.html#logger-improvements)

<!-- PATTERN_COMPLETE -->