feat: new tests added for github downloads and refactoring to suit

lib/downloader.js

@@ -6,6 +6,7 @@ const fs            = require('fs');
 const fsextra       = require('fs-extra');
 const path          = require('path');
 const tmp           = require('tmp');
+const util          = require('util');
 
 class Downloader {
   constructor(opts) {
@@ -103,21 +104,25 @@ class Downloader {
 
   download_from_github(firmware, options) {
     // downloads the file from github and returns the path to the hex file
-    return new Promise(async(resolve, reject) => {
+    // as well as the temp directory it exists in to clean it up
+    let base_uri = null;
+    return new Promise((resolve, reject) => { // need to try and reolve this async out
       // setup GH
 
       if (firmware === undefined) {
         reject(new Error('No firmware provided'));
       }
 
-      console.info(colors.magenta(`Installing ${firmware.name} from github`));
+      /* istanbul ignore next */
+      if (!process.env.TEST) {
+        console.info(colors.magenta(`Installing ${firmware.name} from github`));
+      }
       // work out where the manifest file is
       if (firmware.repo === undefined) {
         reject(new Error('No github repo specified'));
       }
 
       let manifest_uri = null;
-      let base_uri = null;
       let branch = 'master';
       let repo = '';
 
@@ -138,36 +143,50 @@ class Downloader {
         reject(new Error('Wrong protocol used for github'));
       }
 
-      const manifest = await this.get_file_from_github(manifest_uri);
+      this.get_file_from_github(manifest_uri)
+        .then((manifest) => resolve(manifest))
+        .catch(err => reject(err));
+    }).then((manifest) => {
+      // now we process the manifest file, download the hex binary and then
+      // save it and return a path to it.
       const bin_path = this.get_path_from_manifest(manifest, firmware, options);
       const bin_uri = `${base_uri}${bin_path}?${(new Date().getTime())}`;
 
       // save the hex file
-      this.get_file_from_github(bin_uri)
+      return this.get_file_from_github(bin_uri)
         .then(hexdata => {
           const tmpdir = tmp.dirSync();
           const hexpath = `${tmpdir.name}/bin.hex`;
 
-          fs.writeFile(hexpath, hexdata, (err) => {
-            if (err) {
+          // turn the write file into a promise.
+          const writeFile = util.promisify(fs.writeFile);
+
+          // now write the file back.
+          return writeFile(hexpath, hexdata)
+            .then(() => {
+              return {hexpath, tmpdir};
+            })
+            .catch(err => {
               fsextra.removeSync(tmpdir.name);
               tmpdir.removeCallback();
-              reject(err);
-            }
-
-            // send this back as an object
-            resolve({hexpath, tmpdir});
-          });
+              throw err;
+            });
         })
-        .catch(err => reject(err));
+        .catch(err => {
+          throw err;
+        });
     });
   }
 
   download_from_npm(firmware, options) {
     // downloads the file from npm and returns the path to the hex file.
     return new Promise((resolve, reject) => {
       // Set up NPM.
-      console.info(colors.magenta('Installing ' + firmware.name + ' from npm'));
+      /* istanbul ignore next */
+      if (!process.env.TEST) {
+        console.info(colors.magenta('Installing ' + firmware.name + ' from npm'));
+      }
+
       // first try to install the npm package
       let command = 'npm install --no-save ';
 

test/downloader.test.js

@@ -1,3 +1,5 @@
+const fs            = require('fs');
+const fsextra       = require('fs-extra');
 const child_process = require('child_process');
 jest.mock('child_process');
 
@@ -141,30 +143,100 @@ const npm_actions = () => describe('3. NPM related actions for the downloader',
 
 const github_actions = () => describe('4. Github related actions for the downloader', () => {
   // test actions relating to the GH way of getting the files
+  let dl;
+  let temppath;
+  beforeEach(() => {
+    jest.resetModules();
+    dl = new Downloader();
+  });
+
+  afterEach(() => {
+    // clean up temp path if it's been set
+    if (temppath) {
+      console.log('cleaning up: ', temppath.name);
+      fsextra.removeSync(temppath.name);
+      temppath.removeCallback();
+      temppath = null;
+    }
+  });
+
   test('4.1 Getting GH manifest fails if no repo supplied', () => {
-    const dl = new Downloader();
+    // const dl = new Downloader();
     expect(dl.download_from_github()).rejects.toThrow(/firmware/);
   });
 
   test('4.2 getting GH manifest fails if GH configuration is wrong', () => {
+    // tests no repo provided and no git+ssh on path
     const {gh_fw2, gh_fw3} = data;
-    const dl = new Downloader();
+    // const dl = new Downloader();
     expect(dl.download_from_github(gh_fw2)).rejects.toThrow(/github/);
     expect(dl.download_from_github(gh_fw3)).rejects.toThrow(/protocol/);
   });
 
-  test('4.3 Reolve manifest from GitHub if git branch is supplied', async() => {
-    const {gh_branch_fw, manifest, options} = data;
-    const dl = new Downloader();
+  test('4.3 Resolve manifest from GitHub if git branch is supplied', (done) => {
+    // const dl4 = new Downloader();
+    const {manifest, gh_branch_fw, options} = data;
+
+    const mock_download = jest.fn()
+      .mockResolvedValue(manifest)
+      .mockResolvedValueOnce(manifest)
+      .mockResolvedValueOnce('data');
+
+    dl.get_file_from_github = mock_download;
+
+    expect.assertions(3);
+    return dl.download_from_github(gh_branch_fw, options)
+      .then((m) => {
+        expect(dl.get_file_from_github).toHaveBeenCalled();
+        expect(m.hexpath).toBeDefined();
+        expect(m.tmpdir).toBeDefined();
+        // clean up tmp files
+        temppath = m.tmpdir;
+        done();
+      });
+  });
+
+  test('4.4 Resolve manifest file if branch not supplied', (done) => {
+    // this tests using master branch.
+    const {manifest, gh_master_branch_fw, options} = data;
+
     const mock_download = jest.fn()
       .mockResolvedValue(manifest)
       .mockResolvedValueOnce(manifest)
       .mockResolvedValueOnce('data');
 
     dl.get_file_from_github = mock_download;
-    const m = await dl.download_from_github(gh_branch_fw, options);
-    // TODO why is this calling the old get file rather than the mocked one...?:w
-    expect(dl.get_file_from_github).toHaveBeenCalled();
+
+    expect.assertions(3);
+    return dl.download_from_github(gh_master_branch_fw, options)
+      .then((m) => {
+        expect(dl.get_file_from_github).toHaveBeenCalled();
+        expect(m.hexpath).toBeDefined();
+        expect(m.tmpdir).toBeDefined();
+        // clean up tmp files
+        temppath = m.tmpdir;
+        done();
+      });
+  });
+
+  test('4.5 Handle not receiving the hex file from download', (done) => {
+    // this tests using master branch.
+    const {manifest, gh_master_branch_fw, options} = data;
+
+    const mock_download = jest.fn()
+      .mockResolvedValue(manifest)
+      .mockResolvedValueOnce(manifest)
+      .mockRejectedValueOnce(new Error('Bin download error'));
+
+    dl.get_file_from_github = mock_download;
+
+    expect.assertions(2);
+    return dl.download_from_github(gh_master_branch_fw, options)
+      .catch(err => {
+        expect(dl.get_file_from_github).toHaveBeenCalled();
+        expect(err.toString()).toMatch(/download/);
+        done();
+      });
   });
 });