Merge pull request #48 from cortex-lab/dev

2.1

Author: k1o0

CHANGELOG.md

@@ -1,6 +1,19 @@
 # Changelog
 
-## [Latest](https://github.com/cortex-lab/matlab-ci/commits/master) [2.0.0]
+## [Latest](https://github.com/cortex-lab/matlab-ci/commits/master) [2.1.0]
+
+### Modified
+ - More generic handling of submodules
+ - Fix for computing coverage properly
+ - Fix to issue #43 
+ - Fix for issue where jobs added when already on pile
+ - New tests added for listSubmodules
+ - listSubmodules and gerRepoPath now exposed in lib
+ - Removed chai-spies dependency 
+
+
+## [2.0.0]
+
 ### Added
 
  - there are now three major modules: lib, serve and main

README.md

@@ -50,6 +50,7 @@ Some extra optional settings:
 - `events:event:ref_include` - same as `ref_ignore`, but a pass list instead of block list.
 - `kill_children` - if present and true, `tree-kill` is used to kill the child processes, required 
 if shell/batch script forks test process (e.g. a batch script calls python).
+- `repos` - an array of submodules or map of modules to their corresponding paths.
 
 Finally, ensure these scripts are executable by node:
 ```

coverage.js

@@ -33,6 +33,7 @@ var token = process.env.COVERALLS_TOKEN;
  * Loads file containing source code, returns a hash and line count
  * @param {String} path - Path to the source code file.
  * @returns {Object} key `Hash` contains MD5 digest string of file; `count` contains number of lines in source file
+ * @todo Make asynchronous
  */
 function md5(path) {
   var hash = crypto.createHash('md5'); // Creating hash object
@@ -58,12 +59,13 @@ function md5(path) {
 function formatCoverage(classList, srcPath, sha) {
   var job = {};
   var sourceFiles = [];
+  var digest;
   srcPath = typeof srcPath != "undefined" ? srcPath : process.env.HOMEPATH; // default to home dir
   // For each class, create file object containing array of lines covered and add to sourceFile array
   classList.forEach( async c => {
     let file = {}; // Initialize file object
     let fullPath = c.$.filename.startsWith(srcPath)? c.$.filename : path.join(srcPath, c.$.filename);
-    var digest = md5(fullPath); // Create digest and line count for file // FIXME use path lib
+    digest = md5(fullPath); // Create digest and line count for file
     let lines = new Array(digest.count).fill(null); // Initialize line array the size of source code file
     c.lines[0].line.forEach(ln => {
       let n = Number(ln.$.number);
@@ -92,19 +94,18 @@ function formatCoverage(classList, srcPath, sha) {
  * @param {String} path - Path to the XML file containing coverage information.
  * @param {String} sha - The commit SHA for this coverage test
  * @param {String} repo - The repo to which the commit belongs
+ * @param {Array} submodules - A list of submodules for separating coverage into
  * @param {function} callback - The callback function to run when complete
- * @todo Generalize ignoring of submodules
  * @see {@link https://github.com/cobertura/cobertura/wiki|Cobertura Wiki}
  */
-function coverage(path, repo, sha, callback) {
+function coverage(path, repo, sha, submodules, callback) {
   cb = callback; // @fixme Making callback global feels hacky
   fs.readFile(path, function(err, data) { // Read in XML file
-    // @fixme deal with file not found errors
-    if (err) {throw err}
+    if (err) {throw err}  // @fixme deal with file not found errors
     parser.parseString(data, function (err, result) { // Parse XML
         // Extract root code path
         const rootPath = (result.coverage.sources[0].source[0] || process.env.REPO_PATH).replace(/[\/|\\]+$/, '')
-        assert(rootPath.toLowerCase().endsWith(repo || process.env.REPO_NAME), 'Incorrect source code repository')
+        assert(rootPath.endsWith(process.env.REPO_NAME), 'Incorrect source code repository')
         timestamp = new Date(result.coverage.$.timestamp*1000); // Convert UNIX timestamp to Date object
         let classes = []; // Initialize classes array
 
@@ -113,19 +114,22 @@ function coverage(path, repo, sha, callback) {
         classes = classes.reduce((acc, val) => acc.concat(val), []); // Flatten
 
         // The submodules
-        var modules = {'main' : [], 'alyx-matlab' : [], 'signals' : [], 'npy-matlab' : [], 'wheelAnalysis' : []};
+        const byModule = {'main' : []};
+        submodules.forEach((x) => { byModule[x] = []; });  // initialize submodules
+
         // Sort into piles
-        modules['main'] = classes.filter(function (e) {
+        byModule['main'] = classes.filter(function (e) {
             if (e.$.filename.search(/(tests\\|_.*test|docs\\)/i) !== -1) {return false;} // Filter out tests and docs
             if (!Array.isArray(e.lines[0].line)) {return false;} // Filter out files with no functional lines
-            if (e.$.filename.startsWith('alyx-matlab\\')) {modules['alyx-matlab'].push(e); return false;}
-            if (e.$.filename.startsWith('signals\\')) {modules.signals.push(e); return false;}
-            if (e.$.filename.startsWith('npy-matlab\\')) {modules['npy-matlab'].push(e); return false;}
-            if (e.$.filename.startsWith('wheelAnalysis\\')) {modules.wheelAnalysis.push(e); return false;}
-            else {return true}
+            for (let submodule of submodules) {
+               if (e.$.filename.startsWith(submodule)) {
+                  byModule[submodule].push(e); return false;
+               }
+            }
+            return true;
         });
         // Select module
-        modules = modules[repo.toLowerCase()] || modules['main'];
+        let modules = byModule[repo] || byModule['main'];
         formatCoverage(modules, rootPath, callback);
      });
   });

lib.js

@@ -7,6 +7,7 @@ const path = require('path');
 const createDebug  = require('debug');
 const localtunnel = require('localtunnel');
 const kill = require('tree-kill');
+const shell = require('shelljs');
 
 const config = require('./config/config').settings;
 const Coverage = require('./coverage');
@@ -175,19 +176,6 @@ function partial(func) {
    };
 }
 
-function chain(func) {
-   return function curried(...args) {
-      if (args.length >= func.length) {
-         return func.apply(this, args);
-      } else {
-         return function(...args2) {
-            return curried.apply(this, args2.concat(args));
-         }
-      }
-   };
-}
-
-
 
 /**
  * Check if job already has record, if so, update from record and finish, otherwise call tests function.
@@ -235,6 +223,42 @@ const openTunnel = async () => {
 }
 
 
+/**
+ * Lists the submodules within a Git repository.  If none are found null is returned.
+ * @param {String} repoPath - The path of the repository
+ * @returns {Array} A list of submodule names, or null if none were found
+ */
+function listSubmodules(repoPath) {
+   if (!shell.which('git')) { throw new Error('Git not found on path'); }
+   shell.pushd(repoPath);
+   let listModules = 'git config --file .gitmodules --get-regexp path';
+   const modules = shell.exec(listModules)
+   shell.popd();
+   return (!modules.code && modules.stdout !== '')? modules.match(/(?<=submodule.)[\w-]+/g) : [];
+}
+
+
+/**
+ * Get the corresponding repository path for a given repo.  The function first checks the settings.
+ * If the `repos` field doesn't exist, the path in ENV is used.  If the name is not a key in the
+ * `repos` object then we check each repo path for submodules and return the first matching
+ * submodule path.  Otherwise returns null.
+ * @param {String} name - The name of the repository
+ * @returns {String} The repository path if found
+ */
+function getRepoPath(name) {
+   if (!config.repos) { return process.env['REPO_PATH']; }  // Legacy, to remove
+   if (config.repos[name]) { return config.repos[name]; }  // Found path, return
+   const modules = listSubmodules(process.env['REPO_PATH']);
+   let repoPath = process.env['REPO_PATH'];
+   if (modules && modules.includes(name)) {
+      // If the repo is a submodule, modify path
+      repoPath += (path.sep + name);
+   }
+   return repoPath;  // No modules matched, return default
+}
+
+
 /**
  * Starts a timer with a callback to kill the job's process.
  * @param {Object} job - The Job with an associated process in the data field.
@@ -263,8 +287,9 @@ function computeCoverage(job) {
     return;
   }
   console.log('Updating coverage for job #' + job.id)
-  let xmlPath = path.join(config.dataPath, 'reports', job.data.sha, 'CoverageResults.xml')
-  Coverage(xmlPath, job.data.repo, job.data.sha, obj => {
+  const xmlPath = path.join(config.dataPath, 'reports', job.data.sha, 'CoverageResults.xml')
+  const modules = listSubmodules(process.env.REPO_PATH);
+  Coverage(xmlPath, job.data.repo, job.data.sha, modules, obj => {
     // Digest and save percentage coverage
     let misses = 0, hits = 0;
     for (let file of obj.source_files) {
@@ -388,7 +413,7 @@ function getBadgeData(data) {
       report['color'] = 'orange';
       // Check test isn't already on the pile
       let onPile = false;
-      for (let job of queue.pile) { if (job.id === id) { onPile = true; break; } }
+      for (let job of queue.pile) { if (job.data.sha === id) { onPile = true; break; } }
       if (!onPile) { // Add test to queue
          data['skipPost'] = true
          queue.add(data);
@@ -397,7 +422,7 @@ function getBadgeData(data) {
       record = Array.isArray(record) ? record.pop() : record;  // in case of duplicates, take last
       switch (data.context) {
          case 'status':
-            if (record['status'] === 'error' || !record['coverage']) {
+            if (record['status'] === 'error') {
                report['message'] = 'unknown';
                report['color'] = 'orange';
             } else {
@@ -433,5 +458,5 @@ class APIError extends Error {
 module.exports = {
    ensureArray, loadTestRecords, compareCoverage, computeCoverage, getBadgeData, log, shortID,
    openTunnel, APIError, queue, partial, startJobTimer, updateJobFromRecord, shortCircuit, isSHA,
-   fullpath, strToBool, saveTestRecords
+   fullpath, strToBool, saveTestRecords, listSubmodules, getRepoPath
 }

package-lock.json

@@ -31,7 +31,6 @@
   },
   "devDependencies": {
     "chai": "^4.2.0",
-    "chai-spies": "^1.0.0",
     "fake-timers": "^0.1.2",
     "mocha": "^8.2.0",
     "nock": "^13.0.4",

package.json

@@ -12,6 +12,7 @@ function runAllTests(id, repo, logDir)
 try
   %% Initialize enviroment
   dbPath = fullfile(logDir, '.db.json'); % TODO Load from config file
+  [~, repo] = fileparts(repo); % Ensure not full path
   fprintf('Running tests\n')
   fprintf('Repo = %s, sha = %s\n', repo, id)
   origDir = pwd;
@@ -30,7 +31,7 @@ function runAllTests(id, repo, logDir)
   main_tests = testsuite('IncludeSubfolders', true);
 
   %% Gather signals tests
-  root = getOr(dat.paths,'rigbox');
+  root = getOr(dat.paths, 'rigbox');
   signals_tests = testsuite(fullfile(root, 'signals', 'tests'), ...
     'IncludeSubfolders', true);
 
@@ -44,11 +45,11 @@ function runAllTests(id, repo, logDir)
   % the sortByFixtures method to sort the suite.
   all_tests = [main_tests signals_tests alyx_tests];
   % If the repo under test is alyx, filter out irrelevent tests
-  if endsWith(repo, 'alyx')
+  if strcmp(repo, 'alyx')
     all_tests = all_tests(startsWith({all_tests.Name}, 'Alyx', 'IgnoreCase', true));
-  elseif endsWith(repo, 'alyx-matlab')
+  elseif strcmp(repo, 'alyx-matlab')
     all_tests = alyx_tests;
-  elseif endsWith(repo, 'signals')
+  elseif strcmp(repo, 'signals')
     all_tests = signals_tests;
   end
 

runAllTests.m

@@ -85,6 +85,14 @@ async function setAccessToken() {
 
 ///////////////////// MAIN APP ENTRY POINT /////////////////////
 
+/**
+ * Register invalid Github POST requests to handler via /github endpoint.
+ * Failed spoof attempts may end up here but most likely it will be unsupported webhook events.
+ */
+handler.on('error', function (err) {
+  console.log('Error:', err.message);
+});
+
 /**
  * Callback to deal with POST requests from /github endpoint, authenticates as app and passes on
  * request to handler.
@@ -97,17 +105,13 @@ srv.post('/github', async (req, res, next) => {
    console.log('Post received')
    let id = req.header('x-github-hook-installation-target-id');
    if (id != process.env.GITHUB_APP_IDENTIFIER) { next(); return; }  // Not for us; move on
-   await setAccessToken();
-   log.extend('event')('X-GitHub-Event: %s', req.header('X-GitHub-Event'));
-   handler(req, res, () => res.end('ok'));
-});
-
-/**
- * Register invalid Github POST requests to handler via /github endpoint.
- * Failed spoof attempts may end up here but most likely it will be unsupported webhook events.
- */
-handler.on('error', function (err) {
-  console.log('Error:', err.message);
+   if (req.header('X-GitHub-Event') in supportedEvents) {
+      await setAccessToken();
+      handler(req, res, () => res.end('ok'));
+   } else {
+      log('GitHub Event "%s" not supported', req.header('X-GitHub-Event'));
+      res.sendStatus(400);
+   }
 });
 
 
@@ -222,7 +226,7 @@ function runTests(job) {
 
    // Go ahead with tests
    const sha = job.data['sha'];
-   const repoPath = getRepoPath(job.data.repo);
+   const repoPath = lib.getRepoPath(job.data.repo);
    const logName = path.join(config.dataPath, 'reports', sha, `std_output-${lib.shortID(sha)}.log`);
    let fcn = lib.fullpath(config.test_function);
    debug('starting test child process %s', fcn);
@@ -278,7 +282,7 @@ function runTests(job) {
 
 function prepareEnv(job, callback) {
    log('Preparing environment for job #%g', job.id)
-   const repoPath = getRepoPath(job.data.repo);
+   const repoPath = lib.getRepoPath(job.data.repo);
    switch (config.setup_function) {
       case undefined:
          // run some basic git commands
@@ -342,41 +346,6 @@ function checkout(repoPath, ref) {
 }
 
 
-/**
- * Lists the submodules within a Git repository.  If none are found null is returned.
- * @param {String} repoPath - The path of the repository
- * @returns {Array} A list of submodule names, or null if none were found
- */
-function listSubmodules(repoPath) {
-   if (!shell.which('git')) { throw new Error('Git not found on path'); }
-   shell.pushd(repoPath);
-   let listModules = 'git config --file .gitmodules --get-regexp path | awk \'{ print $2 }\'';
-   const modules = shell.exec(listModules);
-   shell.popd();
-   return (!modules.code && modules.stdout !== '')? modules.split('\n') : null;
-}
-
-/**
- * Get the corresponding repository path for a given repo.  The function first checks the settings.
- * If the `repos` field doesn't exist, the path in ENV is used.  If the name is not a key in the
- * `repos` object then we check each repo path for submodules and return the first matching
- * submodule path.  Otherwise returns null.
- * @param {String} name - The name of the repository
- * @returns {String} The repository path if found
- */
-function getRepoPath(name) {
-   if (!config.repos) { return process.env['REPO_PATH']; }  // Legacy, to remove
-   if (config.repos.name) { return config.repos.name; }  // Found path, return
-   for (let repo of config.repos) {
-      let modules = listSubmodules(repo);
-      if (modules && modules.includes(name)) {
-         // If the repo is a submodule, modify path
-         return repo + path.sep + name;
-      }
-   }
-}
-
-
 ///////////////////// OTHER /////////////////////
 
 /**