rewrite BrowserStackLocal spawning to be better

Gemfile

@@ -1,4 +1,7 @@
-source "http://rubygems.org"
+source "https://rubygems.org"
 gem "minitest"
 gem "rake"
 gem "json"
+gemspec
+
+gem "rspec-core", "~> 3.13"

Gemfile.lock

@@ -1,17 +1,30 @@
+PATH
+  remote: .
+  specs:
+    browserstack-local (1.4.3)
+      subprocess (~> 1.5)
+
 GEM
-  remote: http://rubygems.org/
+  remote: https://rubygems.org/
   specs:
-    json (1.8.3)
-    minitest (5.8.4)
-    rake (12.3.3)
+    json (2.7.1)
+    minitest (5.22.2)
+    rake (13.1.0)
+    rspec-core (3.13.0)
+      rspec-support (~> 3.13.0)
+    rspec-support (3.13.0)
+    subprocess (1.5.6)
 
 PLATFORMS
+  arm64-darwin-23
   ruby
 
 DEPENDENCIES
+  browserstack-local!
   json
   minitest
   rake
+  rspec-core (~> 3.13)
 
 BUNDLED WITH
-   1.11.2
+   2.5.5

browserstack-local.gemspec

@@ -7,8 +7,8 @@ Gem::Specification.new do |s|
   s.authors     = ["BrowserStack"]
   s.email       = '[email protected]'
   s.files       = ["lib/browserstack/local.rb", "lib/browserstack/localbinary.rb", "lib/browserstack/localexception.rb"]
-  s.homepage    =
-    'http://rubygems.org/gems/browserstack-local'
-  s.license       = 'MIT'
+  s.homepage    = 'http://rubygems.org/gems/browserstack-local'
+  s.license     = 'MIT'
+  s.add_dependency "subprocess", "~> 1.5"
 end
 

lib/browserstack/local.rb

@@ -1,18 +1,19 @@
 require 'browserstack/localbinary'
 require 'browserstack/localexception'
 require 'json'
+require 'subprocess'
 
 module BrowserStack
 
 class Local
   attr_reader :pid
 
-  def initialize(key = ENV["BROWSERSTACK_ACCESS_KEY"])
+  def initialize(key = ENV["BROWSERSTACK_ACCESS_KEY"], startup_timeout = 120)
     @key = key
     @user_arguments = []
     @logfile = File.join(Dir.pwd, "local.log")
     @is_windows = RbConfig::CONFIG['host_os'].match(/mswin|msys|mingw|cygwin|bccwin|wince|emc|win32/)
-    @exec = @is_windows ? "call" : "exec";
+    @startup_timeout = startup_timeout
   end
 
   def add_args(key, value=nil)
@@ -68,35 +69,36 @@ def start(options = {})
       else
         @binary_path
       end
-
-    if @is_windows
-      system("echo > #{@logfile}")
-    else
-      system("echo '' > '#{@logfile}'")
-    end
 
-    if defined? spawn
-      @process = IO.popen(start_command_args)
-    else
-      @process = IO.popen(start_command)
+    # ensure the logfile exists
+    File.open(@logfile, "a") do
     end
 
-    while true
+    @process = Subprocess::Process.new(start_command_args, stdout: Subprocess::PIPE, stderr: Subprocess::STDOUT)
+    deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + @startup_timeout
+
+    while Process.clock_gettime(Process::CLOCK_MONOTONIC) < deadline
       begin
-        line = @process.readline
+        remaining = [Process.clock_gettime(Process::CLOCK_MONOTONIC) - deadline, 1].max
+        if IO.select([@process.stdout], [], [@process.stdout], remaining).nil?
+          sleep 1
+          next
+        end
+        line = @process.stdout.readline
       rescue EOFError => e
         sleep 1
         next
       end
 
       data = JSON.parse(line) rescue {"message" => "Unable to parse daemon mode JSON output"}
       if data['state'].to_s != "connected"
-        @process.close
         raise BrowserStack::LocalException.new(data["message"]["message"])
         return
       else
         @pid = data["pid"]
-        break
+        @process.wait
+        @process = nil
+        return
       end
     end
   end
@@ -107,32 +109,15 @@ def isRunning
 
   def stop
     return if @pid.nil?
-    @process.close
-    if defined? spawn
-      @process = IO.popen(stop_command_args)
-    else
-      @process = IO.popen(stop_command)
+    unless @process.nil?
+      @process.terminate
+      @process.wait
+      @process = nil
     end
-    @process.close
+    Subprocess.check_call(stop_command_args)
     @pid = nil
   end
 
-  def command
-    start_command
-  end
-
-  def start_command
-    cmd = "#{@binary_path} -d start -logFile '#{@logfile}' #{@folder_flag} #{@key} #{@folder_path} #{@force_local_flag}"
-    cmd += " -localIdentifier #{@local_identifier_flag}" if @local_identifier_flag
-    cmd += " #{@only_flag} #{@only_automate_flag}"
-    cmd += " -proxyHost #{@proxy_host}" if @proxy_host
-    cmd += " -proxyPort #{@proxy_port}" if @proxy_port
-    cmd += " -proxyUser #{@proxy_user}" if @proxy_user
-    cmd += " -proxyPass #{@proxy_pass}" if @proxy_pass
-    cmd += " #{@force_proxy_flag} #{@force_flag} #{@verbose_flag} #{@hosts} #{@user_arguments.join(" ")} 2>&1"
-    cmd.strip
-  end
-
   def start_command_args
     args = [@binary_path, "-d", "start", "-logFile", @logfile, @key, @folder_flag, @folder_path, @force_local_flag]
     args += ["-localIdentifier", @local_identifier_flag] if @local_identifier_flag
@@ -144,25 +129,13 @@ def start_command_args
     args += [@force_proxy_flag, @force_flag, @verbose_flag, @hosts]
     args += @user_arguments
 
-    args = args.select {|a| a.to_s != "" }
-    args.push(:err => [:child, :out])
-    args
-  end
-
-  def stop_command
-    if @local_identifier_flag
-      return "#{@binary_path} -d stop -localIdentifier #{@local_identifier_flag}".strip
-    else
-      return "#{@binary_path} -d stop".strip
-    end
+    args.select {|a| a.to_s != "" }
   end
 
   def stop_command_args
     args = ["#{@binary_path}", "-d", "stop"]
     args +=  ["-localIdentifier", "#{@local_identifier_flag}"] if @local_identifier_flag
-    args = args.select {|a| a.to_s != "" }
-    args.push(:err => [:child, :out])
-    args
+    args.select {|a| a.to_s != "" }
   end
 end