Merge pull request #633 from calabash/feature/better-xcodebuild-proce…

…ss-management

Better xcodebuild process management

lib/run_loop/core_simulator.rb

@@ -157,6 +157,8 @@ def self.terminate_core_simulator_processes
   # @!visibility private
   # Quit any Simulator.app or iOS Simulator.app
   def self.quit_simulator
+    RunLoop::DeviceAgent::Xcodebuild.terminate_simulator_tests
+
     SIMULATOR_QUIT_PROCESSES.each do |process_details|
       process_name = process_details[0]
       send_term_first = process_details[1]

lib/run_loop/device_agent/client.rb

@@ -1308,10 +1308,6 @@ def shutdown
               kill = RunLoop::ProcessTerminator.new(pid, "KILL", process_name, kill_options)
               kill.kill_process
             end
-
-            if process_name == :xcodebuild
-              sleep(10)
-            end
           end
         end
         hash

lib/run_loop/device_agent/ios_device_manager.rb

@@ -108,22 +108,15 @@ def launch(options)
 
         start = Time.now
         if device.simulator?
+          RunLoop::DeviceAgent::Xcodebuild.terminate_simulator_tests
+
           cbxapp = RunLoop::App.new(runner.runner)
           sim = CoreSimulator.new(device, cbxapp)
 
-          should_term_test = lambda do |process_description|
-            xcodebuild_destination_is_simulator?(process_description)
-          end
-          terminate_xcodebuild_test_processes(should_term_test)
-
           sim.install
           sim.launch_simulator
         else
