Proposal: ExUnit.Callbacks.tmp_dir!/0

[Wojtek Mach]

It’s common to create temporary directories for tests, ideally a unique directory per test to run them concurrently.

I’d like to propose adding `ExUnit.Callbacks.tmp_dir!/0` and this is how we could use it:

defmodule MyTest do
use ExUnit.Case, async: true

test "my test" do
assert tmp_dir!() =~ "my test"
assert File.dir?(tmp_dir!())
end
end

The directory is lazily created on the first call, the second call to that function within the same test simply returns the same path.

While the path must be unique per test, I believe it should also be predictable to ease debugging, I picked: tmp/<module>/<test>.

To extract the module & test name, we have two options:

1. Define tmp_dir!/0 as a regular function and use stack trace
2. Define tmp_dir!/0 as a macro

Here’s a proof of concept for option 1:

- the code: https://github.com/wojtekmach/elixir/commit/4c399540802a3cae583c086d865dc90d865df6c8
- example of usage in Elixir test suite: https://github.com/wojtekmach/elixir/commit/01df2551582dd9acdaa2f6ff982d6767763070f1

The downside of using stack trace is it can easily get mangled, people shouldn’t do this:

test "my test" do
tmp_dir!()

Task.async(fn ->
tmp_dir!()
end)
|> Task.await()
end

And instead do that:

test "my test" do
tmp_dir = tmp_dir!()

Task.async(fn ->
tmp_dir
end)
|> Task.await()
end

I believe documenting this might be enough.

This proposal is inspired by https://github.com/golang/go/issues/35998.

Any feedback appreciated!

[Andrea Leopardi]

Personally, given the gotchas you mentioned, I feel like it's easy enough to do this manually in a setup block that it doesn't warrant more stuff added to stdlib.

Andrea

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/066A99E3-B470-44C3-9632-F52E97AB8791%40wojtekmach.pl.

[Manfred Bergmann]

Of course.


Manfred

> To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAM9Rf%2BKYBZ8SFAGONa6BxrrcnRbZdxUyVHjNaFrLoL4Y6FewWA%40mail.gmail.com.

[Devon Estes]

What are the rules for deleting this directory after it’s done being used? Would it be deleted after the test finishes, or would it remain until the user deletes it?

In general this seems ok to me, but it’s also a bit limiting as you only get one directory per test. I’d personally prefer to see something where you get one directory per process instead of one directory per test, or when each call to temp_dir! returns a new, unique directory as this covers some other common use cases (like parallel processing of files in multiple folders) without having to first create the temp_dir for the test and then in that temp_dir additional directories per process. I do this generally using UUIDs to create temporary directories, and I haven’t (personally) had any issues with debugging or anything, but I can see how this might be confusing for folks.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/066A99E3-B470-44C3-9632-F52E97AB8791%40wojtekmach.pl.

--

_________________
Devon Estes
+49 176 2356 4717
www.devonestes.com

[José Valim]

If we are going in this direction, I would rather have @tag tmp_path: true. It automatically generates a path based on the test name and updates the context with said path.

A string can also be given, we create it and clean it before the test.

[Wojtek Mach]

The downside of doing work in setup/0 is if you only need temp dir for one test, unless you mess with tags which adds boilerplate, you’d unnecessarily create dirs for tests that don’t need it. But then again you could argue that if you need it in just one test, do it in that test :) still I think it’s worth removing boilerplate.

Contrary to the above mentioned golang issue, I am not super concerned about leaking temp directories, so I wasn’t planning to do that. Though being able to automatically remove those is arguably the only reason for tight integration with exunit. As you can see the implementation is currently fairly generic.

I focused on the ex_unit case but a more general utility is interesting too.

About @tag tmp_dir:

    @tag tmp_dir: true

     test “foo”, %{tmp_dir: tmp_dir} do

seems to strike a good balance between being explicit and easy enough to use. 👍

On 27 Jun 2020, at 12:47, Devon Estes <devon....@gmail.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGowJcgGwFrF%2Btf%2BjmKnFSzntuVUX7pLb2Mj4jsvP6XyBXXo%2Bw%40mail.gmail.com.

[Manfred Bergmann]

I would argue that the use cases for dealing with files in unit tests might be too diverse for a catch all implementation.
Additionally, file usage in unit tests should be avoided, if possible (by using mocking/faking or so instead) since it requires dependencies in a unit test that usually should not be needed. Except of course you have to test an utility that manages files.


Manfred

> Am 27.06.2020 um 13:01 schrieb José Valim <jose....@dashbit.co>:
>
> If we are going in this direction, I would rather have @tag tmp_path: true. It automatically generates a path based on the test name and updates the context with said path.
>
> A string can also be given, we create it and clean it before the test.
>

