Handle some errors

.github/workflows/ci.yml

@@ -68,7 +68,14 @@ jobs:
           ${{ runner.os }}-build-
           ${{ runner.os }}-
 
+    - name: test-nocover
+      run: |
+        npm install
+        npm run test-nocover
+
+    # only generate coverage on linux
     - name: test
+      if: contains(runner.os, 'Linux')
       run: |
         npm install
         npm run test

package.json

@@ -1,12 +1,12 @@
 {
   "name": "promise-android-tools",
-  "version": "4.0.4",
+  "version": "4.0.5",
   "description": "A wrapper for adb, fastboot, and heimdall that returns convenient promises.",
   "main": "./lib/module.cjs",
   "type": "module",
   "scripts": {
     "test": "npx coverage -x src/cancelable-promise.js -r lcov -r html npm run test-nocover",
-    "test-nocover": "npx mocha -- -R spec './tests/unit-tests/*.js'",
+    "test-nocover": "npx mocha -- -R spec './tests/unit-tests/*.js' './tests/*.js'",
     "lint": "npx eslint tests/**/*.js src/*.js",
     "lint-fix": "npm run lint -- --fix",
     "docs": "npx jsdoc -c jsdoc-config.json",

src/adb.js

@@ -201,7 +201,7 @@ export class Adb extends Tool {
                 if (str.includes("writex")) {
                   // writex namespace indicates external writing
                   pushedSize +=
-                    parseInt(str.split("len=")[1].split(" ")[0]) || 0;
+                    parseInt(str?.split("len=")[1]?.split(" ")[0]) || 0;
                   progress(Math.min(pushedSize / totalSize, 1));
                 }
               } else {

src/common.js

@@ -35,12 +35,3 @@ export function removeFalsy(obj) {
   }
   return Object.keys(obj).length ? obj : null;
 }
-
-/**
- * Add platform-specific quotes to path string (macos can't handle double quotes)
- * @param {String} file path to guard in quotes
- * @returns {String} guarded path
- */
-export function quotepath(file) {
-  return process.platform == "darwin" ? "'" + file + "'" : '"' + file + '"';
-}

src/fastboot.js

@@ -51,7 +51,9 @@ export class Fastboot extends Tool {
       stderr?.includes("Device not unlocked cannot flash or erase") ||
       stderr?.includes("Partition flashing is not allowed") ||
       stderr?.includes("Command not allowed") ||
-      stderr?.includes("device is locked. Cannot flash images")
+      stderr?.includes("not allowed when locked") ||
+      stderr?.includes("device is locked. Cannot flash images") ||
+      stderr?.match(/download for partition '[a-z]+' is not allowed/i)
     ) {
       return "bootloader locked";
     } else if (
@@ -167,7 +169,7 @@ export class Fastboot extends Tool {
    * @returns {Promise}
    */
   boot(image) {
-    return this.exec("boot", common.quotepath(image))
+    return this.exec("boot", image)
       .then(stdout => {
         return;
       })
@@ -183,7 +185,7 @@ export class Fastboot extends Tool {
    * @returns {Promise}
    */
   update(image, wipe) {
-    return this.exec(wipe ? "-w" : "", "update", common.quotepath(image))
+    return this.exec(wipe ? "-w" : "", "update", image)
       .then(stdout => {
         return;
       })

src/heimdall.js

@@ -17,7 +17,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { quotepath } from "./common.js";
 import { Tool } from "./tool.js";
 import { CancelablePromise } from "./cancelable-promise.js";
 
@@ -90,7 +89,7 @@ export class Heimdall extends Tool {
    * @returns {Promise<String>}
    */
   printPit(file) {
-    return this.exec("print-pit", ...(file ? ["--file", quotepath(file)] : []))
+    return this.exec("print-pit", ...(file ? ["--file", file] : []))
       .then(r =>
         r
           .split("\n\nEnding session...")[0]

src/tool.js

@@ -115,7 +115,6 @@ export class Tool extends EventEmitter {
       this.executable,
       [...this.extra, ...args].flat(),
       {
-        shell: true,
         env: {
           ADB_TRACE: "rwx"
         }

tests/test-data/fake fileaccessor.js

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+import fs from "fs";
+
+const [, ...args] = process.argv;
+console.log(fs.statSync(args[1]));

tests/test-data/fake_fileaccessor.js

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+import fs from "fs";
+
+const [, ...args] = process.argv;
+console.log(fs.statSync(args[1]));

tests/test-data/known_errors.js

@@ -114,6 +114,17 @@ export const fastbootErrors = [
     error: { code: 1 },
     stderr: "FAILED (remote: 'device is locked. Cannot flash images')"
   },
+  {
+    expectedReturn: "bootloader locked",
+    error: { code: 1 },
+    stderr:
+      "FAILED (remote: 'Fastboot command (set_active:) is not allowed when locked')"
+  },
+  {
+    expectedReturn: "bootloader locked",
+    error: { code: 1 },
+    stderr: "FAILED (remote: 'download for partition 'boot' is not allowed')"
+  },
   {
     expectedReturn: "enable unlocking",
     error: { killed: false, code: 1, signal: null, cmd: "command" },

tests/test_platform.js

@@ -0,0 +1,67 @@
+"use strict";
+
+/*
+ * Copyright (C) 2017-2019 UBports Foundation <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import chai from "chai";
+import sinon from "sinon";
+import sinonChai from "sinon-chai";
+const expect = chai.expect;
+chai.use(sinonChai);
+
+import util from "util";
+import { spawn, execFile } from "child_process";
+const execFilePromise = util.promisify(execFile);
+const spawnPromise = (file, args) =>
+  new Promise((resolve, reject) => {
+    const cp = spawn(file, args);
+    cp.on("exit", code => (code ? reject({ code }) : resolve()));
+  });
+
+describe("platform", function() {
+  this.timeout(20000);
+  ["fake_fileaccessor", "fake fileaccessor"].forEach(e => {
+    describe(e, function() {
+      [
+        { f: execFilePromise, n: "execFile" },
+        { f: spawnPromise, n: "spawn" }
+      ].forEach(({ f, n }) => {
+        it(`${n} should access path without spaces`, function() {
+          return f("node", [
+            `tests/test-data/${e}.js`,
+            "tests/test-data/test_file"
+          ]);
+        });
+        it(`${n} should access path with spaces`, function() {
+          return f("node", [
+            `tests/test-data/${e}.js`,
+            "tests/test-data/test file"
+          ]);
+        });
+        it(`${n} should throw on inaccessible path`, function(done) {
+          f("node", [
+            `tests/test-data/${e}.js`,
+            "tests/test-data/thisdoesntexist"
+          ]).catch(error => {
+            expect(error).to.haveOwnProperty("code", 1);
+            done();
+          });
+        });
+      });
+    });
+  });
+});

tests/unit-tests/test_common.js

@@ -26,25 +26,6 @@ chai.use(sinonChai);
 import * as common from "../../src/common.js";
 
 describe("Common module", function() {
-  describe("quotepath()", function() {
-    it("should use correct quotes for the platform", function() {
-      expect(common.quotepath("some/path with/ spaces")).to.equal(
-        process.platform == "darwin"
-          ? "'some/path with/ spaces'"
-          : '"some/path with/ spaces"'
-      );
-    });
-    [
-      { platform: "darwin", q: "'" },
-      { platform: "linux", q: '"' },
-      { platform: "win32", q: '"' }
-    ].forEach(p =>
-      it(`should use ${p.q} on ${p.platform}`, function() {
-        sinon.stub(process, "platform").value(p.platform);
-        expect(common.quotepath("a")).to.eql(`${p.q}a${p.q}`);
-      })
-    );
-  });
   describe("removeFalsy()", function() {
     it("should remove falsy values from an object", function() {
       expect(

tests/unit-tests/test_fastboot.js

@@ -28,7 +28,6 @@ chai.use(chaiAsPromised);
 import child_process from "child_process";
 
 import { Fastboot } from "../../src/module.js";
-import * as common from "../../src/common.js";
 import { getAndroidToolPath } from "android-tools-bin";
 import { fastbootErrors } from "../test-data/known_errors.js";
 
@@ -206,7 +205,7 @@ describe("Fastboot module", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.boot("/path/to/image").then(r => {
-          expectArgs("boot", common.quotepath("/path/to/image"));
+          expectArgs("boot", "/path/to/image");
         });
       });
       it("should reject if booting failed", function() {
@@ -223,22 +222,22 @@ describe("Fastboot module", function() {
       it("should resolve if updating works", function() {
         stubExec();
         const fastboot = new Fastboot();
-        return fastboot.update("/path/to/image").then(r => {
-          expectArgs("", "update", common.quotepath("/path/to/image"));
+        return fastboot.update("/path/to an/image").then(r => {
+          expectArgs("", "update", "/path/to an/image");
         });
       });
       it("should not wipe if not specified", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.update("/path/to/image").then(r => {
-          expectArgs("", "update", common.quotepath("/path/to/image"));
+          expectArgs("", "update", "/path/to/image");
         });
       });
       it("should wipe if specified", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.update("/path/to/image", true).then(r => {
-          expectArgs("-w", "update", common.quotepath("/path/to/image"));
+          expectArgs("-w", "update", "/path/to/image");
         });
       });
       it("should reject if updating fails", function() {

tests/unit-tests/test_heimdall.js

@@ -28,7 +28,6 @@ chai.use(chaiAsPromised);
 import child_process from "child_process";
 
 import { Heimdall } from "../../src/module.js";
-import * as common from "../../src/common.js";
 import { getAndroidToolPath } from "android-tools-bin";
 import { heimdallErrors } from "../test-data/known_errors.js";
 
@@ -178,11 +177,7 @@ describe("Heimdall module", function() {
         const heimdall = new Heimdall();
         return heimdall.printPit("/test/test-data/test_file").then(r => {
           expect(r.length).to.eql(3);
-          expectArgs(
-            "print-pit",
-            "--file",
-            common.quotepath("/test/test-data/test_file")
-          );
+          expectArgs("print-pit", "--file", "/test/test-data/test_file");
         });
       });
       it("should reject on error", function() {