fix: Detect old artifacts dirs on adb device, remove automatically using the new --adb-remove-old-artifacts cli option

src/cmd/run.js

@@ -45,6 +45,7 @@ export type CmdRunParams = {|
   adbPort?: string,
   adbDevice?: string,
   adbDiscoveryTimeout?: number,
+  adbRemoveOldArtifacts?: boolean,
   firefoxApk?: string,
   firefoxApkComponent?: string,
 
@@ -87,6 +88,7 @@ export default async function run(
     adbPort,
     adbDevice,
     adbDiscoveryTimeout,
+    adbRemoveOldArtifacts,
     firefoxApk,
     firefoxApkComponent,
     // Chromium CLI options.
@@ -165,6 +167,7 @@ export default async function run(
       adbPort,
       adbBin,
       adbDiscoveryTimeout,
+      adbRemoveOldArtifacts,
 
       // Injected dependencies.
       firefoxApp,

src/extension-runners/firefox-android.js

@@ -71,6 +71,7 @@ export type FirefoxAndroidExtensionRunnerParams = {|
   adbPort?: string,
   adbDevice?: string,
   adbDiscoveryTimeout?: number,
+  adbRemoveOldArtifacts?: boolean,
   firefoxApk?: string,
   firefoxApkComponent?: string,
 
@@ -431,6 +432,7 @@ export class FirefoxAndroidExtensionRunner {
       params: {
         customPrefs,
         firefoxApp,
+        adbRemoveOldArtifacts,
       },
     } = this;
     // Create the preferences file and the Fennec temporary profile.
@@ -441,6 +443,24 @@ export class FirefoxAndroidExtensionRunner {
       customPrefs,
     });
 
+    // Check if there are any artifacts dirs from previous runs and
+    // automatically remove them if adbRemoteOldArtifacts is true.
+    const foundOldArtifacts = await adbUtils.detectOrRemoveOldArtifacts(
+      selectedAdbDevice, adbRemoveOldArtifacts
+    );
+
+    if (foundOldArtifacts) {
+      if (adbRemoveOldArtifacts) {
+        log.info('Old web-ext artifacts have been found and removed ' +
+          `from ${selectedAdbDevice} device`);
+      } else {
+        log.warn(
+          `Old artifacts directories have been found on ${selectedAdbDevice} ` +
+          'device. Use --adb-remove-old-artifacts to remove them automatically.'
+        );
+      }
+    }
+
     // Choose a artifacts dir name for the assets pushed to the
     // Android device.
     this.selectedArtifactsDir = await adbUtils.getOrCreateArtifactsDir(

src/program.js

@@ -663,6 +663,11 @@ Example: $0 --help run.
         type: 'number',
         requiresArg: true,
       },
+      'adb-remove-old-artifacts': {
+        describe: 'Remove old artifacts directories from the adb device',
+        demandOption: false,
+        type: 'boolean',
+      },
       'firefox-apk': {
         describe: (
           'Run a specific Firefox for Android APK. ' +

src/util/adb.js

@@ -8,6 +8,9 @@ import {
 } from '../errors';
 import {createLogger} from '../util/logger';
 
+export const DEVICE_DIR_BASE = '/sdcard/';
+export const ARTIFACTS_DIR_PREFIX = 'web-ext-artifacts-';
+
 const log = createLogger(__filename);
 
 export type ADBUtilsParams = {|
@@ -195,7 +198,7 @@ export default class ADBUtils {
       return artifactsDir;
     }
 
-    artifactsDir = `/sdcard/web-ext-artifacts-${Date.now()}`;
+    artifactsDir = `${DEVICE_DIR_BASE}${ARTIFACTS_DIR_PREFIX}${Date.now()}`;
 
     const testDirOut = (await this.runShellCommand(
       deviceId, `test -d ${artifactsDir} ; echo $?`
@@ -215,6 +218,44 @@ export default class ADBUtils {
     return artifactsDir;
   }
 
+  async detectOrRemoveOldArtifacts(
+    deviceId: string, removeArtifactDirs?: boolean = false
+  ): Promise<boolean> {
+    const {adbClient} = this;
+
+    log.debug('Checking adb device for existing web-ext artifacts dirs');
+
+    return wrapADBCall(async () => {
+      const files = await adbClient.readdir(deviceId, DEVICE_DIR_BASE);
+      let found = false;
+
+      for (const file of files) {
+        if (!file.isDirectory() ||
+            !file.name.startsWith(ARTIFACTS_DIR_PREFIX)) {
+          continue;
+        }
+
+        // Return earlier if we only need to warn the user that some
+        // existing artifacts dirs have been found on the adb device.
+        if (!removeArtifactDirs) {
+          return true;
+        }
+
+        found = true;
+
+        const artifactsDir = `${DEVICE_DIR_BASE}${file.name}`;
+
+        log.debug(
+          `Removing artifacts directory ${artifactsDir} from device ${deviceId}`
+        );
+
+        await this.runShellCommand(deviceId, ['rm', '-rf', artifactsDir]);
+      }
+
+      return found;
+    });
+  }
+
   async clearArtifactsDir(deviceId: string): Promise<void> {
     const artifactsDir = this.artifactsDirMap.get(deviceId);
 
@@ -229,9 +270,7 @@ export default class ADBUtils {
       `Removing ${artifactsDir} artifacts directory on ${deviceId} device`
     );
 
-    await this.runShellCommand(deviceId, [
-      'rm', '-rf', artifactsDir,
-    ]);
+    await this.runShellCommand(deviceId, ['rm', '-rf', artifactsDir]);
   }
 
   async pushFile(

tests/unit/test-extension-runners/test.firefox-android.js

@@ -130,6 +130,7 @@ function prepareSelectedDeviceAndAPKParams(
     startFirefoxAPK: sinon.spy(() => Promise.resolve()),
     setupForward: sinon.spy(() => Promise.resolve()),
     clearArtifactsDir: sinon.spy(() => Promise.resolve()),
+    detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(true)),
     setUserAbortDiscovery: sinon.spy(() => {}),
     ensureRequiredAPKRuntimePermissions: sinon.spy(() => Promise.resolve()),
     ...adbOverrides,
@@ -357,6 +358,88 @@ describe('util/extension-runners/firefox-android', () => {
          );
        });
 
+    it('does check for existing artifacts dirs', async () => {
+      const adbOverrides = {
+        getOrCreateArtifactsDir: sinon.spy(
+          () => Promise.resolve('/sdcard/web-ext-dir')
+        ),
+        detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(false)),
+      };
+      const overriddenProperties = {
+        params: {
+          adbDevice: 'emulator-1',
+          firefoxApk: 'org.mozilla.firefox',
+          buildSourceDir: sinon.spy(() => Promise.resolve({
+            extensionPath: fakeBuiltExtensionPath,
+          })),
+          adbRemoveOldArtifacts: false,
+        },
+      };
+      const {
+        params, fakeADBUtils,
+      } = prepareSelectedDeviceAndAPKParams(
+        overriddenProperties, adbOverrides
+      );
+
+      const runnerInstance = new FirefoxAndroidExtensionRunner(params);
+      await runnerInstance.run();
+
+      sinon.assert.calledWithMatch(
+        fakeADBUtils.detectOrRemoveOldArtifacts,
+        'emulator-1',
+        false,
+      );
+
+      // Ensure the old artifacts are checked or removed after stopping the
+      // apk and before creating the new artifacts dir.
+      sinon.assert.callOrder(
+        fakeADBUtils.amForceStopAPK,
+        fakeADBUtils.detectOrRemoveOldArtifacts,
+        fakeADBUtils.getOrCreateArtifactsDir
+      );
+    });
+
+    it('does optionally remove older artifacts dirs', async () => {
+      const adbOverrides = {
+        getOrCreateArtifactsDir: sinon.spy(
+          () => Promise.resolve('/sdcard/web-ext-dir')
+        ),
+        detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(true)),
+      };
+      const overriddenProperties = {
+        params: {
+          adbDevice: 'emulator-1',
+          firefoxApk: 'org.mozilla.firefox',
+          buildSourceDir: sinon.spy(() => Promise.resolve({
+            extensionPath: fakeBuiltExtensionPath,
+          })),
+          adbRemoveOldArtifacts: true,
+        },
+      };
+      const {
+        params, fakeADBUtils,
+      } = prepareSelectedDeviceAndAPKParams(
+        overriddenProperties, adbOverrides
+      );
+
+      const runnerInstance = new FirefoxAndroidExtensionRunner(params);
+      await runnerInstance.run();
+
+      sinon.assert.calledWithMatch(
+        fakeADBUtils.detectOrRemoveOldArtifacts,
+        'emulator-1',
+        true,
+      );
+
+      // Ensure the old artifacts are checked or removed after stopping the
+      // apk and before creating the new artifacts dir.
+      sinon.assert.callOrder(
+        fakeADBUtils.amForceStopAPK,
+        fakeADBUtils.detectOrRemoveOldArtifacts,
+        fakeADBUtils.getOrCreateArtifactsDir
+      );
+    });
+
     it('does run a specific apk component if specific', async () => {
       const {
         params, fakeADBUtils,

tests/unit/test-util/test.adb.js

@@ -3,14 +3,17 @@
 import EventEmitter from 'events';
 
 import chai from 'chai';
-import {describe, it} from 'mocha';
+import {afterEach, describe, it} from 'mocha';
 import sinon from 'sinon';
 
 import {
   UsageError,
   WebExtError,
 } from '../../../src/errors';
-import ADBUtils from '../../../src/util/adb';
+import ADBUtils, {
+  ARTIFACTS_DIR_PREFIX,
+  DEVICE_DIR_BASE,
+} from '../../../src/util/adb';
 
 const fakeADBPackageList = `
 package:org.mozilla.fennec
@@ -64,6 +67,7 @@ function getFakeADBKit(
       return [];
     }),
     shell: sinon.spy(() => Promise.resolve('')),
+    readdir: sinon.spy(() => Promise.resolve([])),
     startActivity: sinon.spy(() => {}),
     forward: sinon.spy(() => {}),
     push: sinon.spy(() => {
@@ -622,6 +626,96 @@ describe('utils/adb', () => {
        });
   });
 
+  describe('detectOrRemoveOldArtifacts', () => {
+    function createFakeReaddirFile(artifactName: string, isDirectory: boolean) {
+      return {
+        name: artifactName,
+        isDirectory: () => {
+          return isDirectory;
+        },
+      };
+    }
+
+    const filesNotArtifactsDirs = [
+      createFakeReaddirFile('not-an-artifact-dir1', true),
+      createFakeReaddirFile('not-a-dir2', false),
+    ];
+
+    const filesArtifactsDirs = [
+      createFakeReaddirFile(`${ARTIFACTS_DIR_PREFIX}1`, true),
+      createFakeReaddirFile(`${ARTIFACTS_DIR_PREFIX}2`, true),
+    ];
+
+    const allFiles = [...filesNotArtifactsDirs, ...filesArtifactsDirs];
+
+    const sb = sinon.createSandbox();
+    const adbkitSpies = {
+      adbClient: {
+        readdir: sb.spy(() => Promise.resolve([])),
+        shell: sb.spy(() => Promise.resolve('')),
+      },
+      adbkitUtil: {
+        readAll: sb.spy(() => Promise.resolve(Buffer.from('1\n'))),
+      },
+    };
+
+    // Reset the fakeADBClient spies after each test case.
+    afterEach(() => sb.reset());
+
+    it('does detect old artifacts directories', async () => {
+      const adb = getFakeADBKit(adbkitSpies);
+      const adbUtils = new ADBUtils({adb});
+      const fakeADB = adb.fakeADBClient;
+
+      fakeADB.readdir = sb.spy(async () => filesNotArtifactsDirs);
+
+      await assert.becomes(
+        adbUtils.detectOrRemoveOldArtifacts('device1', false),
+        false,
+        'Expected to return false when no old artifacts dirs have been found'
+      );
+      sinon.assert.calledOnce(fakeADB.readdir);
+      sinon.assert.calledWith(fakeADB.readdir, 'device1', DEVICE_DIR_BASE);
+      // Expect adbkit shell to never be called when no artifacts have been found.
+      sinon.assert.notCalled(fakeADB.shell);
+
+      adb.fakeADBClient.readdir = sb.spy(async () => allFiles);
+
+      await assert.becomes(
+        adbUtils.detectOrRemoveOldArtifacts('device1', false),
+        true,
+        'Expected to return true when old artifacts dirs have been found'
+      );
+      sinon.assert.notCalled(fakeADB.shell);
+    });
+
+    it('does optionally remove artifacts directories', async () => {
+      const adb = getFakeADBKit(adbkitSpies);
+      const adbUtils = new ADBUtils({adb});
+
+      adb.fakeADBClient.readdir = sb.spy(async () => allFiles);
+
+      await assert.becomes(
+        adbUtils.detectOrRemoveOldArtifacts('device1', true),
+        true,
+        'Expected to return true when old artifacts dirs have been found'
+      );
+
+      sinon.assert.calledOnce(adb.fakeADBClient.readdir);
+      assert.equal(
+        adb.fakeADBClient.shell.callCount,
+        filesArtifactsDirs.length,
+      );
+
+      for (const fakeFile of filesArtifactsDirs) {
+        sinon.assert.calledWithMatch(
+          adb.fakeADBClient.shell, 'device1',
+          ['rm', '-rf', `${DEVICE_DIR_BASE}${fakeFile.name}`]
+        );
+      }
+    });
+  });
+
   describe('pushFile', () => {
     it('rejects an UsageError on adb binary not found', async () => {
       const adb = await testSpawnADBUsageError({