From a2e4731f9470dbbd1fa912e74ef5534a039e7b53 Mon Sep 17 00:00:00 2001 From: Vishhalghyv Date: Sat, 21 Mar 2020 19:56:50 +0530 Subject: [PATCH 1/2] fix: Cleanup old web-ext run artifacts dir when we are connecting to an android device --- src/cmd/run.js | 3 + src/extension-runners/firefox-android.js | 28 +++++++ src/program.js | 5 ++ src/util/adb.js | 42 +++++++++- .../test.firefox-android.js | 76 +++++++++++++++++++ tests/unit/test-util/test.adb.js | 70 +++++++++++++++++ 6 files changed, 223 insertions(+), 1 deletion(-) diff --git a/src/cmd/run.js b/src/cmd/run.js index c621ce1151..d7880c469c 100644 --- a/src/cmd/run.js +++ b/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, diff --git a/src/extension-runners/firefox-android.js b/src/extension-runners/firefox-android.js index 4584633d76..201930d1ae 100644 --- a/src/extension-runners/firefox-android.js +++ b/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( diff --git a/src/program.js b/src/program.js index 708a2abe9f..3ea4624542 100644 --- a/src/program.js +++ b/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. ' + diff --git a/src/util/adb.js b/src/util/adb.js index 77315022e6..3b8d5822ef 100644 --- a/src/util/adb.js +++ b/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 { + 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 { const artifactsDir = this.artifactsDirMap.get(deviceId); diff --git a/tests/unit/test-extension-runners/test.firefox-android.js b/tests/unit/test-extension-runners/test.firefox-android.js index 92c724b083..a032d5ba71 100644 --- a/tests/unit/test-extension-runners/test.firefox-android.js +++ b/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, diff --git a/tests/unit/test-util/test.adb.js b/tests/unit/test-util/test.adb.js index 4506d4b714..d89088d619 100644 --- a/tests/unit/test-util/test.adb.js +++ b/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({ From 011dffe533fcb58baf00d6493487e2e9c6010968 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 20 Jul 2020 12:49:27 +0200 Subject: [PATCH 2/2] chore: Renamed cli option to --adb-remove-old-artifacts and some minor tweaks to the related tests --- src/cmd/run.js | 6 +- src/extension-runners/firefox-android.js | 46 +++---- src/program.js | 4 +- src/util/adb.js | 33 +++-- .../test.firefox-android.js | 29 +++-- tests/unit/test-util/test.adb.js | 122 +++++++++++------- 6 files changed, 131 insertions(+), 109 deletions(-) diff --git a/src/cmd/run.js b/src/cmd/run.js index d7880c469c..d656d826d8 100644 --- a/src/cmd/run.js +++ b/src/cmd/run.js @@ -45,7 +45,7 @@ export type CmdRunParams = {| adbPort?: string, adbDevice?: string, adbDiscoveryTimeout?: number, - adbCleanArtifacts?: boolean, + adbRemoveOldArtifacts?: boolean, firefoxApk?: string, firefoxApkComponent?: string, @@ -88,7 +88,7 @@ export default async function run( adbPort, adbDevice, adbDiscoveryTimeout, - adbCleanArtifacts, + adbRemoveOldArtifacts, firefoxApk, firefoxApkComponent, // Chromium CLI options. @@ -167,7 +167,7 @@ export default async function run( adbPort, adbBin, adbDiscoveryTimeout, - adbCleanArtifacts, + adbRemoveOldArtifacts, // Injected dependencies. firefoxApp, diff --git a/src/extension-runners/firefox-android.js b/src/extension-runners/firefox-android.js index 201930d1ae..aca3a10448 100644 --- a/src/extension-runners/firefox-android.js +++ b/src/extension-runners/firefox-android.js @@ -71,7 +71,7 @@ export type FirefoxAndroidExtensionRunnerParams = {| adbPort?: string, adbDevice?: string, adbDiscoveryTimeout?: number, - adbCleanArtifacts?: boolean, + adbRemoveOldArtifacts?: boolean, firefoxApk?: string, firefoxApkComponent?: string, @@ -385,30 +385,6 @@ 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, @@ -456,6 +432,7 @@ export class FirefoxAndroidExtensionRunner { params: { customPrefs, firefoxApp, + adbRemoveOldArtifacts, }, } = this; // Create the preferences file and the Fennec temporary profile. @@ -466,8 +443,23 @@ export class FirefoxAndroidExtensionRunner { customPrefs, }); - //Checking for older artifacts - await this.adbOldArtifactsDir(this.params.adbCleanArtifacts); + // 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. diff --git a/src/program.js b/src/program.js index 3ea4624542..a8042eec0c 100644 --- a/src/program.js +++ b/src/program.js @@ -663,8 +663,8 @@ Example: $0 --help run. type: 'number', requiresArg: true, }, - 'adb-clean-artifacts': { - describe: 'Remove old artifact directories from the adb device', + 'adb-remove-old-artifacts': { + describe: 'Remove old artifacts directories from the adb device', demandOption: false, type: 'boolean', }, diff --git a/src/util/adb.js b/src/util/adb.js index 3b8d5822ef..df21118157 100644 --- a/src/util/adb.js +++ b/src/util/adb.js @@ -8,8 +8,9 @@ import { } from '../errors'; import {createLogger} from '../util/logger'; -const DEVICE_DIR_BASE = '/sdcard'; -const ARTIFACTS_DIR_PREFIX = '/web-ext-artifacts'; +export const DEVICE_DIR_BASE = '/sdcard/'; +export const ARTIFACTS_DIR_PREFIX = 'web-ext-artifacts-'; + const log = createLogger(__filename); export type ADBUtilsParams = {| @@ -197,7 +198,7 @@ export default class ADBUtils { return artifactsDir; } - artifactsDir = `${DEVICE_DIR_BASE}${ARTIFACTS_DIR_PREFIX}-${Date.now()}`; + artifactsDir = `${DEVICE_DIR_BASE}${ARTIFACTS_DIR_PREFIX}${Date.now()}`; const testDirOut = (await this.runShellCommand( deviceId, `test -d ${artifactsDir} ; echo $?` @@ -217,38 +218,38 @@ export default class ADBUtils { return artifactsDir; } - async checkOrCleanArtifacts( - deviceId: string, remove?: boolean + async detectOrRemoveOldArtifacts( + deviceId: string, removeArtifactDirs?: boolean = false ): Promise { const {adbClient} = this; - let found = false; - log.debug('Finding older artifacts'); + 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('web-ext-artifacts-')) { + !file.name.startsWith(ARTIFACTS_DIR_PREFIX)) { continue; } - if (!remove) { + // 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}`; + const artifactsDir = `${DEVICE_DIR_BASE}${file.name}`; log.debug( - `Removing ${artifactsDir} artifacts directory on ${deviceId} device` + `Removing artifacts directory ${artifactsDir} from device ${deviceId}` ); - await this.runShellCommand(deviceId, [ - 'rm', '-rf', artifactsDir, - ]); + await this.runShellCommand(deviceId, ['rm', '-rf', artifactsDir]); } return found; @@ -269,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( diff --git a/tests/unit/test-extension-runners/test.firefox-android.js b/tests/unit/test-extension-runners/test.firefox-android.js index a032d5ba71..58e0c55805 100644 --- a/tests/unit/test-extension-runners/test.firefox-android.js +++ b/tests/unit/test-extension-runners/test.firefox-android.js @@ -130,9 +130,7 @@ function prepareSelectedDeviceAndAPKParams( startFirefoxAPK: sinon.spy(() => Promise.resolve()), setupForward: sinon.spy(() => Promise.resolve()), clearArtifactsDir: sinon.spy(() => Promise.resolve()), - checkOrCleanArtifacts: sinon.spy( - () => Promise.resolve(true) - ), + detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(true)), setUserAbortDiscovery: sinon.spy(() => {}), ensureRequiredAPKRuntimePermissions: sinon.spy(() => Promise.resolve()), ...adbOverrides, @@ -360,11 +358,12 @@ describe('util/extension-runners/firefox-android', () => { ); }); - it('check for older artifacts', async () => { + 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: { @@ -373,7 +372,7 @@ describe('util/extension-runners/firefox-android', () => { buildSourceDir: sinon.spy(() => Promise.resolve({ extensionPath: fakeBuiltExtensionPath, })), - adbCleanArtifacts: false, + adbRemoveOldArtifacts: false, }, }; const { @@ -386,21 +385,26 @@ describe('util/extension-runners/firefox-android', () => { await runnerInstance.run(); sinon.assert.calledWithMatch( - fakeADBUtils.checkOrCleanArtifacts, + 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.checkOrCleanArtifacts + fakeADBUtils.detectOrRemoveOldArtifacts, + fakeADBUtils.getOrCreateArtifactsDir ); }); - it('remove plausible older artifacts', async () => { + + 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: { @@ -409,7 +413,7 @@ describe('util/extension-runners/firefox-android', () => { buildSourceDir: sinon.spy(() => Promise.resolve({ extensionPath: fakeBuiltExtensionPath, })), - adbCleanArtifacts: true, + adbRemoveOldArtifacts: true, }, }; const { @@ -422,14 +426,17 @@ describe('util/extension-runners/firefox-android', () => { await runnerInstance.run(); sinon.assert.calledWithMatch( - fakeADBUtils.checkOrCleanArtifacts, + 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.checkOrCleanArtifacts + fakeADBUtils.detectOrRemoveOldArtifacts, + fakeADBUtils.getOrCreateArtifactsDir ); }); diff --git a/tests/unit/test-util/test.adb.js b/tests/unit/test-util/test.adb.js index d89088d619..4134103022 100644 --- a/tests/unit/test-util/test.adb.js +++ b/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 @@ -623,9 +626,8 @@ describe('utils/adb', () => { }); }); - describe('checkOrCleanArtifacts', () => { - const artifactDir = `web-ext-artifacts-${Date.now()}`; - function makeFakeArtifact(artifactName, isDirectory) { + describe('detectOrRemoveOldArtifacts', () => { + function createFakeReaddirFile(artifactName: string, isDirectory: boolean) { return { name: artifactName, isDirectory: () => { @@ -633,62 +635,84 @@ describe('utils/adb', () => { }, }; } - 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), + + const filesNotArtifactsDirs = [ + createFakeReaddirFile('not-an-artifact-dir1', true), + createFakeReaddirFile('not-a-dir2', false), ]; - 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 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; - const promise = adbUtils.checkOrCleanArtifacts('device1', false); - const result = await assert.isFulfilled(promise); - assert.equal(result, true); + fakeADB.readdir = sb.spy(async () => filesNotArtifactsDirs); - sinon.assert.calledOnce(adb.fakeADBClient.readdir); - //While checking of files shell shoudln't be called - sinon.assert.notCalled(adb.fakeADBClient.shell); + 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('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'))), - }, - }); + it('does optionally remove artifacts directories', async () => { + const adb = getFakeADBKit(adbkitSpies); 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); + adb.fakeADBClient.readdir = sb.spy(async () => allFiles); - sinon.assert.calledOnce(adb.fakeADBClient.readdir); - sinon.assert.calledTwice(adb.fakeADBClient.shell); - sinon.assert.calledWithMatch( - adb.fakeADBClient.shell, 'device1', - ['rm', '-rf', artifactDirFullPath1] + await assert.becomes( + adbUtils.detectOrRemoveOldArtifacts('device1', true), + true, + 'Expected to return true when old artifacts dirs have been found' ); - sinon.assert.calledWithMatch( - adb.fakeADBClient.shell, 'device1', - ['rm', '-rf', artifactDirFullPath2] + + 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}`] + ); + } }); });