Own folder structure for files and screenshots.

[artem-zinnatullin]

Currently Composer supports only Spoon's folder structure, would be great to add our own one.

[trevjonez]

I am running into issues with spoon screenshot system where it fails to create external files dir. Is there anything scheduled for this or any planning that has been done?

I would like to advocate adding a test apk library to facilitate screenshot output.

Something similar to what I did with kontrast that will use internal storage thus eliminating the permissions issue. then use the instrumentation status api to send path information back to composer. In addition add support for deletion upon pulling screenshot to avoid file system bloat.

Ultimately you could recommend using falcon to actually handle rendering then use your own logic for file path creation and instrumentation status dispatching.

[artem-zinnatullin]

For now I'd recommend to simply grant permission to read/write external storage in the tests, see https://artemzin.com/blog/easiest-way-to-give-set_animation_scale-permission-for-your-ui-tests-on-android/

But in long run, yes, Composer should support pulling screenshots/files from internal app folder as well!

[trevjonez]

tried that already. even with permissions granted the test apk can't create the app spoon screenshot dir. not sure why. external file directories seem to be a moving target.

[artem-zinnatullin]

Hmmm, definitely works for us.

Maybe you grant permission too late? You also need to have permission in the manifest even if your app doesn't use it normally. (you can add it only for debug builds app/src/debug/AndroidManifest.xml)

Although, we don't use Spoon client jar, we just have own screenshoter that follows Spoon dir structure

[trevjonez]

I'll do some more digging. going to setup an empty project so I can post it all up for review by those with better understanding of system specifics than I.

[yunikkk]

@trevjonez hi, are you using 23+ targetSdk for the case mentioned? We've just experienced issues with such setup but finally got it working.

[trevjonez]

@yunikkk correct. Our main target is 26 but screenshots fail unless I make my CI variant target 22.

[yunikkk]

We managed to solve the problems with granting both READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE to the main application, eg. :

val packageName = targetContext.packageName
listOf(
                "android.permission.WRITE_EXTERNAL_STORAGE",
                "android.permission.READ_EXTERNAL_STORAGE" // needed, too, to create screenshots.
).forEach { permissionName ->
            UiDevice.getInstance(this).executeShellCommand("pm grant $packageName $permissionName")
}

kinda works. The most strange thing is that permissions are granted NOT to instrumentation app, but the app under test. First tried to grant them to instrumentation but failed. Very weird...

[trevjonez]

I'll give that a try. perhaps it was because I was only granting WRITE_EXTERNAL_STORAGE since read is granted implicitly with it. perhaps i'll attempt the same thing with the new permissions rule and see if that gets me anywhere.

either way it would be nice to get it done to the point that you don't need file perms to do the screenshot saving. Again the kontrast system I wrote does just that. It follows similar reactive pull logic like used in composer then I also took it a step further and have the host gradle plugin dispatch delete commands after the file has been pulled to make sure we don't have space issues or leave a mess in our wake.

[yunikkk]

Seems, that READ is not granted automatically if you allow WRITE from adb, so it works differently from allowing it manually from the UI.

Totally agree about removal of permissions at all.

[trevjonez]

so based on that I updated my test rule to have both WRITE and READ and of course it works now.

@get:Rule
val storageRule: GrantPermissionRule = grant(WRITE_EXTERNAL_STORAGE, READ_EXTERNAL_STORAGE)

[artem-zinnatullin]

Yeah, READ is not granted automatically when you grant WRITE, @yunikkk I think I've left comment about that in Juno Rider UI tests

[yunikkk]

Yeah, thats super confusing since granting from UI works the other way...

[christopherperry]

We are using Falcon for our screenshots. To get it to work with composer I had to pull the code from spoon that creates the screenshot directory structure. I think it might be easier to let users specify the screenshot directory via a param and fall back to Spoon's structure if that's missing. Do you guys see any issues with that?

[artem-zinnatullin]

Depends on how you see integration.

Reason why we support Spoon format is because it's allows us figure out which screenshot belongs to which test.

We can't just grab all screenshots from single folder and match them to particular tests unless they have special file name pattern

[christopherperry]

