Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using hound for concurrent tests #71

Closed
josevalim opened this issue Jan 31, 2016 · 20 comments
Closed

Using hound for concurrent tests #71

josevalim opened this issue Jan 31, 2016 · 20 comments

Comments

@josevalim
Copy link
Contributor

Hi @HashNuke! I tried to ping you on IRC but it did not work out, let me know if you would like this discussion to be moved elsewhere.

Today hound already supports multiple sessions, which would allow us to run tests concurrently. With Ecto 2.0, we are introducing an ownership feature, that would also allow Ecto to run concurrently. The next step is to make both of them work together so we have concurrent integration tests.

Therefore, this issue is more of a question, than an actual feature. If changes need to be done in Hound, they will be minimal.

In order for Ecto tests to run concurrently, we need to tell each process which database connection they should use. For example, in your test:

:ok = Ecto.Adapters.SQL.Sandbox.checkout(MyRepo, [])
parent = self()
Task.start_link fn ->
  :ok = Ecto.Adapters.SQL.Sandbox.allow(MyRepo, parent, self())
end

This means that, in order for this to work with integration tests, we would need to include the connection owner in the request, parse it at the beginning of the request and call allow. This means that Hound would need to support a way to send a header for every request or at least a way to uniquely identify sessions.

So that's the question: when we start a hound_session, would it be possible to configure hound to always add a header or an identifier in whatever request is made by that particular session? If we cannot identify particular sessions, can we at least identify requests?

Note this feature would also need to be supported through redirections. I.e. if the server redirects and the driver automatically redirects, we need to still be able to tag those.

I will be glad to send a PR but I need some guidance on what is and what isn't allowed on the driver side.

@HashNuke
Copy link
Owner

@josevalim sorry. I didn't have IRCcloud on. I'm totally open to changes in Hound.
I read about ecto 2.0 sandboxes in a previous plataformatec blog post. I'll take tomorrow to read more and try Ecto 2.0 sandboxes and ping back. I'll have an informed opinion that way.

@danhper
Copy link
Collaborator

danhper commented Mar 16, 2016

@josevalim @HashNuke I am thinking one of the simplest way, that would definitely work with all browsers and drivers, would be to add this metadata directly to the user agent string.
To have this working, we would have to

  1. Add a way to easily set the user agent, as there is not cross-browser way to do this
  2. Add a utility function to add some metadata to the UA string
  3. Add a utility function to extract the data from the UA string

2 and 3 are not really needed, but I think it is better for the end user to let the library take care of those details.

In the integration tests, we could then do something like

setup do
  Hound.add_metadata_to_ua(%{"some-id" => "some-value"})
end

test "test something" do
  my_metadata = conn |> get_request_header("user-agent") |> List.first |> Hound.extract_metadata_from_ua
end

The only drawback of this approach I can think of is that it could cause problems if someone is
relying too much on the user-agent string for something else.

What do you think?

@josevalim
Copy link
Contributor Author

The question is if drivers can let us customize the user agent.

@danhper
Copy link
Collaborator

danhper commented Mar 17, 2016

AFAIK, all drivers and browsers let us customize this, which is why I think it could be a good solution.

@josevalim
Copy link
Contributor Author

If we can do this, it would be excellent. @tuvistavie can you investigate and send a PR to hound that allows us to configure the user agent when we start a session?

@danhper
Copy link
Collaborator

danhper commented Mar 17, 2016

@josevalim Sure, I am going to give it a try!

@HashNuke
Copy link
Owner

@tuvistavie Adding meta data to UserAgent is a brilliant solution ~! 👍

@danhper
Copy link
Collaborator

danhper commented Mar 17, 2016

@HashNuke Do you know if there is a simple way to do this with Firefox? It is trivial for Chrome and PhantomJS, but for Firefox it seems we need to implement the profile, which seems to be quite a lot work for such a simple task.

@darksheik
Copy link
Contributor

Here was my solution for creating a firefox profile. It was a bit of a chore, but once it is in place it works very well and you can customize with whatever preferences you need. I've even got it set up to use a locally-running proxy, but you may not need that.

To launch the session, create the profile with a Base64-encoded profile directory (zipped) and pass to desiredCapabilities:

b64 = MyApp.BrowserInstance.firefox(local_port, agent.user_agent)
Hound.start_session(%{firefox_profile: b64})

The BrowserInstance module which builds the Base64:

