Get the port from the server

Ask, don't tell, or something like that!

Now the WebSocketServer decides what port it will use and everything
else relies on that. By default an ephemeral port is used, but a fixed
port number can be passed in too.

This work is necessary to support Windows (#240) because on Windows a
TCP socket cannot be closed and immediately reopened, which we are doing
in the tests. So this change means that when restarting, a new ephemeral
port can be used by the server.

lib/capybara/poltergeist.rb

@@ -17,7 +17,6 @@ module Capybara
     require 'capybara/poltergeist/network_traffic'
     require 'capybara/poltergeist/errors'
     require 'capybara/poltergeist/cookie'
-    require 'capybara/poltergeist/util'
   end
 end
 

lib/capybara/poltergeist/client.rb

@@ -14,10 +14,10 @@ module Capybara::Poltergeist
       client
     end
 
-    attr_reader :pid, :port, :path, :window_size, :phantomjs_options
+    attr_reader :pid, :server, :path, :window_size, :phantomjs_options
 
-    def initialize(port, options = {})
-      @port              = port
+    def initialize(server, options = {})
+      @server            = server
       @path              = options[:path]              || PHANTOMJS_NAME
       @window_size       = options[:window_size]       || [1024, 768]
       @phantomjs_options = options[:phantomjs_options] || []
@@ -57,14 +57,12 @@ module Capybara::Poltergeist
     end
 
     def command
-      @command ||= begin
-        parts = [path]
-        parts.concat phantomjs_options
-        parts << PHANTOMJS_SCRIPT
-        parts << port
-        parts.concat window_size
-        parts
-      end
+      parts = [path]
+      parts.concat phantomjs_options
+      parts << PHANTOMJS_SCRIPT
+      parts << server.port
+      parts.concat window_size
+      parts
     end
 
     private

lib/capybara/poltergeist/driver.rb

@@ -34,14 +34,11 @@ module Capybara::Poltergeist
     end
 
     def server
-      @server ||= Server.new(
-        options.fetch(:port)    { Util.find_available_port },
-        options.fetch(:timeout) { DEFAULT_TIMEOUT          }
-      )
+      @server ||= Server.new(options[:port], options.fetch(:timeout) { DEFAULT_TIMEOUT })
     end
 
     def client
-      @client ||= Client.start(server.port,
+      @client ||= Client.start(server,
         :path              => options[:phantomjs],
         :window_size       => options[:window_size],
         :phantomjs_options => phantomjs_options,

lib/capybara/poltergeist/server.rb

@@ -1,19 +1,23 @@
 module Capybara::Poltergeist
   class Server
-    attr_reader :port, :socket, :timeout
+    attr_reader :socket, :fixed_port, :timeout
 
-    def initialize(port, timeout = nil)
-      @port    = port
-      @timeout = timeout
+    def initialize(port = nil, timeout = nil)
+      @fixed_port = fixed_port
+      @timeout    = timeout
       start
     end
 
+    def port
+      @socket.port
+    end
+
     def timeout=(sec)
       @timeout = @socket.timeout = sec
     end
 
     def start
-      @socket = WebSocketServer.new(port, timeout)
+      @socket = WebSocketServer.new(fixed_port, timeout)
     end
 
     def stop

lib/capybara/poltergeist/util.rb

@@ -1,12 +0,0 @@
-require 'socket'
-
-module Capybara::Poltergeist
-  module Util
-    def self.find_available_port
-      server = TCPServer.new('127.0.0.1', 0)
-      server.addr[1]
-    ensure
-      server.close if server
-    end
-  end
-end

lib/capybara/poltergeist/web_socket_server.rb

@@ -57,21 +57,26 @@ module Capybara::Poltergeist
     # How many seconds to try to bind to the port for before failing
     BIND_TIMEOUT = 5
 
+    HOST = '127.0.0.1'
+
     attr_reader :port, :parser, :socket, :handler, :server
     attr_accessor :timeout
 
-    def initialize(port, timeout = nil)
-      @port    = port
-      @parser  = Http::Parser.new
-      @server  = start_server
+    def initialize(port = nil, timeout = nil)
       @timeout = timeout
+      @parser  = Http::Parser.new
+      @server  = start_server(port)
     end
 
-    def start_server
+    def port
+      server.addr[1]
+    end
+
+    def start_server(port)
       time = Time.now
 
       begin
-        TCPServer.open(port)
+        TCPServer.open(HOST, port || 0)
       rescue Errno::EADDRINUSE
         if (Time.now - time) < BIND_TIMEOUT
           sleep(0.01)

spec/integration/driver_spec.rb

@@ -54,7 +54,7 @@ module Capybara::Poltergeist
       end
     end
 
-    it 'raises an error and restart the client, if the client dies while executing a command' do
+    it 'raises an error and restarts the client, if the client dies while executing a command' do
       lambda { @driver.browser.command('exit') }.should raise_error(DeadClient)
       @session.visit('/')
       @driver.html.should include('Hello world')

spec/unit/client_spec.rb

@@ -2,7 +2,9 @@ require 'spec_helper'
 
 module Capybara::Poltergeist
   describe Client do
-    subject { Client.new(6000) }
+    let(:server) { stub(port: 6000) }
+
+    subject { Client.new(server) }
 
     it 'raises an error if phantomjs is too old' do
       `true` # stubbing $?
@@ -21,7 +23,7 @@ module Capybara::Poltergeist
     end
 
     it 'raises an error if phantomjs returns a non-zero exit code' do
-      subject = Client.new(6000, :path => 'exit 42 && ')
+      subject = Client.new(server, :path => 'exit 42 && ')
       expect { subject.start }.to raise_error(Error)
 
       begin
@@ -32,16 +34,16 @@ module Capybara::Poltergeist
     end
 
     context 'with width and height specified' do
-      subject { Client.new(6000, :window_size => [800, 600]) }
+      subject { Client.new(server, :window_size => [800, 600]) }
 
       it 'starts phantomjs, passing the width and height through' do
         Process.should_receive(:spawn).with("phantomjs", Client::PHANTOMJS_SCRIPT, "6000", "800", "600", out: $stdout)
         subject.start
       end
     end
-    
+
     context 'with a custom phantomjs_logger' do
-      subject { Client.new(6000, :phantomjs_logger => :my_custom_logger, :window_size => [800, 600]) }
+      subject { Client.new(server, :phantomjs_logger => :my_custom_logger, :window_size => [800, 600]) }
 
       it 'starts phantomjs, capturing the STDOUT to custom phantomjs_logger' do
         Process.should_receive(:spawn).with("phantomjs", Client::PHANTOMJS_SCRIPT, "6000", "800", "600", out: :my_custom_logger)
@@ -50,7 +52,7 @@ module Capybara::Poltergeist
     end
 
     context 'with additional command-line options' do
-      subject { Client.new(6000, :phantomjs_options => %w[--ignore-ssl-error=yes --load-images=no]) }
+      subject { Client.new(server, :phantomjs_options => %w[--ignore-ssl-error=yes --load-images=no]) }
 
       it 'passed additional command-line options to phantomjs' do
         Process.should_receive(:spawn).with("phantomjs", '--ignore-ssl-error=yes', '--load-images=no', anything, anything, anything, anything, out: $stdout)
@@ -64,7 +66,7 @@ module Capybara::Poltergeist
           alias old_wait wait
         end
 
-        client = Client.new(1234)
+        client = Client.new(server)
         Process.stub(spawn: 5678)
         client.start