add check for chromedriver chrome version match (#688)

* Fixes #685
When google chrome and chromedriver versions are out of sync wallaby can start in a bad state.
We added a version comparison step to `Wallaby.Chrome.validate/0` that will error
during application start if chrome and chrome versions do not match to a major, minor, and build version.

Co-authored-by: Alex McLain <[email protected]>
Co-authored-by: Johnny Otsuka <[email protected]>
Co-authored-by: John Cotton <[email protected]>
Co-authored-by: Liam Elder <[email protected]>

* Use correct config option in DependencyError

To configure the Chrome binary, you use the `:chromdriver` option and
set it to `[binary: "/path/to/chrome"]`

* Emit a warning instead of raising an error

Sometimes having a mismatched chromedriver/chrome version still works,
and having this error might inconvenience the developer. Emitting a
warning I think is sufficient to alert the developer as to a potential
problem.

This also fixes some incorrect config value. The path to an alternative
chrome binary is found is found with `Application.get_env(:wallaby,
:chromedriver)[:binary]`, not
`Application.get_env(:wallaby, :chrome)[:path]`.

Co-authored-by: Matt Hall <[email protected]>
Co-authored-by: Alex McLain <[email protected]>
Co-authored-by: Johnny Otsuka <[email protected]>
Co-authored-by: John Cotton <[email protected]>
Co-authored-by: Liam Elder <[email protected]>

Author: 6 people

integration_test/cases/browser/assert_refute_has_test.exs

@@ -35,13 +35,14 @@ defmodule Wallaby.Integration.Browser.AssertRefuteHasTest do
     end
 
     test "mentions the count of found vs. expected index", %{session: session} do
-      assert_raise ExpectationNotMetError,
-                   ~r/Expected.*and return element at index 7 but only 6 visible elements were found/i,
-                   fn ->
-                     session
-                     |> visit("nesting.html")
-                     |> assert_has(@wrong_at_query)
-                   end
+      expect =
+        ~r/Expected.*and return element at index 7, but only 6 visible elements were found/i
+
+      assert_raise ExpectationNotMetError, expect, fn ->
+        session
+        |> visit("nesting.html")
+        |> assert_has(@wrong_at_query)
+      end
     end
 
     test "passes if `at` element exists", %{session: session} do

integration_test/cases/browser/window_position_test.exs

@@ -1,13 +1,17 @@
 defmodule Wallaby.Integration.Browser.WindowPositionTest do
   use Wallaby.Integration.SessionCase, async: true
 
-  test "getting the window position", %{session: session} do
-    window_position =
-      session
-      |> visit("/")
-      |> move_window(100, 200)
-      |> window_position()
+  # this test dows not return the right values on mac
+  # reason is unclear, I think it's a bug in chromedriver on mac
+  if :os.type() != {:unix, :darwin} do
+    test "getting the window position", %{session: session} do
+      window_position =
+        session
+        |> visit("/")
+        |> move_window(100, 200)
+        |> window_position()
 
-    assert %{"x" => 100, "y" => 200} = window_position
+      assert %{"x" => 100, "y" => 200} = window_position
+    end
   end
 end

integration_test/chrome/starting_sessions_test.exs

@@ -1,6 +1,8 @@
 defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
   use ExUnit.Case, async: false
 
+  import ExUnit.CaptureIO
+
   import Wallaby.SettingsTestHelpers
   import Wallaby.TestSupport.ApplicationControl
   import Wallaby.TestSupport.TestScriptUtils
@@ -23,7 +25,7 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
 
     test_script_path =
       chromedriver_path
-      |> ChromeTestScript.build_wrapper_script()
+      |> ChromeTestScript.build_chromedriver_wrapper_script()
       |> write_test_script!(workspace_path)
 
     ensure_setting_is_reset(:wallaby, :chromedriver)
@@ -41,7 +43,7 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
 
     test_script_path =
       chromedriver_path
-      |> ChromeTestScript.build_wrapper_script()
+      |> ChromeTestScript.build_chromedriver_wrapper_script()
       |> write_test_script!(workspace_path)
 
     ensure_setting_is_reset(:wallaby, :chromedriver)
@@ -96,7 +98,7 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
     workspace_path: workspace_path
   } do
     test_script_path =
-      ChromeTestScript.build_version_mock_script(version: "2.29")
+      ChromeTestScript.build_chromedriver_version_mock_script(version: "2.29")
       |> write_test_script!(workspace_path)
 
     ensure_setting_is_reset(:wallaby, :chromedriver)
@@ -105,17 +107,48 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
     assert {:error, _} = Application.start(:wallaby)
   end
 