defmodule MyApp.BrowserInstance do
  def firefox(local_port, user_agent) do
    prefs = """
    user_pref("general.useragent.override", "#{user_agent.string}");
    user_pref("network.http.pipelining", true);
    user_pref("network.http.pipelining.aggressive", true);
    user_pref("network.http.pipelining.maxrequests", 8);
    user_pref("network.http.pipelining.ssl", true);
    user_pref("browser.cache.use_new_backend", 1);
    user_pref("browser.tabs.animate", false);
    user_pref("browser.display.show_image_placeholders", false);
    user_pref("network.dns.disableIPv6", true);
    user_pref("browser.startup.homepage", "about:blank");
    user_pref("startup.homepage_welcome_url", "about:blank");
    user_pref("startup.homepage_welcome_url.additional", "about:blank");
    """

    if local_port do
      settings = """
      user_pref("network.proxy.type", 1);
      user_pref("network.proxy.http", "localhost");
      user_pref("network.proxy.http_port", #{local_port.port});
      user_pref("network.proxy.ssl", "localhost");
      user_pref("network.proxy.ssl_port", #{local_port.port});
      user_pref("network.proxy.socks", "localhost");
      user_pref("network.proxy.socks_port", #{local_port.port});
      user_pref("network.proxy.share_proxy_settings", true);
      """
    else
      settings = ""
    end

    profile_dir = temp_path
    File.mkdir(profile_dir)
    settings_path = profile_dir <> "/user.js"
    File.write!(settings_path, settings)

    prefs_path = profile_dir <> "/prefs.js"
    File.write!(prefs_path, prefs)


    zip = profile_dir <> "/profile.zip"

    files = File.ls!(profile_dir)
            |> Enum.map(&String.to_char_list/1)

    :zip.create(String.to_char_list(zip), files,[cwd: profile_dir])

    {b64, _} = System.cmd("base64", [zip])

    File.rm_rf!(profile_dir)
    b64
  end

  defp temp_path do
    rand = Base.encode32(:crypto.rand_bytes(20))
    Path.join(System.tmp_dir, rand)
  end
end

@josevalim
Copy link
Contributor Author

Awesome, if we can make this work for concurrent testing, things will be even easier from the Ecto/Phoenix integration side!

@danhper
Copy link
Collaborator

danhper commented Mar 21, 2016

@darksheik Thank you very much for sharing, this is really helpful!
Would you have the time to send a PR with your implementation and
possibly some tests?
If you don't have the time, just let me know and I will try to integrate
your solution into Hound.
Thank you ! 😄

@darksheik
Copy link
Contributor

A bit swamped at the moment - This works well in my own Ubuntu/Mac infrastructure but I have been meaning to investigate whether I can do this without using the file system.

@HashNuke
Copy link
Owner

@darksheik Thank you for posting details ~!

@danhper
Copy link
Collaborator

danhper commented Mar 23, 2016

After merging #94, we will have the possibility to add the user agent for all browsers:

ua = "whatever string we want"

# firefox
alias Hound.Firefox.Profile
profile = Profile.new |> Profile.set_user_agent(ua) |> Profile.dump
Hound.start_session(%{firefox_profile: profile})

# chrome
Hound.start_session(%{
  browserName: "chrome",
  chromeOptions: %{"args" => ["--user-agent=#{ua}"]}
})

# phantomjs
Hound.start_session(%{
  :browserName => "phantomjs",
  "phantomjs.page.settings.userAgent" => ua
})

However, if we use it to add metadata, I think that having a common interface
to set this will be required.

I was thinking that being able to pass a hound key in desired capabilities
could be a clean way to do this, as it is obvious that we are doing something
hound specific and not driver specific.

Hound.start_session(%{
  hound: %{
    user_agent: ua,
    metadata: %{"some" => "metadata for concurrent tests or anything we like"}
  }
})

What do you think?

@josevalim
Copy link
Contributor Author

That's great. It would be nice, however, if it was a single option indeed, so we can do:

Hound.start_session(user_agent: "our strings")

In particular, I would keep the options for start_session all Hound specific and nest the driver options under the :driver key.

@josevalim
Copy link
Contributor Author

@tuvistavie once this is all said and done on the hound side, could you also please investigate integration from the Phoenix.Ecto side too? Thank you!

@josevalim
Copy link
Contributor Author

/cc @tokafish

@danhper
Copy link
Collaborator

danhper commented Mar 23, 2016

In particular, I would keep the options for start_session all Hound specific and nest the driver options under the :driver key.

This does seem cleaner, thank you for the feedback!
@HashNuke What do you think?

@tuvistavie once this is all said and done on the hound side, could you also please investigate integration from the Phoenix.Ecto side too? Thank you!

Sure, as soon as I have the time!

@HashNuke
Copy link
Owner

@tuvistavie I vote for the driver options being under a key too. That way Hound options are easily identifiable.

@danhper
Copy link
Collaborator

danhper commented Apr 19, 2016

I think we are done here 🎉

@danhper danhper closed this as completed Apr 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants