Add custom host IP address support - Issue #716 #879

arturocastro · 2017-04-02T03:02:23Z

Travis CI test uses a custom host, in local environments it can be tested with the POLTERGEIST_TEST_HOST env var.

arturocastro · 2017-04-02T03:55:37Z

It's my understanding those two failed tests happen as result of PhantomJS version? They seem to occur even before applying my changes from a stable version.

Failures:
  1) Capybara::Poltergeist::Client forcibly kills the child if it does not respond to SIGTERM
     Failure/Error: parts << server.host
       #<Double (anonymous)> received unexpected message :host with (no args)
       Debug info:
     # ./lib/capybara/poltergeist/client.rb:95:in `command'
     # ./lib/capybara/poltergeist/client.rb:70:in `block in start'
     # ./lib/capybara/poltergeist/client.rb:108:in `redirect_stdout'
     # ./lib/capybara/poltergeist/client.rb:69:in `start'
     # ./spec/unit/client_spec.rb:57:in `block (2 levels) in <module:Poltergeist>'
  2) Capybara::Poltergeist::Driver with a :timeout option starts the server with the provided timeout
     Failure/Error: @server ||= Server.new(options[:port], options.fetch(:timeout) { DEFAULT_TIMEOUT }, options[:host])
       #<Capybara::Poltergeist::Server (class)> received :new with unexpected arguments
         expected: (anything, 3)
              got: (nil, 3, nil)
       Debug info:
     # ./lib/capybara/poltergeist/driver.rb:40:in `server'
     # ./spec/unit/driver_spec.rb:116:in `block (3 levels) in <module:Poltergeist>'

twalpole · 2017-04-02T04:29:56Z

README.md

@@ -281,6 +281,7 @@ end
 *   `:extensions` (Array) - An array of JS files to be preloaded into
    the phantomjs browser. Useful for faking unsupported APIs.
 *   `:port` (Fixnum) - The port which should be used to communicate
+*   `:host` (String) - The name or IP of the PhantomJS host. Default is '127.0.0.1'.


This needs to move down a line since it split the comment from the :port line

twalpole · 2017-04-02T04:30:31Z

lib/capybara/poltergeist/client/connection.coffee

@@ -1,5 +1,8 @@
 class Poltergeist.Connection
-  constructor: (@owner, @port) ->
+  constructor: (@owner, @port, @host) ->


Just add a default value for @host in the function definition

twalpole · 2017-04-02T04:30:56Z

lib/capybara/poltergeist/client/connection.coffee

-  constructor: (@owner, @port) ->
+  constructor: (@owner, @port, @host) ->
+    #@socket = new WebSocket "ws://#{@host}:#{@port}/"
+    if !@host?


not needed if default value added in function definition

twalpole · 2017-04-02T04:31:25Z

lib/capybara/poltergeist/client/connection.coffee

+  constructor: (@owner, @port, @host) ->
+    #@socket = new WebSocket "ws://#{@host}:#{@port}/"
+    if !@host?
+        @host = "127.0.0.1"
    @socket = new WebSocket "ws://127.0.0.1:#{@port}/"


I'm guessing this needs to be updated to use the @host value?

twalpole · 2017-04-02T04:34:42Z

spec/integration/driver_spec.rb

+        # Use /etc/hosts or iptables for this: https://superuser.com/questions/516208/how-to-change-ip-address-to-point-to-localhost
+        # A custom host and corresponding env var for Travis is specified in .travis.yml
+        # If var is unspecified, use localhost
+        env_var = ENV['POLTERGEISt_TEST_HOST']


Capitalization

twalpole · 2017-04-02T04:40:19Z

The test failures have nothing to do with PhantomJS version, and everything to do with the changes to the Server API and the doubles not being updated to match.

twalpole · 2017-04-02T04:42:06Z

spec/integration/driver_spec.rb

@@ -860,6 +860,23 @@ def create_screenshot(file, *args)
      end
    end

+    it 'allows the driver to have a custom host' do


Skip this test if ENV['POLTERGEIST_TEST_HOST'] isn't set

use the RSpec provided skip to skip the test so a notice gets printed that a test was skipped

arturocastro · 2017-04-02T07:01:40Z

My bad, I messed my git history. Here it goes again.

twalpole · 2017-04-02T07:44:32Z

lib/capybara/poltergeist/web_socket_server.rb

      time = Time.now

      begin
-        TCPServer.open(HOST, port || 0).tap do |server|
+        custom_host = custom_host.nil? ? custom_host : HOST


This logic looks wrong, since it will only assign custom_host if it's nil, looks like you could just do TCPServer.open(custom_host || HOST, port || 0)

twalpole · 2017-04-02T07:49:04Z

CHANGELOG.md

@@ -1,3 +1,6 @@
+### 1.15.0 ###
+*   Driver now supports specifying a custom IP address (host) with the :host option (Arturo Castro)


Please add the Features section like all other version numbers have

arturocastro · 2017-04-14T03:23:40Z

I've incorporated the suggestions to the branch

twalpole · 2017-04-15T17:10:47Z

@arturocastro Thanks, once you fix the tests this breaks then it should be good to go.

arturocastro · 2017-04-15T21:45:07Z

Fixed the tests!

twalpole · 2017-04-17T16:33:50Z

Thanks!

twalpole suggested changes Apr 2, 2017

View reviewed changes

twalpole reviewed Apr 2, 2017

View reviewed changes

arturocastro force-pushed the custom_host branch from c9ac701 to 2acbc7c Compare April 2, 2017 06:59

twalpole reviewed Apr 2, 2017

View reviewed changes

arturocastro force-pushed the custom_host branch from 2acbc7c to 7fd258e Compare April 14, 2017 02:58

twalpole approved these changes Apr 15, 2017

View reviewed changes

Add custom host IP address support - Issue teampoltergeist#716

97be0eb

arturocastro force-pushed the custom_host branch from 7fd258e to 97be0eb Compare April 15, 2017 21:27

twalpole merged commit e0513a7 into teampoltergeist:master Apr 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom host IP address support - Issue #716 #879

Add custom host IP address support - Issue #716 #879

arturocastro commented Apr 2, 2017

arturocastro commented Apr 2, 2017

twalpole Apr 2, 2017

twalpole Apr 2, 2017

twalpole Apr 2, 2017

twalpole Apr 2, 2017

twalpole Apr 2, 2017

twalpole commented Apr 2, 2017

twalpole Apr 2, 2017

twalpole Apr 2, 2017

arturocastro commented Apr 2, 2017

twalpole Apr 2, 2017 •

edited

twalpole Apr 2, 2017

arturocastro commented Apr 14, 2017

twalpole commented Apr 15, 2017

arturocastro commented Apr 15, 2017

twalpole commented Apr 17, 2017

		@@ -1,3 +1,6 @@
		### 1.15.0 ###
		* Driver now supports specifying a custom IP address (host) with the :host option (Arturo Castro)

Add custom host IP address support - Issue #716 #879

Add custom host IP address support - Issue #716 #879

Conversation

arturocastro commented Apr 2, 2017

arturocastro commented Apr 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twalpole commented Apr 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturocastro commented Apr 2, 2017

twalpole Apr 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturocastro commented Apr 14, 2017

twalpole commented Apr 15, 2017

arturocastro commented Apr 15, 2017

twalpole commented Apr 17, 2017

twalpole Apr 2, 2017 •

edited