From 2ceab4e525f2326d5058ebb268cdfa82329ac132 Mon Sep 17 00:00:00 2001 From: Matthew Horan Date: Thu, 22 Nov 2012 10:30:43 -0500 Subject: [PATCH] Forward stderr via Open3#popen3 Forwarding stderr via 2>&1 causes an additional child process to be spawned. Killing that process does not kill webkit_server. JavaScipt console messages and alerts are now written to the logger instead of directly to stdout. --- lib/capybara/webkit/connection.rb | 47 +++++++++---------------------- spec/connection_spec.rb | 4 ++- spec/driver_spec.rb | 3 +- spec/spec_helper.rb | 2 +- src/WebPage.cpp | 4 +-- 5 files changed, 20 insertions(+), 40 deletions(-) diff --git a/lib/capybara/webkit/connection.rb b/lib/capybara/webkit/connection.rb index c6a80a0..7f125ab 100644 --- a/lib/capybara/webkit/connection.rb +++ b/lib/capybara/webkit/connection.rb @@ -1,6 +1,7 @@ require 'socket' require 'timeout' require 'thread' +require 'open3' module Capybara::Webkit class Connection @@ -11,8 +12,7 @@ module Capybara::Webkit def initialize(options = {}) @socket_class = options[:socket_class] || TCPSocket - @stdout = options.has_key?(:stdout) ? options[:stdout] : $stdout - @command = options[:command] || SERVER_PATH + @output_target = options.has_key?(:stdout) ? options[:stdout] : $stdout start_server connect end @@ -38,12 +38,12 @@ module Capybara::Webkit def start_server open_pipe discover_port - forward_stdout_in_background_thread + forward_output_in_background_thread end def open_pipe - @pipe = IO.popen(@command) - @pid = @pipe.pid + _, @pipe_stdout, @pipe_stderr, wait_thr = Open3.popen3(SERVER_PATH) + @pid = wait_thr[:pid] register_shutdown_hook end @@ -65,40 +65,19 @@ module Capybara::Webkit end def discover_port - if IO.select([@pipe], nil, nil, WEBKIT_SERVER_START_TIMEOUT) - @port = ((@pipe.first || '').match(/listening on port: (\d+)/) || [])[1].to_i + if IO.select([@pipe_stdout], nil, nil, WEBKIT_SERVER_START_TIMEOUT) + @port = ((@pipe_stdout.first || '').match(/listening on port: (\d+)/) || [])[1].to_i end end - def forward_stdout_in_background_thread - @stdout_thread = Thread.new do + def forward_output_in_background_thread + Thread.new do Thread.current.abort_on_exception = true - forward_stdout + IO.copy_stream(@pipe_stdout, @output_target) end - end - - def forward_stdout - while pipe_readable? - line = @pipe.readline - if @stdout - @stdout.write(line) - @stdout.flush - end - end - rescue EOFError - end - - if !defined?(RUBY_ENGINE) || (RUBY_ENGINE == "ruby" && RUBY_VERSION <= "1.8") - # please note the use of IO::select() here, as it is used specifically to - # preserve correct signal handling behavior in ruby 1.8. - # https://github.com/thibaudgg/rb-fsevent/commit/d1a868bf8dc72dbca102bedbadff76c7e6c2dc21 - # https://github.com/thibaudgg/rb-fsevent/blob/1ca42b987596f350ee7b19d8f8210b7b6ae8766b/ext/fsevent/fsevent_watch.c#L171 - def pipe_readable? - IO.select([@pipe]) - end - else - def pipe_readable? - !@pipe.eof? + Thread.new do + Thread.current.abort_on_exception = true + IO.copy_stream(@pipe_stderr, @output_target) end end diff --git a/spec/connection_spec.rb b/spec/connection_spec.rb index 0e2aa12..11ff67a 100644 --- a/spec/connection_spec.rb +++ b/spec/connection_spec.rb @@ -22,12 +22,14 @@ describe Capybara::Webkit::Connection do io = StringIO.new redirected_connection = Capybara::Webkit::Connection.new(:stdout => io) script = 'console.log("hello world")' + redirected_connection.puts "EnableLogging" + redirected_connection.puts 0 redirected_connection.puts "Execute" redirected_connection.puts 1 redirected_connection.puts script.to_s.bytesize redirected_connection.print script sleep(0.5) - io.string.should include "hello world\n" + io.string.should include "hello world \n" end it "returns the server port" do diff --git a/spec/driver_spec.rb b/spec/driver_spec.rb index d86c679..f6531da 100644 --- a/spec/driver_spec.rb +++ b/spec/driver_spec.rb @@ -2042,8 +2042,7 @@ describe Capybara::Webkit::Driver do end let(:driver) do - command = "#{Capybara::Webkit::Connection::SERVER_PATH} 2>&1" - connection = Capybara::Webkit::Connection.new(:command => command, :stdout => output) + connection = Capybara::Webkit::Connection.new(:stdout => output) browser = Capybara::Webkit::Browser.new(connection) Capybara::Webkit::Driver.new(AppRunner.app, :browser => browser) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d571067..9299cf2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -22,7 +22,7 @@ RSpec.configure do |c| end require 'capybara/webkit' -connection = Capybara::Webkit::Connection.new(:socket_class => TCPSocket, :stdout => nil) +connection = Capybara::Webkit::Connection.new(:socket_class => TCPSocket) $webkit_browser = Capybara::Webkit::Browser.new(connection) if ENV['DEBUG'] diff --git a/src/WebPage.cpp b/src/WebPage.cpp index efbbe55..cf7b55d 100644 --- a/src/WebPage.cpp +++ b/src/WebPage.cpp @@ -139,13 +139,13 @@ void WebPage::javaScriptConsoleMessage(const QString &message, int lineNumber, c if (!sourceID.isEmpty()) fullMessage = sourceID + "|" + QString::number(lineNumber) + "|" + fullMessage; m_consoleMessages.append(fullMessage.replace("\n", "\\n")); - std::cout << qPrintable(fullMessage) << std::endl; + m_manager->logger() << qPrintable(fullMessage); } void WebPage::javaScriptAlert(QWebFrame *frame, const QString &message) { Q_UNUSED(frame); m_alertMessages.append(message); - std::cout << "ALERT: " << qPrintable(message) << std::endl; + m_manager->logger() << "ALERT:" << qPrintable(message); } bool WebPage::javaScriptConfirm(QWebFrame *frame, const QString &message) {