Version 0.7.0 breaks previous behaviour

[stevedomin]

Hello,

I recently upgraded one of my project using bypass 0.6 to 0.8 and it completely broke my test suite. It seems that having multiple expect/2 in the same test case is no longer supported.

Example:

defmodule Test do
  setup do
    bypass = Bypass.open()
    {:ok, bypass: bypass}
  end

  test "a test", %{bypass: bypass} do
    Bypass.expect bypass, fn conn ->
      assert "/email" == conn.request_path
      assert "POST" == conn.method
      Plug.Conn.resp(conn, 200, "ok")
    end
   
    Client.make_request()
  end

  test "a second test", %{bypass: bypass} do
    Bypass.expect bypass, fn conn ->
      assert "/email" == conn.request_path
      assert "POST" == conn.method
      Plug.Conn.resp(conn, 404, "not found")
    end

    Client.make_request()
  end
end

With this setup I get the following error:

** (RuntimeError) Route already installed for any, any
    (bypass) lib/bypass/instance.ex:129: Bypass.Instance.do_handle_call/3
    (stdlib) gen_server.erl:636: :gen_server.try_handle_call/4
    (stdlib) gen_server.erl:665: :gen_server.handle_msg/6
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3

This seems like a backward-incompatible change and I haven't a found to make it work as it use to, any help would be appreciated. Thanks!

[MSch]

Thanks for reporting this issue.

Unfortunately I'm unsure how to reproduce this, looking at our PSPDFKit Instant codebase we have test cases with exactly the same pattern that all do work.

Can you post a complete failing example, so I can reproduce and debug?

[stevedomin]

Sorry for the slow reply @MSch, turns out that I slightly mis-represented the failing test case in my issue description, that's why you haven't been able to reproduce it.

The codebase I refer to is actually using setup_all callbacks as opposed to setup ones.

Here is a before/after PR: swoosh/swoosh#149

I'm wondering if that should still be mentioned as a breaking change somewhere, or if setup_all were never expected to work nicely with Bypass.

[MSch]

No worries @stevedomin and I'm glad we figured it out and it's working for you now 👍

You're right, setup_all was never meant to be the right place for Bypass.open to be called, because Bypass needs to be able to verify the expectation, and that happens at the end of the test.

So in that case it's not a breaking change, since it never was expected to work.

I'm closing this, but please let me know if you think there's a way to improve the documentation to make these expectations clearer. A PR is welcome too :)

[stevedomin]

Thanks @MSch, opened a PR with a note in the README: #46