-  test "application starts when chromedriver version >= 2.30", %{
+  test "application starts when chromedriver version >= 2.30", %{workspace_path: workspace_path} do
+    chromedriver_test_script_path =
+      ChromeTestScript.build_chromedriver_version_mock_script(version: "2.30")
+      |> write_test_script!(workspace_path)
+
+    chrome_test_script_path =
+      ChromeTestScript.build_chrome_version_mock_script(version: "2.30")
+      |> write_test_script!(workspace_path)
+
+    ensure_setting_is_reset(:wallaby, :chromedriver)
+    Application.put_env(:wallaby, :chromedriver, path: chromedriver_test_script_path)
+    Application.put_env(:wallaby, :chromedriver, binary: chrome_test_script_path)
+
+    log =
+      capture_io(:stderr, fn ->
+        assert :ok == Application.start(:wallaby)
+      end)
+
+    assert log =~ "Looks like you're trying to run Wallaby with a mismatched version of Chrome"
+  end
+
+  test "application does not start when chrome version != chromedriver version", %{
     workspace_path: workspace_path
   } do
-    test_script_path =
-      ChromeTestScript.build_version_mock_script(version: "2.30")
+    chromedriver_test_script_path =
+      ChromeTestScript.build_chromedriver_version_mock_script(version: "99.0.3945.36")
+      |> write_test_script!(workspace_path)
+
+    chrome_test_script_path =
+      ChromeTestScript.build_chrome_version_mock_script(version: "101.0.3945.36")
       |> write_test_script!(workspace_path)
 
     ensure_setting_is_reset(:wallaby, :chromedriver)
-    Application.put_env(:wallaby, :chromedriver, path: test_script_path)
+    Application.put_env(:wallaby, :chromedriver, path: chromedriver_test_script_path)
+    Application.put_env(:wallaby, :chromedriver, binary: chrome_test_script_path)
 
-    assert :ok = Application.start(:wallaby)
+    log =
+      capture_io(:stderr, fn ->
+        assert :ok == Application.start(:wallaby)
+      end)
+
+    assert log =~ "Looks like you're trying to run Wallaby with a mismatched version of Chrome"
   end
 
   test "works with a path in the home directory" do
@@ -137,11 +170,7 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
   test "fails to start when chromedriver path is configured incorrectly" do
     ensure_setting_is_reset(:wallaby, :chromedriver)
 
-    Application.put_env(
-      :wallaby,
-      :chromedriver,
-      path: "this-really-should-not-exist"
-    )
+    Application.put_env(:wallaby, :chromedriver, path: "this-really-should-not-exist")
 
     assert {:error, _} = Application.start(:wallaby)
   end
@@ -150,7 +179,7 @@ defmodule Wallaby.Integration.Chrome.StartingSessionsTest do
     {:ok, chromedriver_path} = Chrome.find_chromedriver_executable()
 
     chromedriver_path
-    |> ChromeTestScript.build_wrapper_script(opts)
+    |> ChromeTestScript.build_chromedriver_wrapper_script(opts)
     |> write_test_script!(base_dir)
   end
 end

lib/wallaby/chrome.ex

@@ -6,7 +6,7 @@ defmodule Wallaby.Chrome do
 
   Start a Wallaby Session using this driver with the following command:
 
-  ```
+  ```elixir
   {:ok, session} = Wallaby.start_session()
   ```
 
@@ -20,7 +20,7 @@ defmodule Wallaby.Chrome do
   This will override the default capabilities and capabilities set with application configuration.
   This will _not_ override capabilities passed in directly to `Wallaby.start_session/1`.
 
-  ```
+  ```elixir
   config :wallaby,
     chromedriver: [
       headless: false
@@ -31,7 +31,7 @@ defmodule Wallaby.Chrome do
 
   These capabilities will override the default capabilities.
 
-  ```
+  ```elixir
   config :wallaby,
     chromedriver: [
       capabilities: %{
@@ -44,7 +44,7 @@ defmodule Wallaby.Chrome do
 
   If ChromeDriver is not available in your path, you can specify it's location.
 
-  ```
+  ```elixir
   config :wallaby,
     chromedriver: [
       path: "path/to/chrome"
@@ -58,7 +58,7 @@ defmodule Wallaby.Chrome do
   This will override the default capabilities and capabilities set with application configuration.
   This will _not_ override capabilities passed in directly to `Wallaby.start_session/1`.
 
-  ```
+  ```elixir
   config :wallaby,
     chromedriver: [
       binary: "path/to/chrome"
@@ -107,7 +107,6 @@ defmodule Wallaby.Chrome do
   @behaviour Wallaby.Driver
 
   @default_readiness_timeout 5_000
-  @chromedriver_version_regex ~r/^ChromeDriver (\d+)\.(\d+)/
 
   alias Wallaby.Chrome.Chromedriver
   alias Wallaby.WebdriverClient
@@ -154,14 +153,91 @@ defmodule Wallaby.Chrome do
   @doc false
   @spec validate() :: :ok | {:error, DependencyError.t()}
   def validate do
-    with {:ok, executable} <- find_chromedriver_executable() do
-      {version, 0} = System.cmd(executable, ["--version"])
-
-      @chromedriver_version_regex
-      |> Regex.run(version)
-      |> Enum.drop(1)
-      |> Enum.map(&String.to_integer/1)
-      |> version_check()
+    with {:ok, chromedriver_version} <- get_chromedriver_version(),
+         {:ok, chrome_version} <- get_chrome_version(),
+         :ok <- minimum_version_check(chromedriver_version) do
+      version_compare(chrome_version, chromedriver_version)
+    end
+  end
+
+  @doc false
+  @spec get_chrome_version() :: {:ok, list(String.t())} | {:error, term}
+  def get_chrome_version do
+    case :os.type() do
+      {:win32, :nt} ->
+        {stdout, 0} =
+          System.cmd("reg", [
+            "query",
+            "HKLM\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Google Chrome"
+          ])
+
+        chrome_version = parse_version("Version", stdout)
+
+        {:ok, chrome_version}
+
+      _ ->
+        case find_chrome_executable() do
+          {:ok, chrome_executable} ->
+            {stdout, 0} = System.cmd(chrome_executable, ["--version"])
+            chrome_version = parse_version("Google Chrome", stdout)
+
+            {:ok, chrome_version}
+
+          error ->
+            error
+        end
+    end
+  end
+
+  @doc false
+  @spec get_chromedriver_version() :: {:ok, list(String.t())} | {:error, term}
+  def get_chromedriver_version do
+    case find_chromedriver_executable() do
+      {:ok, chromedriver_executable} ->
+        {stdout, 0} = System.cmd(chromedriver_executable, ["--version"])
+        chromedriver_version = parse_version("ChromeDriver", stdout)
+
+        {:ok, chromedriver_version}
+
+      error ->
+        error
+    end
+  end
+
+  @doc false
+  @spec find_chrome_executable :: {:ok, String.t()} | {:error, DependencyError.t()}
+  def find_chrome_executable do
+    default_chrome_path =
+      case :os.type() do
+        {:unix, :darwin} ->
+          "/Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome"
+
+        {:unix, :linux} ->
+          "google-chrome"
+
+        {:win32, :nt} ->
+          "C:\\Program Files\\Google\\Chrome\\Application\\chrome.exe"
+      end
+
+    chrome_path =
+      :wallaby
+      |> Application.get_env(:chromedriver, [])
+      |> Keyword.get(:binary, default_chrome_path)
+
+    [Path.expand(chrome_path), default_chrome_path]
+    |> Enum.find(&System.find_executable/1)
+    |> case do
+      path when is_binary(path) ->
+        {:ok, path}
+
+      nil ->
+        exception =
+          DependencyError.exception("""
+          Wallaby can't find Chrome. Make sure you have chrome installed and included in your path.
+          You can also provide a path using `config :wallaby, :chromedriver, binary: <path>`.
+          """)
+
+        {:error, exception}
     end
   end
 
@@ -182,25 +258,48 @@ defmodule Wallaby.Chrome do
       nil ->
         exception =
           DependencyError.exception("""
-          Wallaby can't find chromedriver. Make sure you have chromedriver installed
-          and included in your path.
+          Wallaby can't find chromedriver. Make sure you have chromedriver installed and included in your path.
           You can also provide a path using `config :wallaby, :chromedriver, path: <path>`.
           """)
 
         {:error, exception}
     end
   end
 
-  defp version_check([major_version, _minor_version]) when major_version > 2 do
+  defp version_compare(chrome_version, chromedriver_version) do
+    case chrome_version == chromedriver_version do
+      true ->
+        :ok
+
+      _ ->
+        exception =
+          DependencyError.exception("""
+          Looks like you're trying to run Wallaby with a mismatched version of Chrome: #{Enum.join(chrome_version, ".")} and chromedriver: #{Enum.join(chromedriver_version, ".")}.
+          Chrome and chromedriver must match to a major, minor, and build version.
+          """)
+
+        IO.warn(exception.message)
+
+        :ok
+    end
+  end
+
+  defp minimum_version_check([major_version, _minor_version, _build_version])
+       when major_version > 2 do
+    :ok
+  end
+
+  defp minimum_version_check([major_version, minor_version, _build_version])
+       when major_version == 2 and minor_version >= 30 do
     :ok
   end
 
-  defp version_check([major_version, minor_version])
+  defp minimum_version_check([major_version, minor_version])
        when major_version == 2 and minor_version >= 30 do
     :ok
   end
 
-  defp version_check(_version) do
+  defp minimum_version_check(_version) do
     exception =
       DependencyError.exception("""
       Looks like you're trying to run an older version of chromedriver. Wallaby needs at least
@@ -210,6 +309,19 @@ defmodule Wallaby.Chrome do
     {:error, exception}
   end
 
+  defp parse_version(prefix, body) do
+    result =
+      case Regex.run(~r/\b#{prefix}\b.*?(\d+\.\d+(\.\d+)?)/, body) do
+        [_, version, _] ->
+          String.split(version, ".")
+
+        [_, version] ->
+          String.split(version, ".")
+      end
+
+    Enum.map(result, &String.to_integer/1)
+  end
+
   @doc false
   @spec start_session([start_session_opts]) :: Wallaby.Driver.on_start_session() | no_return
   def start_session(opts \\ []) do