[screenscrab] use AdbHelper instead of duplicated code (#15402)

* [screenscrab] Use AdbHelper instead of own code

Co-authored-by: Josh Holtz <josh@rokkincat.com>

* Rebase from master to account for adb host PR being merged

fastlane/lib/fastlane/helper/adb_helper.rb

@@ -12,21 +12,30 @@ class AdbHelper
       # Path to the adb binary
       attr_accessor :adb_path
 
+      # Path to the adb binary
+      attr_accessor :adb_host
+
       # All available devices
       attr_accessor :devices
 
-      def initialize(adb_path: nil)
+      def initialize(adb_path: nil, adb_host: nil)
         android_home = ENV['ANDROID_HOME'] || ENV['ANDROID_SDK_ROOT'] || ENV['ANDROID_SDK']
         if (adb_path.nil? || adb_path == "adb") && android_home
           adb_path = File.join(android_home, "platform-tools", "adb")
         end
+
         self.adb_path = adb_path
+        self.adb_host = adb_host
+      end
+
+      def host_option
+        return self.adb_host ? "-H #{adb_host}" : nil
       end
 
       # Run a certain action
       def trigger(command: nil, serial: nil)
         android_serial = serial != "" ? "ANDROID_SERIAL=#{serial}" : nil
-        command = [android_serial, adb_path.shellescape, command].join(" ").strip
+        command = [android_serial, adb_path.shellescape, host_option, command].compact.join(" ").strip
         Action.sh(command)
       end
 
@@ -43,10 +52,10 @@ def device_available?(serial)
       def load_all_devices
         self.devices = []
 
-        command = [adb_path.shellescape, "devices"].join(" ")
+        command = [adb_path.shellescape, host_option, "devices -l"].compact.join(" ")
         output = Actions.sh(command, log: false)
         output.split("\n").each do |line|
-          if (result = line.match(/(.*)\tdevice$/))
+          if (result = line.match(/^(\S+)(\s+)(device )/))
             self.devices << AdbDevice.new(serial: result[1])
           end
         end

fastlane/spec/helper/adb_helper_spec.rb

@@ -0,0 +1,100 @@
+describe Fastlane::Helper::AdbHelper do
+  describe "#load_all_devices" do
+    context 'adb host' do
+      it 'no host specified' do
+        devices = Fastlane::Helper::AdbHelper.new
+        expect(devices.host_option).to eq(nil)
+      end
+
+      it 'host specified' do
+        devices = Fastlane::Helper::AdbHelper.new(adb_host: 'device_farm')
+        expect(devices.host_option).to eq("-H device_farm")
+      end
+    end
+
+    context 'no devices' do
+      it 'does not find any active devices' do
+        adb_response = strip_heredoc(<<-ADB_OUTPUT)
+        List of devices attached
+
+        ADB_OUTPUT
+
+        allow(Fastlane::Actions).to receive(:sh).and_return(adb_response)
+        devices = Fastlane::Helper::AdbHelper.new.load_all_devices
+
+        expect(devices.count).to eq(0)
+      end
+    end
+
+    context 'one device with spurious ADB output mixed in' do
+      it 'finds an active device' do
+        adb_response = strip_heredoc(<<-ADB_OUTPUT)
+          List of devices attached
+          adb server version (39) doesn't match this client (36); killing...
+          * daemon started successfully
+          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
+
+
+        ADB_OUTPUT
+
+        allow(Fastlane::Actions).to receive(:sh).and_return(adb_response)
+        devices = Fastlane::Helper::AdbHelper.new.load_all_devices
+
+        expect(devices.count).to eq(1)
+        expect(devices[0].serial).to eq("T065002LTT")
+      end
+    end
+
+    context 'one device' do
+      it 'finds an active device' do
+        adb_response = strip_heredoc(<<-ADB_OUTPUT)
+          List of devices attached
+          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
+
+
+        ADB_OUTPUT
+
+        allow(Fastlane::Actions).to receive(:sh).and_return(adb_response)
+        devices = Fastlane::Helper::AdbHelper.new.load_all_devices
+
+        expect(devices.count).to eq(1)
+        expect(devices[0].serial).to eq("T065002LTT")
+      end
+    end
+
+    context 'multiple devices' do
+      it 'finds an active device' do
+        adb_response = strip_heredoc(<<-ADB_OUTPUT)
+          List of devices attached
+          emulator-5554          device product:sdk_phone_x86_64 model:Android_SDK_built_for_x86_64 device:generic_x86_64
+          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
+
+        ADB_OUTPUT
+
+        allow(Fastlane::Actions).to receive(:sh).and_return(adb_response)
+        devices = Fastlane::Helper::AdbHelper.new.load_all_devices
+
+        expect(devices.count).to eq(2)
+        expect(devices[0].serial).to eq("emulator-5554")
+        expect(devices[1].serial).to eq("T065002LTT")
+      end
+    end
+
+    context 'one device booting' do
+      it 'finds an active device' do
+        adb_response = strip_heredoc(<<-ADB_OUTPUT)
+          List of devices attached
+          emulator-5554 offline
+          T065002LTT  device usb:437387264X product:ghost_retail model:XT1053 device:ghost
+
+        ADB_OUTPUT
+
+        allow(Fastlane::Actions).to receive(:sh).and_return(adb_response)
+        devices = Fastlane::Helper::AdbHelper.new.load_all_devices
+
+        expect(devices.count).to eq(1)
+        expect(devices[0].serial).to eq("T065002LTT")
+      end
+    end
+  end
+end

screengrab/lib/screengrab/runner.rb

@@ -1,5 +1,6 @@
 require 'fastlane_core/print_table'
 require 'fastlane_core/command_executor'
+require 'fastlane/helper/adb_helper'
 require_relative 'reports_generator'
 require_relative 'module'
 
@@ -84,35 +85,26 @@ def run
     end
 
     def select_device
-      devices = run_adb_command("devices -l", print_all: true, print_command: true).split("\n")
-      # the first output by adb devices is "List of devices attached" so remove that and any adb startup output
-      devices.reject! do |device|
-        [
-          'server is out of date',    # The adb server is out of date and must be restarted
-          'unauthorized',             # The device has not yet accepted ADB control
-          'offline',                  # The device is offline, skip it
-          '* daemon',                 # Messages printed when the daemon is starting up
-          'List of devices attached', # Header of table for data we want
-          "doesn't match this client" # Message printed when there is an ADB client/server version mismatch
-        ].any? { |status| device.include?(status) }
-      end
+      adb = Fastlane::Helper::AdbHelper.new(adb_host: @config[:adb_host])
+      devices = adb.load_all_devices
 
       UI.user_error!('There are no connected and authorized devices or emulators') if devices.empty?
 
-      devices.select! { |d| d.include?(@config[:specific_device]) } if @config[:specific_device]
+      specific_device = @config[:specific_device]
+      if specific_device
+        devices.select! do |d|
+          d.serial.include?(specific_device)
+        end
+      end
 
-      UI.user_error!("No connected devices matched your criteria: #{@config[:specific_device]}") if devices.empty?
+      UI.user_error!("No connected devices matched your criteria: #{specific_device}") if devices.empty?
 
       if devices.length > 1
         UI.important("Multiple connected devices, selecting the first one")
         UI.important("To specify which connected device to use, use the -s (specific_device) config option")
       end
 
-      # grab the serial number. the lines of output can look like these:
-      # 00c22d4d84aec525       device usb:2148663295X product:bullhead model:Nexus_5X device:bullhead
-      # 192.168.1.100:5555       device usb:2148663295X product:bullhead model:Nexus_5X device:genymotion
-      # emulator-5554       device usb:2148663295X product:bullhead model:Nexus_5X device:emulator
-      devices[0].match(/^\S+/)[0]
+      devices.first.serial
     end
 
     def select_app_apk(discovered_apk_paths)

screengrab/spec/runner_spec.rb

@@ -135,87 +135,6 @@ def mock_adb_response(mock_response)
     end
   end
 
-  describe :select_device do
-    let(:adb_list_devices_command) { 'adb devices -l' }
-
-    before do
-      expect(mock_android_environment).to receive(:adb_path).and_return("adb")
-    end
-
-    context 'no devices' do
-      it 'does not find any active devices' do
-        adb_response = strip_heredoc(<<-ADB_OUTPUT)
-        List of devices attached
-
-        ADB_OUTPUT
-        mock_adb_response_for_command(adb_list_devices_command, adb_response)
-
-        expect(ui).to receive(:user_error!).with(/no connected.* devices/).and_call_original
-
-        expect { @runner.select_device }.to raise_fastlane_error
-      end
-    end
-
-    context 'one device with spurious ADB output mixed in' do
-      it 'finds an active device' do
-        adb_response = strip_heredoc(<<-ADB_OUTPUT)
-          List of devices attached
-          adb server version (39) doesn't match this client (36); killing...
-          * daemon started successfully
-          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
-
-
-        ADB_OUTPUT
-        mock_adb_response_for_command(adb_list_devices_command, adb_response)
-
-        expect(@runner.select_device).to eq('T065002LTT')
-      end
-    end
-
-    context 'one device' do
-      it 'finds an active device' do
-        adb_response = strip_heredoc(<<-ADB_OUTPUT)
-          List of devices attached
-          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
-
-
-        ADB_OUTPUT
-        mock_adb_response_for_command(adb_list_devices_command, adb_response)
-
-        expect(@runner.select_device).to eq('T065002LTT')
-      end
-    end
-
-    context 'multiple devices' do
-      it 'finds an active device' do
-        adb_response = strip_heredoc(<<-ADB_OUTPUT)
-          List of devices attached
-          emulator-5554          device product:sdk_phone_x86_64 model:Android_SDK_built_for_x86_64 device:generic_x86_64
-          T065002LTT             device usb:437387264X product:ghost_retail model:XT1053 device:ghost
-
-        ADB_OUTPUT
-
-        mock_adb_response_for_command(adb_list_devices_command, adb_response)
-        expect(@runner.select_device).to eq('emulator-5554')
-      end
-    end
-
-    context 'one device booting' do
-      it 'finds an active device' do
-        adb_response = strip_heredoc(<<-ADB_OUTPUT)
-          List of devices attached
-          emulator-5554 offline
-          T065002LTT  device
-
-        ADB_OUTPUT
-
-        mock_adb_response_for_command(adb_list_devices_command, adb_response)
-
-        expect(@runner.select_device).to eq('T065002LTT')
-      end
-    end
-  end
-
   describe :uninstall_existing do
     let(:device_serial) { 'device_serial' }
     let(:app_package_name) { 'tools.fastlane.dev' }
@@ -307,16 +226,17 @@ def mock_adb_response(mock_response)
 
       expect(@runner.run_adb_command("test").lines.any? { |line| line.start_with?('adb: ') }).to eq(false)
     end
+  end
 
+  describe :select_device do
     it 'connects to host if specified' do
       config[:adb_host] = "device_farm"
-      adb_response = strip_heredoc(<<-ADB_OUTPUT)
-            List of devices attached
-            e1dbf228               device usb:1-1.2 product:a33gdd model:SM_A300H device:a33g
 
-          ADB_OUTPUT
+      mock_helper = double('mock helper')
+      device = Fastlane::Helper::AdbDevice.new(serial: 'e1dbf228')
 
-      mock_adb_response_for_command("adb -H device_farm devices -l", adb_response)
+      expect(Fastlane::Helper::AdbHelper).to receive(:new).with(adb_host: 'device_farm').and_return(mock_helper)
+      expect(mock_helper).to receive(:load_all_devices).and_return([device])
 
       expect(@runner.select_device).to eq('e1dbf228')
     end