-
-          should_term_test = lambda do |process_description|
-            xcodebuild_destination_is_same?(process_description)
-          end
-          terminate_xcodebuild_test_processes(should_term_test)
+          RunLoop::DeviceAgent::Xcodebuild.terminate_device_test(device.udid)
 
           if !install_timeout
             raise ArgumentError, %Q[
@@ -190,56 +183,6 @@ def launch(options)
         pid.to_i
       end
 
-      def terminate_xcodebuild_test_processes(should_term_test)
-        options = { :timeout => 0.5, :raise_on_timeout => false }
-        pids = RunLoop::ProcessWaiter.new("xcodebuild", options).pids
-        pids.each do |pid|
-          if should_term_test.call(process_env(pid))
-            terminate_running_xcodebuild_process(pid)
-          end
-        end
-      end
-
-      def process_env(pid)
-        options = {:log_cmd => true}
-        args = ["ps", "-p", pid.to_s, "-wwwE"]
-        hash = run_shell_command(args, options)
-        hash[:out]
-      end
-
-      def xcodebuild_destination_is_simulator?(process_description)
-        id_part = process_description[/id=#{device.udid}/]
-        return false if !id_part || id_part == ""
-
-        tokens = id_part.split("=")
-        udid = tokens[1]
-        udid[RunLoop::Regex::DEVICE_UDID_REGEX, 0].nil?
-      end
-
-      def xcodebuild_destination_is_same?(process_description)
-        process_description[/id=#{device.udid}/]
-      end
-
-      def terminate_running_xcodebuild_process(pid)
-        term_options = { :timeout => 1.5 }
-        kill_options = { :timeout => 1.0 }
-
-        process_name = "xcodebuild test-without-building"
-
-        term = RunLoop::ProcessTerminator.new(pid.to_i,
-                                              "TERM",
-                                              process_name,
-                                              term_options)
-        if !term.kill_process
-          kill = RunLoop::ProcessTerminator.new(pid.to_i,
-                                                "KILL",
-                                                process_name,
-                                                kill_options)
-          kill.kill_process
-        end
-        sleep(1.0)
-      end
-
       def app_installed?(bundle_identifier)
         options = {:log_cmd => true}
 

lib/run_loop/device_agent/xcodebuild.rb

@@ -116,7 +116,69 @@ def default_workspace
         relative = File.expand_path(File.join(this_dir, "..", "..", "..", ".."))
         File.join(relative, "DeviceAgent.iOS/DeviceAgent.xcworkspace")
       end
+
+      # @visibility private
+      def self.terminate_simulator_tests
+        should_term_test = lambda do |process_description|
+          xcodebuild_destination_is_simulator?(process_description)
+        end
+
+        self.terminate_xcodebuild_test_processes(should_term_test)
+      end
+
+      # @visibility private
+      def self.terminate_device_test(udid)
+        should_term_test = lambda do |process_description|
+          process_description[/id=#{udid}/]
+        end
+        self.terminate_xcodebuild_test_processes(should_term_test)
+      end
+
+      # @visibility private
+      def self.terminate_xcodebuild_test_processes(should_term_test)
+        options = { :timeout => 0.5, :raise_on_timeout => false }
+        pids = RunLoop::ProcessWaiter.new("xcodebuild", options).pids
+        pids.each do |pid|
+          if should_term_test.call(process_env(pid))
+            RunLoop.log_debug("Will terminate xcodebuild process: #{pid}")
+            terminate_xcodebuild_test_process(pid)
+          end
+        end
+      end
+
+      # @visibility private
+      def self.terminate_xcodebuild_test_process(pid)
+        term_options = { :timeout => 1.5 }
+        kill_options = { :timeout => 1.0 }
+
+        process_name = "xcodebuild test-without-building"
+
+        term = RunLoop::ProcessTerminator.new(pid.to_i,
+                                              "TERM",
+                                              process_name,
+                                              term_options)
+        if !term.kill_process
+          kill = RunLoop::ProcessTerminator.new(pid.to_i,
+                                                "KILL",
+                                                process_name,
+                                                kill_options)
+          kill.kill_process
+        end
+        sleep(1.0)
+      end
+
+      # @visibility private
+      def self.xcodebuild_destination_is_simulator?(process_description)
+        process_description[/-destination id=#{RunLoop::Regex::CORE_SIMULATOR_UDID_REGEX}/]
+      end
+
+      # @visibility private
+      def self.process_env(pid)
+        options = {:log_cmd => true}
+        args = ["ps", "-p", pid.to_s, "-wwwE"]
+        hash = RunLoop::Shell.run_shell_command(args, options)
+        hash[:out]
+      end
     end
   end
 end
-

spec/integration/device_agent/client_spec.rb

@@ -44,24 +44,76 @@
         end
       end
 
-      it "ios_device_manager" do
-        cbx_launcher = RunLoop::DeviceAgent::IOSDeviceManager.new(device)
-        client = RunLoop::DeviceAgent::Client.new(bundle_identifier,
-                                                  device, cbx_launcher,
-                                                  {})
-        client.launch
-
-        options = { :raise_on_timeout => true, :timeout => 5 }
-        RunLoop::ProcessWaiter.new("Preferences", options).wait_for_any
-
-        if RunLoop::Environment.ci?
-          sleep(5)
-        else
-          sleep(1)
+      context "iOSDeviceManager" do
+
+        def launch_app(device, bundle_identifier)
+          cbx_launcher = RunLoop::DeviceAgent::IOSDeviceManager.new(device)
+          client = RunLoop::DeviceAgent::Client.new(bundle_identifier,
+                                                    device, cbx_launcher,
+                                                    {})
+          client.launch
+
+          options = { :raise_on_timeout => true, :timeout => 5 }
+          RunLoop::ProcessWaiter.new("Preferences", options).wait_for_any
+
+          if RunLoop::Environment.ci?
+            sleep(5)
+          else
+            sleep(1)
+          end
+          client
         end
 
-        point = client.query_for_coordinate({marked: "General"})
-        client.perform_coordinate_gesture("touch", point[:x], point[:y])
+        def touch_general_row(client)
+          point = client.query_for_coordinate({marked: "General"})
+          client.perform_coordinate_gesture("touch", point[:x], point[:y])
+          sleep(1.0)
+        end
+
+        it "Simulator does not relaunch after it is quit" do
+          client = launch_app(device, bundle_identifier)
+          touch_general_row(client)
+
+          RunLoop::CoreSimulator.quit_simulator
+
+          20.times do
+            waiter = RunLoop::ProcessWaiter.new("Simulator")
+            expect(waiter.pids.empty?).to be_truthy
+            sleep 1.0
+          end
+        end
+
+        it "Simulator does not relaunch if next test targets same simulator" do
+          client = launch_app(device, bundle_identifier)
+          touch_general_row(client)
+
+          pid = RunLoop::ProcessWaiter.new("Preferences").pids.first
+          expect(pid).to be_truthy
+          RunLoop::ProcessTerminator.new(pid, "TERM", "Preferences",
+                                         {raise_on_no_terminate: true})
+
+          client = launch_app(device, bundle_identifier)
+          touch_general_row(client)
+        end
+
+        it "RunLoop::Client#shutdown terminates xcodebuild-Simulator process" do
+          client = launch_app(device, bundle_identifier)
+          touch_general_row(client)
+
+          client.send(:shutdown)
+
+          RunLoop::ProcessWaiter.new("xcodebuild").wait_for_none
+        end
+
+        it "next test targets a different Simulator" do
+          client = launch_app(device, bundle_identifier)
+          touch_general_row(client)
+
+          version = RunLoop::Version.new("9.0")
+          other_device = Resources.shared.random_simulator_device(version)
+          client = launch_app(other_device, bundle_identifier)
+          touch_general_row(client)
+        end
       end
     end
   end

spec/lib/device_agent/ios_device_manager_spec.rb

@@ -155,38 +155,4 @@
       end
     end
   end
-
-  context "#xcodebuild_destination_is_simulator?" do
-    let(:idm) { RunLoop::DeviceAgent::IOSDeviceManager.new(simulator) }
-
-    it "returns false if process description does not contain id=<destination>" do
-      description = "does not contain 'id=' pattern"
-      expect(idm.xcodebuild_destination_is_simulator?(description)).to be_falsey
-    end
-
-    it "returns false if process description has a physical device destination" do
-      description = "xcodebuild id=#{device.udid}"
-      expect(idm.xcodebuild_destination_is_simulator?(description)).to be_falsey
-    end
-
-    it "returns true if process description is a simulator" do
-      description = "xcodebuild id=#{simulator.udid}"
-      expect(idm.xcodebuild_destination_is_simulator?(description)).to be_truthy
-    end
-  end
-
-  context "#xcodebuild_destination_is_same?" do
-    let(:idm) { RunLoop::DeviceAgent::IOSDeviceManager.new(device) }
-
-    it "returns false if the id=<destination> is not the same" do
-      description = "xcodebuild id=#{simulator.udid}"
-      expect(idm.xcodebuild_destination_is_same?(description)).to be_falsey
-    end
-
-    it "returns true if the id=<destination> is the same" do
-      description = "xcodebuild id=#{device.udid}"
-      expect(idm.xcodebuild_destination_is_same?(description)).to be_truthy
-    end
-  end
 end
-

spec/resources.rb

@@ -204,9 +204,13 @@ def mock_core_simulator_device_data_dir(sdk)
     end
   end
 
-  def random_simulator_device
+  def random_simulator_device(min_version=nil)
     simctl.simulators.shuffle.detect do |device|
-      device.name[/Resizable/,0] == nil
+      [
+        !device.name[/Resizable/],
+        !device.name[/rspec/],
+        min_version ? device.version >= min_version : true
+      ].all?
     end
   end