Support for short ids

Author: k1o0

CHANGELOG.md

@@ -8,7 +8,7 @@
 ## Modified
 
  - removed old dependencies
- 
+ - support for short ids and branch names for all endpoints
 
 ## [2.1.0]
 

README.md

@@ -1,12 +1,12 @@
 # MATLAB-ci
 [![Build Status](https://travis-ci.com/cortex-lab/matlab-ci.svg?branch=master)](https://travis-ci.com/cortex-lab/matlab-ci)
-[![Coverage](https://img.shields.io/badge/coverage-72.35-yellowgreen)](https://img.shields.io/badge/coverage-72.35-yellowgreen)
+[![Coverage](https://img.shields.io/badge/coverage-81.07-green)](https://img.shields.io/badge/coverage-72.35-yellowgreen)
 
 A small set of modules written in Node.js for running automated tests of MATLAB code in response to GitHub events.  Also submits code coverage to the Coveralls API.
 
 Currently unsupported:
 * Running tests on forked repositories
-* Testing multiple repos (unless they are submodules) 
+* Testing multiple repos (unless they are submodules)
 
 ## Getting Started
 

lib.js

@@ -330,7 +330,7 @@ function compareCoverage(job) {
   }
   log('Comparing coverage for %g -> %g', job.data.sha, job.data.base);
   var records;
-  if (!job.coverage) {
+  if (!job.data.coverage) {
     log('No coverage in job data; loading from records');
     records = loadTestRecords([job.data.sha, job.data.base]);
     // Filter duplicates just in case
@@ -341,7 +341,7 @@ function compareCoverage(job) {
     records = [curr, loadTestRecords(job.data.base)];
   }
   log('The following records were found: %O', records);
-  const has_coverage = records.every(o => (typeof o.coverage !== 'undefined' && o.coverage > 0));
+  const hasCoverage = records.every(o => (o.coverage > 0));
 
   // Check if any errored or failed to update coverage
   if (records.filter(o => o.status === 'error').length > 0) {
@@ -350,7 +350,7 @@ function compareCoverage(job) {
     job.data.description = 'Failed to determine coverage as tests incomplete due to errors';
 
   // Both records present and they have coverage
-  } else if (records.length === 2 && has_coverage) {
+  } else if (records.length === 2 && hasCoverage) {
     log('Calculating coverage difference');
     // Ensure first record is for head commit
     if (records[0].commit === job.data.base) { records.reverse() }

serve.js

@@ -117,6 +117,45 @@ srv.post('/github', async (req, res, next) => {
 
 ///////////////////// STATUS DETAILS /////////////////////
 
+/**
+ * Serve the test records for requested commit id.  Returns JSON data for the commit.
+ * @param {string} id - A commit SHA of any length, or branch name.
+ * @param {boolean|null} [isBranch] - If true, id treated as a branch name. Inferred from id by default.
+ * @param {string} [module] - (Sub)module name. REPO_NAME by default.
+ * @return {Promise} - Resolved to full commit SHA.
+ */
+function fetchCommit(id, isBranch=null, module) {
+   isBranch = isBranch === null ? !lib.isSHA(id) : isBranch
+   const data = {
+      owner: process.env['REPO_OWNER'],
+      repo: module || process.env.REPO_NAME,
+      id: id
+   };
+   let endpoint = `GET /repos/:owner/:repo/${isBranch ? 'branches': 'commits'}/:id`;
+   return request(endpoint, data).then(response => {
+      return isBranch ? response.data.commit.sha : response.data.sha;
+   });
+}
+
+/**
+ * Parse the short SHA or branch name and redirect to static reports directory.
+ */
+srv.get(`/coverage/:id`, (req, res) => {
+   let id = lib.shortID(req.params.id);
+   let isSHA = (req.query.branch || !lib.isSHA(req.params.id)) === false;
+   console.log('Request for test coverage for ' + (isSHA? `commit ${id}` : `branch ${req.params.id}`));
+   fetchCommit(req.params.id, !isSHA, req.query.module)
+      .then(id => {
+         log('Commit ID found: %s', id);
+         res.redirect(301, `/${ENDPOINT}/coverage/${id}`);
+      })
+      .catch(err => {
+         log('%s', err.message);
+    	   res.statusCode = 404;
+    	   res.send(`Coverage for ${isSHA? 'commit' : 'branch'} ${req.params.id} not found`);
+      });
+})
+
 /**
  * Serve the reports tree as a static resource; allows users to inspect the HTML coverage reports.
  * We will add a link to the reports in the check details.
@@ -130,23 +169,16 @@ srv.get(`/${ENDPOINT}/records/:id`, function (req, res) {
    let id = lib.shortID(req.params.id);
    let isSHA = (req.query.branch || !lib.isSHA(req.params.id)) === false;
    console.log('Request for test records for ' + (isSHA? `commit ${id}` : `branch ${req.params.id}`));
-   const data = {
-      owner: process.env['REPO_OWNER'],
-      repo: req.query.module || process.env.REPO_NAME,
-      id: req.params.id
-   };
-   let endpoint = `GET /repos/:owner/:repo/${isSHA ? 'commits' : 'branches'}/:id`;
-   request(endpoint, data)
-      .then(response => {
-         let id = isSHA ? response.data.sha : response.data.commit.sha;
+   fetchCommit(req.params.id, !isSHA, req.query.module)
+      .then(id => {
          log('Commit ID found: %s', id);
          let record = lib.loadTestRecords(id);
          if (record) {
             res.setHeader('Content-Type', 'application/json');
             res.end(JSON.stringify(record));
          } else {
             res.statusCode = 404;
-    	      res.send(`${isSHA? 'Commit' : 'Branch'} ${req.params.id} not recognized.`);
+    	      res.send(`${isSHA? 'Commit' : 'Branch'} ${id} not recognized.`);
          }
       })
       .catch(err => {
@@ -163,27 +195,35 @@ srv.get(`/${ENDPOINT}/records/:id`, function (req, res) {
  */
 srv.get(`/${ENDPOINT}/:id`, function (req, res) {
    let id = lib.shortID(req.params.id);
-   let isSHA = lib.isSHA(req.params.id);
    let log_only = (req.query.type || '').startsWith('log')
+   let isSHA = (req.query.branch || !lib.isSHA(req.params.id)) === false;
    console.log(
       `Request for test ${log_only ? 'log' : 'stdout'} for ` +
       (isSHA? `commit ${id}` : `branch ${req.params.id}`)
    );
-   let filename = log_only? `test_output.log` : `std_output-${id}.log`;
-   let logFile = path.join(config.dataPath, 'reports', req.params.id, filename);
-   fs.readFile(logFile, 'utf8', (err, data) => {
-      if (err) {
+   fetchCommit(req.params.id, !isSHA, req.query.module)
+      .then(id => {
+         let filename = log_only? `test_output.log` : `std_output-${lib.shortID(req.params.id)}.log`;
+         let logFile = path.join(config.dataPath, 'reports', id, filename);
+         fs.readFile(logFile, 'utf8', (err, data) => {
+            if (err) {
+               log('%s', err.message);
+               res.statusCode = 404;
+               res.send(`Record for ${isSHA? 'commit' : 'branch'} ${id} not found`);
+            } else {
+               res.statusCode = 200;
+               // Wrap in HTML tags so that the formatting is a little nicer.
+               let preText = '<html lang="en-GB"><body><pre>';
+               let postText = '</pre></body></html>';
+               res.send(preText + data + postText);
+            }
+         });
+      })
+      .catch(err => {
          log('%s', err.message);
     	   res.statusCode = 404;
     	   res.send(`Record for ${isSHA? 'commit' : 'branch'} ${req.params.id} not found`);
-      } else {
-    	   res.statusCode = 200;
-    	   // Wrap in HTML tags so that the formatting is a little nicer.
-    	   let preText = '<html lang="en-GB"><body><pre>';
-    	   let postText = '</pre></body></html>';
-         res.send(preText + data + postText);
-      }
-   });
+      });
 });
 
 
@@ -367,15 +407,15 @@ async function updateStatus(data, targetURL = '') {
    await setAccessToken();
    return request("POST /repos/:owner/:repo/statuses/:sha", {
       owner: data['owner'] || process.env['REPO_OWNER'],
-      repo: data['repo'],
+      repo: data['repo'] || process.env['REPO_NAME'],
       headers: {
          authorization: `token ${token['token']}`,
          accept: "application/vnd.github.machine-man-preview+json"
       },
       sha: data['sha'],
       state: data['status'],
       target_url: targetURL,
-      description: data['description'].substring(0, maxN),
+      description: (data['description'] || '').substring(0, maxN),
       context: data['context']
     });
 }
@@ -547,5 +587,5 @@ queue.on('finish', (err, job) => { // On job end post result to API
 });
 
 module.exports = {
-   updateStatus, srv, handler, setAccessToken, prepareEnv, runTests, eventCallback
+   updateStatus, srv, handler, setAccessToken, prepareEnv, runTests, eventCallback, fetchCommit
 }

test/lib.test.js

@@ -60,6 +60,81 @@ describe('Test partial:', function() {
 });
 
 
+/**
+ * A test for the function getRepoPath
+ */
+describe('Test getRepoPath:', function() {
+    it('expect returned from env', function () {
+        let repoPath = lib.getRepoPath()
+        expect(repoPath).eq(process.env.REPO_PATH)
+    });
+});
+
+
+/**
+ * A test for the function compareCoverage.
+ * @todo add test for strict compare
+ */
+describe('Test compareCoverage:', function() {
+   var job;
+
+   beforeEach(function () {
+      queue.process(async (_job, _done) => {
+      });  // nop
+      queue.pile = [];
+      job = {
+         data: {
+            sha: null
+         }
+      };
+   })
+
+   it('expect coverage diff', function () {
+      // Test decrease in coverage
+      job.data.sha = ids[0];
+      job.data.base = ids[1];
+      lib.compareCoverage(job);
+      expect(job.data.status).eq('failure');
+      expect(job.data.description).contains('decreased');
+      expect(queue.pile).empty;
+
+      // Test increase in coverage
+      job.data.coverage = 95.56
+      lib.compareCoverage(job);
+      expect(job.data.status).eq('success');
+      expect(job.data.description).contains('increased');
+      expect(queue.pile).empty;
+   });
+
+   it('expect ReferenceError', function () {
+      job.data.base = null;
+      expect(() => lib.compareCoverage(job)).throws(ReferenceError);
+   });
+
+   it('expect fail status', function () {
+      job.data.sha = ids[0];
+      job.data.base = ids[3];  // errored
+      lib.compareCoverage(job);
+      expect(job.data.status).eq('failure');
+      expect(job.data.description).contains('incomplete');
+      expect(queue.pile).empty;
+   });
+
+   it('expect job added', function () {
+      // Test decrease in coverage
+      job.data.sha = ids[2];  // fake
+      job.data.base = ids[1];
+      job.data.context = 'coverage';
+      lib.compareCoverage(job);
+      expect(queue.pile.length).eq(2);
+      expect(job.data.skipPost).true;  // Job should be skipped to allow time for jobs to run
+      expect(queue.pile[0].data.sha).eq(ids[1])
+      expect(queue.pile[1].data.skipPost).false;
+      expect(queue.pile[1].data.context).eq(job.data.context)
+   });
+});
+
+
 /**
  * A test for the function updateJobFromRecord.
  * @todo add test for compareCoverage call