Skip to content

Allow optional context in on_exit callbacks.#15496

Open
christhekeele wants to merge 2 commits into
elixir-lang:mainfrom
christhekeele:exunit-on_exit-context
Open

Allow optional context in on_exit callbacks.#15496
christhekeele wants to merge 2 commits into
elixir-lang:mainfrom
christhekeele:exunit-on_exit-context

Conversation

@christhekeele

Copy link
Copy Markdown
Contributor

Per #elixir-lang-core discussion, this PR allows ExUnit.Callbacks.on_exit/2 to be given a callback function that accepts an argument. This argument can be used to perform conditional cleanup based on test results.

In the discussion I proposed that this argument be an ExUnit.state(). In this implementation, however, I decided to allow it to be the full %ExUnit.Test{} | %ExUnit.TestModule{} struct, under the reasoning that:

  • What the ExUnit.state() is for an %ExUnit.TestModule{} is somewhat unclear
    • I think it's always nil, and when passed along to a callback it loses context as to if a setup/1 test passed, or the callback was provided in setup_all/1
    • I considered instead passing either test.state or test_module.name along, and am still open to this approach
  • This allows the callback author to decide how to handle the full context of setup/1 vs setup_all/1
    • Including writing a single re-usable function that handles both structs and can be used in both
  • Should these structs expose more context beyond status in the future, the conditional cleanup we can do becomes more powerful
    • As a contrived example, a test could now use %ExUnit.Test{time: time_in_ms} to change how it does cleanup
    • It is worth discussing if this is a good thing or not

@christhekeele christhekeele force-pushed the exunit-on_exit-context branch 2 times, most recently from 4cc3f16 to f60d9cf Compare June 16, 2026 16:45
Comment thread lib/ex_unit/lib/ex_unit/on_exit_handler.ex
@christhekeele christhekeele force-pushed the exunit-on_exit-context branch from f60d9cf to 1f876a1 Compare June 16, 2026 17:07
@christhekeele christhekeele force-pushed the exunit-on_exit-context branch from 1f876a1 to 5fe6af8 Compare June 16, 2026 17:14
Comment thread lib/ex_unit/lib/ex_unit/callbacks.ex Outdated
Comment thread lib/ex_unit/lib/ex_unit/on_exit_handler.ex Outdated

@sabiwara sabiwara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants