fix: Cleanup old web-ext run artifacts dir when we are connecting to an android device

Author: Vishalghyv

src/cmd/run.js

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

src/extension-runners/firefox-android.js

@@ -71,6 +71,7 @@ export type FirefoxAndroidExtensionRunnerParams = {|
   adbPort?: string,
   adbDevice?: string,
   adbDiscoveryTimeout?: number,
+  adbCleanArtifacts?: boolean,
   firefoxApk?: string,
   firefoxApkComponent?: string,
 
@@ -384,6 +385,30 @@ export class FirefoxAndroidExtensionRunner {
     await adbUtils.amForceStopAPK(selectedAdbDevice, selectedFirefoxApk);
   }
 
+  async adbOldArtifactsDir(removeArtifacts?: boolean) {
+    const {
+      adbUtils,
+      selectedAdbDevice,
+    } = this;
+
+    const foundDirectories = await adbUtils.checkOrCleanArtifacts(
+      selectedAdbDevice, removeArtifacts
+    );
+
+    if (!foundDirectories) {
+      return;
+    }
+    if (removeArtifacts) {
+      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-clean-artifacts to remove them automatically.'
+      );
+    }
+  }
+
   async adbCheckRuntimePermissions() {
     const {
       adbUtils,
@@ -441,6 +466,9 @@ export class FirefoxAndroidExtensionRunner {
       customPrefs,
     });
 
+    //Checking for older artifacts
+    await this.adbOldArtifactsDir(this.params.adbCleanArtifacts);
+
     // 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-clean-artifacts': {
+        describe: 'Remove old artifact 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,8 @@ import {
 } from '../errors';
 import {createLogger} from '../util/logger';
 
