Running mix.test shows Ecto errors

[fmterrorf]

Describe the bug Running mix test produces the following error:

Client #PID<0.758.0> is still using a connection from owner at location:

    :prim_inet.send_recv_reply/2
    (postgrex 0.15.5) lib/postgrex/protocol.ex:2918: Postgrex.Protocol.do_send/3
    (postgrex 0.15.5) lib/postgrex/protocol.ex:1858: Postgrex.Protocol.rebind_execute/4
    (ecto_sql 3.4.5) lib/ecto/adapters/sql/sandbox.ex:370: Ecto.Adapters.SQL.Sandbox.Connection.proxy/3
    (db_connection 2.2.2) lib/db_connection/holder.ex:316: DBConnection.Holder.holder_apply/4
    (db_connection 2.2.2) lib/db_connection.ex:1255: DBConnection.run_execute/5
    (db_connection 2.2.2) lib/db_connection.ex:1342: DBConnection.run/6
    (db_connection 2.2.2) lib/db_connection.ex:595: DBConnection.execute/4
    (ecto_sql 3.4.5) lib/ecto/adapters/postgres/connection.ex:80: Ecto.Adapters.Postgres.Connection.execute/4
    (ecto_sql 3.4.5) lib/ecto/adapters/sql.ex:544: Ecto.Adapters.SQL.execute!/4
    (ecto_sql 3.4.5) lib/ecto/adapters/sql.ex:526: Ecto.Adapters.SQL.execute/5
    (ecto 3.4.6) lib/ecto/repo/queryable.ex:192: Ecto.Repo.Queryable.execute/4
    (ecto 3.4.6) lib/ecto/repo/queryable.ex:17: Ecto.Repo.Queryable.all/3
    (chat_api 0.1.0) lib/chat_api/conversations/helpers.ex:22: ChatApi.Conversations.Helpers.broadcast_conversation_updates_to_slack/1
    (elixir 1.10.4) lib/task/supervised.ex:90: Task.Supervised.invoke_mfa/2
    (stdlib 3.13) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

The connection itself was checked out by #PID<0.758.0> at location:

    (ecto_sql 3.4.5) lib/ecto/adapters/postgres/connection.ex:80: Ecto.Adapters.Postgres.Connection.execute/4
    (ecto_sql 3.4.5) lib/ecto/adapters/sql.ex:544: Ecto.Adapters.SQL.execute!/4
    (ecto_sql 3.4.5) lib/ecto/adapters/sql.ex:526: Ecto.Adapters.SQL.execute/5
    (ecto 3.4.6) lib/ecto/repo/queryable.ex:192: Ecto.Repo.Queryable.execute/4
    (ecto 3.4.6) lib/ecto/repo/queryable.ex:17: Ecto.Repo.Queryable.all/3
    (chat_api 0.1.0) lib/chat_api/conversations/helpers.ex:22: ChatApi.Conversations.Helpers.broadcast_conversation_updates_to_slack/1
    (elixir 1.10.4) lib/task/supervised.ex:90: Task.Supervised.invoke_mfa/2
    (stdlib 3.13) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

To Reproduce Steps to reproduce the behavior:

1. Run mix test

Expected behavior It should not show those errors

Additional context I think the issue is in the slack_controller_test.exs. If you run mix test test/chat_api_web/controllers/slack_controller_test.exs this command it'll show the error above.

You can also see the errors here https://github.com/papercups-io/papercups/pull/613/checks?check_run_id=2027280678#step:6:350

[a8t]

I'm getting this too.

[reichert621]

yup this is a known issue -- the errors show up due to async tasks being run during the tests (e.g. when a message is created, it triggers webhook events and slack notifications to be sent async)

not sure what the best fix is at the moment, but definitely would be nice to reduce the noise :+1:

[fmterrorf]

Looking at this blog post too https://dockyard.com/blog/2019/02/13/understanding-test-concurrency-in-elixir specifically at Ecto.Sandbox's shared mode, but it looks like test are already using that https://github.com/papercups-io/papercups/blob/master/test/support/conn_case.ex#L38.

This isnt a big deal tho, since doesn't seem to be affecting the tests

[reichert621]

yeah... definitely annoying though 😅

[fmterrorf]

~~Hmm when testing the slack_controller_test.exs, we only want to tests the controller right, and not the Conversations.Notification which calls Conversations.Notification.notify/1, which contains the call to Task.start~~

~~What if we mock the calls to Conversations.Notification using Mox. That way the controller tests are solely focused on controllers, and we can make another tests for the Chat.Notification context~~

Nvm. i just noticed that this project is using Mocks

[fmterrorf]

Ok I have a branch that removes these errors

It uses Mock's setup_with_mocks to mock the calls to notify functions (that calls Task.start).

The downside is that you'll have to do async: false coz it doesnt support mocking asynchronously. Here's a comparison of how it affects the test.

# master
Finished in 16.4 seconds
1 doctest, 486 tests, 0 failures

# my branch
Finished in 20.5 seconds
1 doctest, 486 tests, 0 failures

[reichert621]

hmmm let me think about this a bit more @fmterrorf -- i think it's not too urgent at the moment