Skip to content

ex_unit: Add :capture_io tag #14623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wojtekmach
Copy link
Member

Possible future work:

  1. Allow ExUnit.start(capture_io: true) to mimic capture_log: true?

  2. Add api to programmatically append to the StringIO input buffer, right now it's only possible on creation:

    {:ok, pid} = StringIO.open("3")
    "3" = IO.gets(pid, "1 + 2")

    The idea is something like this:

    {:ok, pid} = StringIO.open("")
    
    StringIO.buffer("1 + 2")
    "1 + 2" = IO.gets(pid, "> ")
    
    :eof = IO.gets(pid, "> ")
    
    StringIO.buffer("2 + 3")
    "2 + 3" = IO.gets(pid, "> ")

    And so we could replace the usage of:

    capture_io(prompt, fn -> ... end)

    too.

    Maybe instead of calling it StringIO.buffer it could be called StringIO.input_write and StringIO.input_puts to mimic IO.write and IO.puts. Not sure if we'd need .input_binwrite too.

  3. @tag capture_io: :standard_error?

    I think we could do this as long as the test is running in async: false which we'd know and then we can raise otherwise. This is as opposed to capture_io(:standard_error, ...) where ExUnit does not know if it's sync or async.

    If we do this, do we want to capture both? I guess we could accept a list of devices and set it in context:

    @tag capture_io: [:stdio, :standard_error]
    test "foo", %{capture_io: [stdio, stderr]}

    Not sure if :stdio is even semantically correct, my understanding is it's not stdio per se but the group leader. So yeah, it's probably a pass, and a reason to use capture_io in this case, but I thought I'd mention this for completeness.

  4. Make the StringIO available to @tag :capture_log tests in context as %{capture_log: io}. This could be considered a breaking change since today the context is usually set as %{capture_log: true} or %{capture_log: options} but I can't think of a reason to read them back in a test.

@sabiwara
Copy link
Contributor

sabiwara commented Jul 4, 2025

StringIO.buffer("1 + 2")

StringIO.buffer(pid, "1 + 2") right?

Make the StringIO available to @tag :capture_log tests in context as %{capture_log: io}. This could be considered a breaking change since today the context is usually set as %{capture_log: true} or %{capture_log: options} but I can't think of a reason to read them back in a test.

This one seems solvable by picking a slightly different name, captured_log: / captured_io:?

Copy link
Contributor

@sabiwara sabiwara left a comment

Choose a reason for hiding this comment

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

This is a really neat addition 🤩 💜

@josevalim
Copy link
Member

@tag capture_io: :standard_error?

Right, this doesn't play well with StringIO.flush, because then you are flushing data between tests. I think it is fine to keep this specific to stdin/stdout. It is potentially the same reason why adding a similar API to capture_log would be confusing, you cannot use flush and that would lead to more imprecise tests. So I believe this is good to go as is.

time: 0,
tags: %{},
logs: "",
capture_io: "",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should call it stdout for consistency with logs?

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.

3 participants