+const DEVICE_DIR_BASE = '/sdcard';
+const ARTIFACTS_DIR_PREFIX = '/web-ext-artifacts';
 const log = createLogger(__filename);
 
 export type ADBUtilsParams = {|
@@ -195,7 +197,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 +217,44 @@ export default class ADBUtils {
     return artifactsDir;
   }
 
+  async checkOrCleanArtifacts(
+    deviceId: string, remove?: boolean
+  ): Promise<boolean> {
+    const {adbClient} = this;
+
+    let found = false;
+    log.debug('Finding older artifacts');
+
+    return wrapADBCall(async () => {
+      const files = await adbClient.readdir(deviceId, DEVICE_DIR_BASE);
+
+      for (const file of files) {
+        if (!file.isDirectory() ||
+              !file.name.startsWith('web-ext-artifacts-')) {
+          continue;
+        }
+
+        if (!remove) {
+          return true;
+        }
+
+        found = true;
+
+        const artifactsDir = `${DEVICE_DIR_BASE}/${file.name}`;
+
+        log.debug(
+          `Removing ${artifactsDir} artifacts directory on ${deviceId} device`
+        );
+
+        await this.runShellCommand(deviceId, [
+          'rm', '-rf', artifactsDir,
+        ]);
+      }
+
+      return found;
+    });
+  }
+
   async clearArtifactsDir(deviceId: string): Promise<void> {
     const artifactsDir = this.artifactsDirMap.get(deviceId);
 

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

@@ -130,6 +130,9 @@ function prepareSelectedDeviceAndAPKParams(
     startFirefoxAPK: sinon.spy(() => Promise.resolve()),
     setupForward: sinon.spy(() => Promise.resolve()),
     clearArtifactsDir: sinon.spy(() => Promise.resolve()),
+    checkOrCleanArtifacts: sinon.spy(
+      () => Promise.resolve(true)
+    ),
     setUserAbortDiscovery: sinon.spy(() => {}),
     ensureRequiredAPKRuntimePermissions: sinon.spy(() => Promise.resolve()),
     ...adbOverrides,
@@ -357,6 +360,79 @@ describe('util/extension-runners/firefox-android', () => {
          );
        });
 
+    it('check for older artifacts', async () => {
+      const adbOverrides = {
+        getOrCreateArtifactsDir: sinon.spy(
+          () => Promise.resolve('/sdcard/web-ext-dir')
+        ),
+      };
+      const overriddenProperties = {
+        params: {
+          adbDevice: 'emulator-1',
+          firefoxApk: 'org.mozilla.firefox',
+          buildSourceDir: sinon.spy(() => Promise.resolve({
+            extensionPath: fakeBuiltExtensionPath,
+          })),
+          adbCleanArtifacts: false,
+        },
+      };
+      const {
+        params, fakeADBUtils,
+      } = prepareSelectedDeviceAndAPKParams(
+        overriddenProperties, adbOverrides
+      );
+
+      const runnerInstance = new FirefoxAndroidExtensionRunner(params);
+      await runnerInstance.run();
+
+      sinon.assert.calledWithMatch(
+        fakeADBUtils.checkOrCleanArtifacts,
+        'emulator-1',
+        false,
+      );
+
+      sinon.assert.callOrder(
+        fakeADBUtils.amForceStopAPK,
+        fakeADBUtils.checkOrCleanArtifacts
+      );
+    });
+    it('remove plausible older artifacts', async () => {
+      const adbOverrides = {
+        getOrCreateArtifactsDir: sinon.spy(
+          () => Promise.resolve('/sdcard/web-ext-dir')
+        ),
+      };
+      const overriddenProperties = {
+        params: {
+          adbDevice: 'emulator-1',
+          firefoxApk: 'org.mozilla.firefox',
+          buildSourceDir: sinon.spy(() => Promise.resolve({
+            extensionPath: fakeBuiltExtensionPath,
+          })),
+          adbCleanArtifacts: true,
+        },
+      };
+      const {
+        params, fakeADBUtils,
+      } = prepareSelectedDeviceAndAPKParams(
+        overriddenProperties, adbOverrides
+      );
+
+      const runnerInstance = new FirefoxAndroidExtensionRunner(params);
+      await runnerInstance.run();
+
+      sinon.assert.calledWithMatch(
+        fakeADBUtils.checkOrCleanArtifacts,
+        'emulator-1',
+        true,
+      );
+
+      sinon.assert.callOrder(
+        fakeADBUtils.amForceStopAPK,
+        fakeADBUtils.checkOrCleanArtifacts
+      );
+    });
+
     it('does run a specific apk component if specific', async () => {
       const {
         params, fakeADBUtils,

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

@@ -64,6 +64,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 +623,75 @@ describe('utils/adb', () => {
        });
   });
 
+  describe('checkOrCleanArtifacts', () => {
+    const artifactDir = `web-ext-artifacts-${Date.now()}`;
+    function makeFakeArtifact(artifactName, isDirectory) {
+      return {
+        name: artifactName,
+        isDirectory: () => {
+          return isDirectory;
+        },
+      };
+    }
+    const files = [
+      makeFakeArtifact(artifactDir.concat('fake-dir1'), true),
+      makeFakeArtifact(artifactDir.concat('fake-dir2'), true),
+      makeFakeArtifact(artifactDir.concat('fake-file1'), false),
+      makeFakeArtifact('not-web-ext-artifacts-dir', true),
+    ];
+
+    it('checks old artifacts', async () => {
+      const adb = getFakeADBKit({
+        adbClient: {
+          readdir: sinon.spy(() => Promise.resolve(files)),
+          shell: sinon.spy(() => Promise.resolve('')),
+        },
+        adbkitUtil: {
+          readAll: sinon.spy(() => Promise.resolve(Buffer.from('1\n'))),
+        },
+      });
+      const adbUtils = new ADBUtils({adb});
+
+      const promise = adbUtils.checkOrCleanArtifacts('device1', false);
+      const result = await assert.isFulfilled(promise);
+      assert.equal(result, true);
+
+      sinon.assert.calledOnce(adb.fakeADBClient.readdir);
+      //While checking of files shell shoudln't be called
+      sinon.assert.notCalled(adb.fakeADBClient.shell);
+    });
+
+    it('removes plausible artifacts directory', async () => {
+      const adb = getFakeADBKit({
+        adbClient: {
+          readdir: sinon.spy(() => Promise.resolve(files)),
+          shell: sinon.spy(() => Promise.resolve('')),
+        },
+        adbkitUtil: {
+          readAll: sinon.spy(() => Promise.resolve(Buffer.from('1\n'))),
+        },
+      });
+      const adbUtils = new ADBUtils({adb});
+      const artifactDirFullPath1 = `/sdcard/${artifactDir}fake-dir1`;
+      const artifactDirFullPath2 = `/sdcard/${artifactDir}fake-dir2`;
+
+      const promise = adbUtils.checkOrCleanArtifacts('device1', true);
+      const result = await assert.isFulfilled(promise);
+      assert.equal(result, true);
+
+      sinon.assert.calledOnce(adb.fakeADBClient.readdir);
+      sinon.assert.calledTwice(adb.fakeADBClient.shell);
+      sinon.assert.calledWithMatch(
+        adb.fakeADBClient.shell, 'device1',
+        ['rm', '-rf', artifactDirFullPath1]
+      );
+      sinon.assert.calledWithMatch(
+        adb.fakeADBClient.shell, 'device1',
+        ['rm', '-rf', artifactDirFullPath2]
+      );
+    });
+  });
+
   describe('pushFile', () => {
     it('rejects an UsageError on adb binary not found', async () => {
       const adb = await testSpawnADBUsageError({