Make concurrent acceptance tests smoother

[danhper]

Hi,

I am currently working on Hound and would like to have your feedback to make concurrent acceptance tests smoother.
After having discussed with @josevalim in HashNuke/hound#71, here is what we came up with:
HashNuke/hound#98

Basically, we can now pass any metadata to the driver, it will be sent through the UA string.
The serialization/deserialization will be handled automatically so that it works with any browser or driver.
An improvement compared to the current approach is that we do not rely on cookies, so PhantomJS should also work fine, at least for the Ecto part.

A sample usage would be

setup do
  :ok = Ecto.Adapters.SQL.Sandbox.checkout(YourApp.Repo)
  Hound.start_session(metadata: %{repo: YourApp.Repo, owner: self()}
end

We could then have the plug to extract the metadata and allow the connection:

defmodule Phoenix.Ecto.SQL.Sandbox do
  def init(opts \\ []) do
    Keyword.get(opts, :sandbox, Ecto.Adapters.SQL.Sandbox)
  end

  def call(conn, sandbox) do
    conn
    |> get_resp_header("user-agent")
    |> List.first
    |> Hound.Metadata.extract
    |> allow_sandbox_access(sandbox)

    conn
  end

  defp allow_sandbox_access(%{repo: repo, owner: owner}, sandbox) do
    Enum.each(List.wrap(repo), &sandbox.allow(&1, owner, self()))
  end
  defp allow_sandbox_access(_metadata, _sandbox), do: :ok
end

I believe the above example should work but I did not try it yet.
As it depends directly on Hound, I am not sure if it is ok to add
this here, or if it is better we create another package dependent
on both phoenix_ecto and hound to avoid adding dependencies.

Anyway, I would very much like to have your feedback.

Thank you!

[josevalim]

Thank you @tuvistavie! I will review the Hound stuff soon.

In general, I agree with this but we should not depend on Hound in the Plug. We should agree on a contract on how those are encoded in the user agent, so projects like Wallaby also work.

@tokafish @keathley, what are your thoughts?

[keathley]

Using the user agent seems like a good solution to me. If this is the contract that we'd like to have does it make sense to move the encoding/decoding logic into phoenix_ecto? The only concern I have with sharing this logic is that it introduces a dependency on phoenix_ecto which we currently don't have. Thats not a large concern though.

[josevalim]

We don't need to move the logic to phoenix_ecto. We just need to agree on
the encoding/decoding format and have everyone abide to it. It will
probably be a tuple with term_to_binary and Base64 encoding.

On Sunday, April 3, 2016, Chris Keathley notifications@github.com wrote:

Using the user agent seems like a good solution to me. If this is the
contract that we'd like to have does it make sense to move the
encoding/decoding logic into phoenix_ecto? The only concern I have with
sharing this logic is that it introduces a dependency on phoenix_ecto
which we currently don't have. Thats not a large concern though.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#41 (comment)

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[keathley]

That seems good to me.

[danhper]

I also agree it is better to avoid having dependencies between the packages where we do not need them.

In the current encoding, I am appending a string /HoundMetadata (metadata) to the normal user agent.
Here, metadata is a map or keyword list joined by a , with each key/value pairs encoded with term_to_binary + b64 without padding and joined with a =.
So for example, given f = b64 . term_to_binary, {a: 1, b: 2} will become

/HoundMetadata (f(:a)=f(1),f(:b)=f(2))

Of course, if we are to make this a common format, I suppose we should replace HoundMetadata by Metadata or something.
From what I have checked, it should work perfectly fine with any browser, and it works with fairly complex types.

Please let me know if you have suggestions to improve the encoding format.

[josevalim]

My propose in the Hound issues tracker was:

BeamMetadata (#{{:v1, map} |> :erlang.term_to_binary |> Binary.url_encode64})

We could convert it to key=value& pairs but I don't see much the purpose as it would be easier to just encode everything. :)

[josevalim]

@tuvistavie could you please send a pull request to Phoenix.Ecto that uses the new metadata we have in Hound? Thank you! :)

[josevalim]

Then let's do new hound and phoenix-ecto releases and get the smoother concurrent tests out there :D

[keathley]

Excited to get this into Wallaby as well 👍

[danhper]

@josevalim Sure! I should have time to do this during next week!

[josevalim]

@tuvistavie do you think it would be possible to have this sooner? We want to release a phoenix-rc. If you can't focus on it, it is OK, I can tackle the missing parts as you already did most of the work. :) Thank you.

[danhper]

@josevalim Alright, it should not take too long so I will give it a try tonight!

[josevalim]

❤️ 💚 💙 💛 💜