Fix bugs in storage and demomode

screengrab/lib/screengrab/runner.rb

@@ -19,7 +19,6 @@ def initialize(executor = FastlaneCore::CommandExecutor,
 
     def run
       # Standardize the locales
-      @config[:locales].map! { |locale| locale.gsub("_", "-") }
       FastlaneCore::PrintTable.print_values(config: @config, hide_keys: [], title: "Summary for screengrab #{Fastlane::VERSION}")
 
       app_apk_path = @config.fetch(:app_apk_path, ask: false)
@@ -59,16 +58,16 @@ def run
 
       # Root is needed to access device paths at /data
       if @config[:use_adb_root]
-        run_adb_command("-s #{device_serial} root", print_all: false, print_command: true)
-        run_adb_command("-s #{device_serial} wait-for-device", print_all: false, print_command: true)
+        run_adb_command("-s #{device_serial.shellescape} root", print_all: false, print_command: true)
+        run_adb_command("-s #{device_serial.shellescape} wait-for-device", print_all: false, print_command: true)
       end
 
       clear_device_previous_screenshots(@config[:app_package_name], device_serial, device_screenshots_paths)
 
       app_apk_path ||= select_app_apk(discovered_apk_paths)
       tests_apk_path ||= select_tests_apk(discovered_apk_paths)
 
-      number_of_screenshots = run_tests(device_type_dir_name, device_screenshots_paths, device_serial, app_apk_path, tests_apk_path, test_classes_to_use, test_packages_to_use, @config[:launch_arguments])
+      number_of_screenshots = run_tests(device_type_dir_name, device_serial, app_apk_path, tests_apk_path, test_classes_to_use, test_packages_to_use, @config[:launch_arguments])
 
       ReportsGenerator.new.generate
 
@@ -128,8 +127,8 @@ def screenshot_file_names_in(output_directory, device_type)
     def get_device_environment_variable(device_serial, variable_name)
       # macOS evaluates $foo in `echo $foo` before executing the command,
       # Windows doesn't - hence the double backslash vs. single backslash
-      command = Helper.windows? ? "shell echo \$#{variable_name}" : "shell echo \\$#{variable_name}"
-      value = run_adb_command("-s #{device_serial} #{command}",
+      command = Helper.windows? ? "shell echo \$#{variable_name.shellescape.shellescape}" : "shell echo \\$#{variable_name.shellescape.shellescape}"
+      value = run_adb_command("-s #{device_serial.shellescape} #{command}",
                               print_all: true,
                               print_command: true)
       return value.strip
@@ -165,7 +164,7 @@ def clear_device_previous_screenshots(app_package_name, device_serial, device_sc
 
       device_screenshots_paths.each do |device_path|
         if_device_path_exists(app_package_name, device_serial, device_path) do |path|
-          run_adb_command("-s #{device_serial} shell run-as #{app_package_name} rm -rf #{path}",
+          run_adb_command("-s #{device_serial.shellescape} shell run-as #{app_package_name.shellescape.shellescape} rm -rf #{path.shellescape.shellescape}",
                           print_all: true,
                           print_command: true)
         end
@@ -174,13 +173,13 @@ def clear_device_previous_screenshots(app_package_name, device_serial, device_sc
 
     def install_apks(device_serial, app_apk_path, tests_apk_path)
       UI.message('Installing app APK')
-      apk_install_output = run_adb_command("-s #{device_serial} install -t -r #{app_apk_path.shellescape}",
+      apk_install_output = run_adb_command("-s #{device_serial.shellescape} install -t -r #{app_apk_path.shellescape}",
                                            print_all: true,
                                            print_command: true)
       UI.user_error!("App APK could not be installed") if apk_install_output.include?("Failure [")
 
       UI.message('Installing tests APK')
-      apk_install_output = run_adb_command("-s #{device_serial} install -t -r #{tests_apk_path.shellescape}",
+      apk_install_output = run_adb_command("-s #{device_serial.shellescape} install -t -r #{tests_apk_path.shellescape}",
                                            print_all: true,
                                            print_command: true)
       UI.user_error!("Tests APK could not be installed") if apk_install_output.include?("Failure [")
@@ -191,47 +190,47 @@ def uninstall_apks(device_serial, app_package_name, tests_package_name)
 
       if packages.include?(app_package_name.to_s)
         UI.message('Uninstalling app APK')
-        run_adb_command("-s #{device_serial} uninstall #{app_package_name}",
+        run_adb_command("-s #{device_serial.shellescape} uninstall #{app_package_name.shellescape}",
                         print_all: true,
                         print_command: true)
       end
 
       if packages.include?(tests_package_name.to_s)
         UI.message('Uninstalling tests APK')
-        run_adb_command("-s #{device_serial} uninstall #{tests_package_name}",
+        run_adb_command("-s #{device_serial.shellescape} uninstall #{tests_package_name.shellescape}",
                         print_all: true,
                         print_command: true)
       end
     end
 
     def grant_permissions(device_serial)
       UI.message('Granting the permission necessary to change locales on the device')
-      run_adb_command("-s #{device_serial} shell pm grant #{@config[:app_package_name]} android.permission.CHANGE_CONFIGURATION",
+      run_adb_command("-s #{device_serial.shellescape} shell pm grant #{@config[:app_package_name].shellescape.shellescape} android.permission.CHANGE_CONFIGURATION",
                       print_all: true,
                       print_command: true,
                       raise_errors: false)
 
       UI.message('Granting the permissions necessary to access device external storage')
-      run_adb_command("-s #{device_serial} shell pm grant #{@config[:app_package_name]} android.permission.WRITE_EXTERNAL_STORAGE",
+      run_adb_command("-s #{device_serial.shellescape} shell pm grant #{@config[:app_package_name].shellescape.shellescape} android.permission.WRITE_EXTERNAL_STORAGE",
                       print_all: true,
                       print_command: true,
                       raise_errors: false)
-      run_adb_command("-s #{device_serial} shell pm grant #{@config[:app_package_name]} android.permission.READ_EXTERNAL_STORAGE",
+      run_adb_command("-s #{device_serial.shellescape} shell pm grant #{@config[:app_package_name].shellescape.shellescape} android.permission.READ_EXTERNAL_STORAGE",
                       print_all: true,
                       print_command: true,
                       raise_errors: false)
     end
 
     def kill_app(device_serial, package_name)
-      run_adb_command("-s #{device_serial} shell am force-stop #{package_name}.test",
+      run_adb_command("-s #{device_serial.shellescape} shell am force-stop #{package_name.shellescape.shellescape}.test",
                       print_all: true,
                       print_command: true)
-      run_adb_command("-s #{device_serial} shell am force-stop #{package_name}",
+      run_adb_command("-s #{device_serial.shellescape} shell am force-stop #{package_name.shellescape.shellescape}",
                       print_all: true,
                       print_command: true)
     end
 
-    def run_tests(device_type_dir_name, device_screenshots_paths, device_serial, app_apk_path, tests_apk_path, test_classes_to_use, test_packages_to_use, launch_arguments)
+    def run_tests(device_type_dir_name, device_serial, app_apk_path, tests_apk_path, test_classes_to_use, test_packages_to_use, launch_arguments)
       sdk_version = device_api_version(device_serial)
       unless @config[:reinstall_app]
         install_apks(device_serial, app_apk_path, tests_apk_path)
@@ -249,26 +248,25 @@ def run_tests(device_type_dir_name, device_screenshots_paths, device_serial, app
         else
           kill_app(device_serial, @config[:app_package_name])
         end
-        number_of_screenshots += run_tests_for_locale(device_type_dir_name, device_screenshots_paths, locale, device_serial, test_classes_to_use, test_packages_to_use, launch_arguments, sdk_version)
+        number_of_screenshots += run_tests_for_locale(device_type_dir_name, locale, device_serial, test_classes_to_use, test_packages_to_use, launch_arguments, sdk_version)
       end
 
       number_of_screenshots
     end
 
-    def run_tests_for_locale(device_type_dir_name, device_screenshots_paths, locale, device_serial, test_classes_to_use, test_packages_to_use, launch_arguments, sdk_version)
+    def run_tests_for_locale(device_type_dir_name, locale, device_serial, test_classes_to_use, test_packages_to_use, launch_arguments, sdk_version)
       UI.message("Running tests for locale: #{locale}")
 
-      instrument_command = ["-s #{device_serial} shell am instrument --no-window-animation -w",
-                            "-e testLocale #{locale.tr('-', '_')}",
-                            "-e endingLocale #{@config[:ending_locale].tr('-', '_')}"]
+      instrument_command = ["-s #{device_serial.shellescape} shell am instrument --no-window-animation -w",
+                            "-e testLocale #{locale.shellescape.shellescape}"]
       if sdk_version >= 28
         instrument_command << "--no-hidden-api-checks"
       end
       instrument_command << "-e appendTimestamp #{@config[:use_timestamp_suffix]}"
-      instrument_command << "-e class #{test_classes_to_use.join(',')}" if test_classes_to_use
-      instrument_command << "-e package #{test_packages_to_use.join(',')}" if test_packages_to_use
+      instrument_command << "-e class #{test_classes_to_use.join(',').shellescape.shellescape}" if test_classes_to_use
+      instrument_command << "-e package #{test_packages_to_use.join(',').shellescape.shellescape}" if test_packages_to_use
       instrument_command << launch_arguments.map { |item| '-e ' + item }.join(' ') if launch_arguments
-      instrument_command << "#{@config[:tests_package_name]}/#{@config[:test_instrumentation_runner]}"
+      instrument_command << "#{@config[:tests_package_name].shellescape.shellescape}/#{@config[:test_instrumentation_runner].shellescape.shellescape}"
 
       test_output = run_adb_command(instrument_command.join(" \\\n"),
                                     print_all: true,
@@ -282,15 +280,21 @@ def run_tests_for_locale(device_type_dir_name, device_screenshots_paths, locale,
         end
       end
 
-      pull_screenshots_from_device(locale, device_serial, device_screenshots_paths, device_type_dir_name)
+      pull_screenshots_from_device(locale, device_serial, device_type_dir_name)
     end
 
-    def pull_screenshots_from_device(locale, device_serial, device_screenshots_paths, device_type_dir_name)
+    def pull_screenshots_from_device(locale, device_serial, device_type_dir_name)
       UI.message("Pulling captured screenshots for locale #{locale} from the device")
       starting_screenshot_count = screenshot_file_names_in(@config[:output_directory], device_type_dir_name).length
 
       UI.verbose("Starting screenshot count is: #{starting_screenshot_count}")
 
+      device_screenshots_paths = [
+        determine_external_screenshots_path(device_serial, @config[:app_package_name], [locale]),
+        determine_internal_screenshots_paths(device_serial, @config[:app_package_name], [locale])
+      ].flatten
+
+
       # Make a temp directory into which to pull the screenshots before they are moved to their final location.
       # This makes directory cleanup easier, as the temp directory will be removed when the block completes.
 
@@ -299,14 +303,14 @@ def pull_screenshots_from_device(locale, device_serial, device_screenshots_paths
           if_device_path_exists(@config[:app_package_name], device_serial, device_path) do |path|
             UI.message(path)
             next unless path.include?(locale)
-            out = run_adb_command("-s #{device_serial} pull #{path} #{tempdir}",
+            out = run_adb_command("-s #{device_serial.shellescape} pull #{path.shellescape} #{tempdir.shellescape}",
                                   print_all: false,
                                   print_command: true,
                                   raise_errors: false)
             if out =~ /Permission denied/
               dir = File.dirname(path)
               base = File.basename(path)
-              run_adb_command("-s #{device_serial} shell run-as #{@config[:app_package_name]} \"tar -cC #{dir} #{base}\" | tar -xv -f- -C #{tempdir}",
+              run_adb_command("-s #{device_serial.shellescape} shell run-as #{@config[:app_package_name].shellescape.shellescape} \"tar -cC #{dir} #{base}\" | tar -xv -f- -C #{tempdir}",
                               print_all: false,
                               print_command: true)
             end
@@ -371,7 +375,7 @@ def move_pulled_screenshots(locale, pull_dir, device_type_dir_name)
     # Some device commands fail if executed against a device path that does not exist, so this helper method
     # provides a way to conditionally execute a block only if the provided path exists on the device.
     def if_device_path_exists(app_package_name, device_serial, device_path)
-      return if run_adb_command("-s #{device_serial} shell run-as #{app_package_name} ls #{device_path}",
+      return if run_adb_command("-s #{device_serial.shellescape} shell run-as #{app_package_name.shellescape.shellescape} ls #{device_path.shellescape.shellescape}",
                                 print_all: false,
                                 print_command: false).include?('No such file')
 
@@ -383,15 +387,15 @@ def if_device_path_exists(app_package_name, device_serial, device_path)
 
     # Return an array of packages that are installed on the device
     def installed_packages(device_serial)
-      packages = run_adb_command("-s #{device_serial} shell pm list packages",
+      packages = run_adb_command("-s #{device_serial.shellescape} shell pm list packages",
                                  print_all: true,
                                  print_command: true)
       packages.split("\n").map { |package| package.gsub("package:", "") }
     end
 
     def run_adb_command(command, print_all: false, print_command: false, raise_errors: true)
       adb_host = @config[:adb_host]
-      host = adb_host.nil? ? '' : "-H #{adb_host} "
+      host = adb_host.nil? ? '' : "-H #{adb_host.shellescape} "
       output = ''
       begin
         errout = nil
@@ -412,7 +416,7 @@ def run_adb_command(command, print_all: false, print_command: false, raise_error
     end
 
     def device_api_version(device_serial)
-      run_adb_command("-s #{device_serial} shell getprop ro.build.version.sdk",
+      run_adb_command("-s #{device_serial.shellescape} shell getprop ro.build.version.sdk",
                       print_all: true, print_command: true).to_i
     end
 
@@ -422,11 +426,11 @@ def enable_clean_status_bar(device_serial, sdk_version)
       UI.message('Enabling clean status bar')
 
       # Grant the DUMP permission
-      run_adb_command("-s #{device_serial} shell pm grant #{@config[:app_package_name]} android.permission.DUMP",
+      run_adb_command("-s #{device_serial.shellescape} shell pm grant #{@config[:app_package_name].shellescape.shellescape} android.permission.DUMP",
                       print_all: true, print_command: true, raise_errors: false)
 
       # Enable the SystemUI demo mode
-      run_adb_command("-s #{device_serial} shell settings put global sysui_demo_allowed 1",
+      run_adb_command("-s #{device_serial.shellescape} shell settings put global sysui_demo_allowed 1",
                       print_all: true, print_command: true)
     end
   end

screengrab/screengrab-lib/build.gradle

@@ -64,3 +64,8 @@ dependencies {
     implementation 'androidx.test.uiautomator:uiautomator:2.2.0'
     implementation 'com.jraska:falcon:2.1.1'
 }
+gradle.projectsEvaluated {
+    tasks.withType(JavaCompile){
+        options.compilerArgs << "-Xlint:deprecation"
+    }
+}

screengrab/screengrab-lib/src/main/java/tools.fastlane.screengrab/FileWritingScreenshotCallback.java

@@ -29,15 +29,17 @@ public class FileWritingScreenshotCallback implements ScreenshotCallback {
     private static final String APPEND_TIMESTAMP_CONFIG_KEY = "appendTimestamp";
 
     private final Context appContext;
+    private final String locale;
 
-    public FileWritingScreenshotCallback(Context appContext) {
+    public FileWritingScreenshotCallback(Context appContext, String locale) {
         this.appContext = appContext;
+        this.locale = locale;
     }
 
     @Override
     public void screenshotCaptured(String screenshotName, Bitmap screenshot) {
         try {
-            File screenshotDirectory = getFilesDirectory(appContext, Locale.getDefault());
+            File screenshotDirectory = getFilesDirectory(appContext, locale);
             File screenshotFile = getScreenshotFile(screenshotDirectory, screenshotName);
 
             OutputStream fos = null;
@@ -65,7 +67,8 @@ protected File getScreenshotFile(File screenshotDirectory, String screenshotName
     }
 
     @SuppressLint("WorldReadableFiles")
-    private static File getFilesDirectory(Context context, Locale locale) throws IOException {
+    @SuppressWarnings("deprecation")
+    private static File getFilesDirectory(Context context, String locale) throws IOException {
         File base;
         if (Build.VERSION.SDK_INT > Build.VERSION_CODES.Q) {
             base = context.getDir(SCREENGRAB_DIR_NAME, Context.MODE_PRIVATE);
@@ -79,7 +82,7 @@ private static File getFilesDirectory(Context context, Locale locale) throws IOE
             throw new IOException("Unable to get a world-readable directory");
         }
 
-        File directory = initializeDirectory(new File(base, localeToDirName(locale)));
+        File directory = initializeDirectory(new File(new File(base, locale), "/images/screenshots"));
         if (directory == null) {
             throw new IOException("Unable to get a screenshot storage directory");
         }
@@ -102,17 +105,6 @@ private static File initializeDirectory(File dir) {
         return null;
     }
 
-    private static String localeToDirName(Locale locale) {
-        StringBuilder sb = new StringBuilder(locale.getLanguage());
-        String localeCountry = locale.getCountry();
-
-        if (localeCountry.length() != 0) {
-            sb.append("-").append(localeCountry);
-        }
-
-        return sb.append("/images/screenshots").toString();
-    }
-
     private static void createPathTo(File dir) throws IOException {
         File parent = dir.getParentFile();
         if (parent != null && !parent.exists()) {

screengrab/screengrab-lib/src/main/java/tools.fastlane.screengrab/Screengrab.java

@@ -25,12 +25,40 @@
 import androidx.test.platform.app.InstrumentationRegistry;
 
 import java.util.regex.Pattern;
+import java.util.Locale;
 
 public class Screengrab {
     private static final Pattern TAG_PATTERN = Pattern.compile("[a-zA-Z0-9_-]+");
 
     private static ScreenshotStrategy defaultScreenshotStrategy = new UiAutomatorScreenshotStrategy();
 
+    private static String locale;
+
+    /**
+     * @return The current locale
+     */
+    public static String getLocale() {
+        if (locale == null) {
+            Locale implicitLocale = Locale.getDefault();
+            StringBuilder sb = new StringBuilder(implicitLocale.getLanguage());
+            String localeCountry = implicitLocale.getCountry();
+
+            if (localeCountry.length() != 0) {
+                sb.append("-").append(localeCountry);
+            }
+            return sb.toString();
+        } else {
+            return locale;
+        }
+    }
+
+    /**
+     * Set the current locale that will be returned from {@link #getLocale()}
+     */
+    public static void setLocale(String locale) {
+        Screengrab.locale = locale;
+    }
+
     /**
      * @return The default {@link ScreenshotStrategy} used in {@link #screenshot(String)} invocations
      */
@@ -72,7 +100,7 @@ public static void screenshot(String screenshotName, ScreenshotStrategy strategy
                 .getTargetContext()
                 .getApplicationContext();
 
-        screenshot(screenshotName, strategy, new FileWritingScreenshotCallback(appContext));
+        screenshot(screenshotName, strategy, new FileWritingScreenshotCallback(appContext, getLocale()));
     }
 
     /**