> --
> You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4%2B44eW9GeJOwmMbr1RbOp_4k1x9s4b%3DyR3n6SQeDMQUnQ%40mail.gmail.com.

[Wojtek Mach]

Here is a proof of concept for the tag-based solution: https://gist.github.com/wojtekmach/2c59fb3c3c1c274e35d164014662975a

Of course it would be slightly different as part of ExUnit, but you get the idea. (And you can play with _this_ in your projects right now!)

[Leandro Cesquini Pereira]

Looks like @tag tmp_dir: true would be a way to set up individual tests, which would work as an addition to existing setup_all and setup.

What about supporting a new annotation @setup that can be used to call tmp_dir but also custom functions in the test suite, which would be useful to fine-tune setup for specific tests, especially for expensive setups. Eg:

def shared_setup_all(_context) do
  # perform setup
  :ok
end

def shared_setup(_context) do
  # perform setup
  :ok
end

# something that only few tests needs
def do_something_expensive(_context) do
  # perform expensive setup
  :ok
end

setup_all :shared_setup_all
setup :shared_setup

@setup tmp_dir: true
test "my test A", context do
end

@setup tmp_dir: "my_tmp_dir", :do_something_expensive
@tag :slow
test "my test B", context do
end

@tag :mock
test "my test C", context do
end

@tag is a filter (as stated by the doc), so enabling it to run setup/actions sounds like mixing up responsibilities and AFAIK it wouldn't be extensible to support custom setup functions like described above.

Is it too overkill or does it make sense?

On Saturday, June 27, 2020 at 7:43:37 AM UTC-4, Wojtek Mach wrote:

Here is a proof of concept for the tag-based solution: https://gist.github.com/wojtekmach/2c59fb3c3c1c274e35d164014662975a

Of course it would be slightly different as part of ExUnit, but you get the idea. (And you can play with _this_ in your projects right now!)

On 27 Jun 2020, at 13:17, Wojtek Mach <woj...@wojtekmach.pl> wrote:

The downside of doing work in setup/0 is if you only need temp dir for one test, unless you mess with tags which adds boilerplate, you’d unnecessarily create dirs for tests that don’t need it. But then again you could argue that if you need it in just one test, do it in that test :) still I think it’s worth removing boilerplate.

Contrary to the above mentioned golang issue, I am not super concerned about leaking temp directories, so I wasn’t planning to do that. Though being able to automatically remove those is arguably the only reason for tight integration with exunit. As you can see the implementation is currently fairly generic.

I focused on the ex_unit case but a more general utility is interesting too.

About @tag tmp_dir:

    @tag tmp_dir: true

     test “foo”, %{tmp_dir: tmp_dir} do

seems to strike a good balance between being explicit and easy enough to use. 👍

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-l...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/066A99E3-B470-44C3-9632-F52E97AB8791%40wojtekmach.pl.

--

_________________
Devon Estes
+49 176 2356 4717
www.devonestes.com

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-l...@googlegroups.com.

[Wojtek Mach]

What about supporting a new annotation @setup that can be used to call tmp_dir but also custom functions in the test suite, which would be useful to fine-tune setup for specific tests, especially for expensive setups. Eg:

@setup tmp_dir: "my_tmp_dir", :do_something_expensive
@tag :slow
test "my test B", context do
end

I don’t think we need the `@setup` tag as I believe we can achieve what you have in mind with describes and their own setups:

    @moduletag :tmp_dir

    describe "foo" do

      setup [:expensive]

      # ...

    end

What do you think?

@tag is a filter (as stated by the doc), so enabling it to run setup/actions sounds like mixing up responsibilities and AFAIK it wouldn't be extensible to support custom setup functions like described above.

FWIW, we also have :capture_log and :timeout tags that are more than just filters. :)

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/d7fb19b1-d06c-4587-a609-f07b3b7ff8b9o%40googlegroups.com.

[Devon Estes]

I think in general the two questions to ask for this are

1) can this be part of a library instead of added to ExUnit, and

2) is this needed for the development of Elixir itself

right?

It seems to me like this isn’t needed for the development of Elixir (although It would make some tests nicer/easier) and it’s definitely possible for this to be part of a library (maybe it could be included in assertions? 

https://hexdocs.pm/assertions/Assertions.html), so it seems to me like this might not be needed in ExUnit itself.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/0A935EC4-BBF3-4351-88AB-0B2EE800B7F1%40wojtekmach.pl.

[José Valim]

I would say this definitely fits 2) - we use it in multiple test suites in Elixir, it is just that the person who implemented this at the time (read: me) did not think about abstracting it properly before.

That same person then proceeded to copy the same helpers to multiple other apps. :P

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGowJcgiR%3DszA2fbZzx_FLNjvw2JLZ59N_xb%3DHfschnwwDoKdg%40mail.gmail.com.