stavro / arc

:paperclip: Flexible file upload and attachment library for Elixir
1.16k stars 210 forks source link

Red logs [error] {:error, :invalid_file} in test environment #268

Open williamweckl opened 6 years ago

williamweckl commented 6 years ago

Environment

Actual behavior

In my unit tests, when I'm testing arc validations using invalid file types my logs shows [error] {:error, :invalid_file}. All the tests passes but this logs are unnecessary in test environment and gives a false feeling that something is wrong. I'm noticing it since version 0.9.

Expected behavior

Not showing this logs when I'm running application in test environment.

ghost commented 5 years ago

@williamweckl I see

19:14:04.695 request_id=2ljdu7k9mafurd8gbo00fgj1 [error] {:error, :invalid_file_path}

in my tests and I think it's arc_ecto printing that error here. Here's what I'm calling instead of cast_attachments in my code to avoid that message:

  defp maybe_cast_attachments(struct, %{"file" => %Plug.Upload{path: path}} = params) do
    case File.exists?(path) do
      true ->
        cast_attachments(struct, params, [:file], allow_paths: true)

      _ ->
        struct
    end
  end

  defp maybe_cast_attachments(struct, _params), do: struct
dbernheisel commented 5 years ago

@williamweckl can you verify if your current tests are using fake files, or nonexistent files? I haven't run into this, but I also usually do something like echo "Hello" > tests/fixtures/test.txt and use that file for any local tests.

williamweckl commented 5 years ago

@dbernheisel I am using invalid extension files on purpose to test validations.

dbernheisel commented 5 years ago

gotcha. You might want those logs to appear in real logs then, but not in tests. right?

If so, then what I do to suppress those log outputs is to use ExUnit.CaptureIO and either swallow the logs, or assert the output.

williamweckl commented 5 years ago

@dbernheisel that's the exact behavior I'm expecting :) Could this be the default for the lib? In earlier versions this logs was not shown.

danadaldos commented 5 years ago

As an example of this for future people who find this thread (as I did), you can do something like the following. In our case, the line

conn = post(conn, path, %{"file" => photo_upload, "valuation_id" => valuation.id})

was throwing an arc_ecto error in the test results, but not causing the tests to fail. By doing this:

      ExUnit.CaptureLog.capture_log(fn ->
        conn = post(conn, path, %{"file" => photo_upload, "valuation_id" => valuation.id})
        assert text_response(conn, 422) == "Could not insert photo"
      end)

we were able to wrap the test body in capture_log and swallow the stderror while still allowing the test to run (ExUnit evaluates the assertions correctly and will fail on a mismatch). This wasn't clear from the ExUnit.CaptureIO or ExUnit.CaptureLog docs, which both make it seem like something about the capture itself needs to be asserted.