I forgot about that, good point. What if we provide the directory structure and require usage of our own Screenshot call? We could be in control of the file format under the user specified root directory, and allow usage of whichever screenshot lib the user wants to plug in (Falcon, etc). Perhaps provide a default implementation if none is specified.

Here's what we're currently doing:

public void screenshot(Activity activity, String tag) {
  StackTraceElement testClass = findTestClassTraceElement(Thread.currentThread().getStackTrace());
  String className = testClass.getClassName().replaceAll("[^A-Za-z0-9._-]", "_");
  String methodName = testClass.getMethodName();
  return screenshot(activity, tag, className, methodName);
}

public void screenshot(Activity activity, String tag, String testClassName, String testMethodName) {
  Falcon.takeScreenshot(activity, screenshotFile(activity, tag, testClassName, testMethodName));
}

screenshotFile() is using Spoon's source to create the directory and also the sub directory and file name. We could let the user specify a root dir and we do the rest.

[artem-zinnatullin]

Yeah I have a screenshot library on my personal list that can be recommended default for Composer

We're doing pretty much same, just take screenshots differently.

Btw, Falcon has some compatibility option for Spoon, but it might be limited https://github.com/jraska/Falcon#spoon-compat

[christopherperry]

Falcon spoon compat has a dependency on Spoon, which I didn't care for.

[tir38]

When this finally happens, you will still need to expose that format to other libs that actually capture screenshots (e.g. Falcon).

In the meantime, can you just expose the hard-coded spoon format that you depend on? Something like this (w/ code copied from how Spoon works now)

import static android.content.Context.MODE_WORLD_READABLE;
import static android.os.Build.VERSION.SDK_INT;
import static android.os.Build.VERSION_CODES.LOLLIPOP;
import static android.os.Environment.getExternalStorageDirectory;

import android.app.Activity;
import android.content.Context;

import java.io.File;
import java.util.regex.Pattern;

public class ComposerScreenshot {

    private static final String SPOON_SCREENSHOTS = "spoon-screenshots";
    private static final String NAME_SEPARATOR = "_";
    private static final Pattern TAG_VALIDATION = Pattern.compile("[a-zA-Z0-9_-]+");
    private static final String EXTENSION = ".png";

    public static File getFileFormat(final Activity activity, final String tag, final String className,
                                     final String methodName) {
        // we use spoon format internally but nobody need to know that since we just return a file for any screenshot
        // lib to put the image in
        return getSpoonFile(activity, tag, className, methodName);
    }

    private static File getSpoonFile(final Activity activity, final String tag, final String className,
                                     final String methodName) {
        if (!TAG_VALIDATION.matcher(tag).matches()) {
            throw new IllegalArgumentException("Tag must match " + TAG_VALIDATION.pattern() + ".");
        }
        File screenshotDirectory =
                obtainDirectory(activity.getApplicationContext(), className, methodName, SPOON_SCREENSHOTS);
        String screenshotName = System.currentTimeMillis() + NAME_SEPARATOR + tag + EXTENSION;
        File screenshotFile = new File(screenshotDirectory, screenshotName);
        return screenshotFile;
    }

    private static File obtainDirectory(final Context context, final String testClassName,
                                        final String testMethodName, final String directoryName) {
        File directory;
        if (SDK_INT >= LOLLIPOP) {
            // Use external storage.
            directory = new File(getExternalStorageDirectory(), "app_" + directoryName);
        } else {
            // Use internal storage.
            directory = context.getDir(directoryName, MODE_WORLD_READABLE);
        }

        File dirClass = new File(directory, testClassName);
        File dirMethod = new File(dirClass, testMethodName);
        createDir(dirMethod);
        return dirMethod;
    }

    private static void createDir(final File dir) {
        File parent = dir.getParentFile();
        if (!parent.exists()) {
            createDir(parent);
        }
        if (!dir.exists() && !dir.mkdirs()) {
            throw new RuntimeException("Unable to create output dir: " + dir.getAbsolutePath());
        }

        // copied from chmodPlusRWX(dir);
        dir.setReadable(true, false);
        dir.setWritable(true, false);
        dir.setExecutable(true, false);
    }
}

[trevjonez]

PSA: when targeting api 29 the spoon format wont work due to the changes introduced for external storage. https://developer.android.com/training/data-storage/files/external-scoped