Exposed getRepoPath and listSubmodules; added test to listSubmodules; refactored coverage

Author: k1o0

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) {
@@ -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
 }

runAllTests.m

@@ -49,10 +49,8 @@ function runAllTests(id, repo, logDir)
     all_tests = all_tests(startsWith({all_tests.Name}, 'Alyx', 'IgnoreCase', true));
   elseif strcmp(repo, 'alyx-matlab')
     all_tests = alyx_tests;
-    root = fullfile(root, repo);
   elseif strcmp(repo, 'signals')
     all_tests = signals_tests;
-    root = fullfile(root, repo);
   end
 
   % Filter out performance tests

serve.js

@@ -222,7 +222,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 +278,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 +342,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';
-   const modules = shell.exec(listModules)
-   shell.popd();
-   return (!modules.code && modules.stdout !== '')? modules.match(/(?<=submodule.)[\w-]+/g) : 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
-   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
-}
-
-
 ///////////////////// OTHER /////////////////////
 
 /**
@@ -578,6 +543,5 @@ queue.on('finish', (err, job) => { // On job end post result to API
 });
 
 module.exports = {
-   updateStatus, srv, handler, setAccessToken, prepareEnv,
-   runTests, eventCallback, getRepoPath, listSubmodules
+   updateStatus, srv, handler, setAccessToken, prepareEnv, runTests, eventCallback
 }

test/coverage.test.js

@@ -18,7 +18,7 @@ describe('Test coverage parser:', function() {
         let md5 = '385a5d56850127317c317b0f66e91078';
         let code = 'line1\nline2\n\rline3\n\rline4';
         testable = function(obj, done) {
-            expect([291, 63]).to.include(obj.source_files.length);
+            expect([496, 63]).to.include(obj.source_files.length);
             let file = obj.source_files[0];
             expect(file).to.have.all.keys('name', 'source_digest', 'coverage');
             expect(file['source_digest']).to.eq(md5);
@@ -27,21 +27,19 @@ describe('Test coverage parser:', function() {
         sandbox = sinon.createSandbox();
         sandbox
             .stub(fs, 'readFileSync')
-            .withArgs(sinon.match((x) => x.replace('\\', '/').startsWith('C:/Rigbox')))
-            .returns(code)
-            .withArgs(sinon.match((x) => x.replace('\\', '/').startsWith('C:/ibllib')))
+            .withArgs(sinon.match((x) => x.replace('\\', '/').startsWith('C:/Hello-World')))
             .returns(code);
         fs.readFileSync.callThrough();
     })
 
     it('Check loading MATLAB', function (done) {
         let xmlPath = path.resolve('test', 'fixtures', 'CoverageResults.mat.xml')
-        Coverage(xmlPath, 'rigbox', dummy_id, obj => testable(obj, done) );
+        Coverage(xmlPath, 'Hello-World', dummy_id, [], obj => testable(obj, done) );
     });
 
     it('Check loading Python', function (done) {
         let xmlPath = path.resolve('test', 'fixtures', 'CoverageResults.py.xml')
-        Coverage(xmlPath, 'ibllib', dummy_id, obj => testable(obj, done) );
+        Coverage(xmlPath, 'Hello-World', dummy_id, [], obj => testable(obj, done) );
     });
 
     afterEach(function () { sandbox.restore(); });

test/fixtures/CoverageResults.mat.xml

@@ -6,7 +6,7 @@
           timestamp="1603119812.882"
           version="">
    <sources>
-      <source>C:\Rigbox\</source>
+      <source>C:\Hello-World</source>
    </sources>
    <packages>
       <package branch-rate="NaN" complexity="NaN" line-rate="0.31707" name="">

test/fixtures/CoverageResults.py.xml

@@ -3,7 +3,7 @@
 	<!-- Generated by coverage.py: https://coverage.readthedocs.io -->
 	<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
 	<sources>
-		<source>C:/ibllib/</source>
+		<source>C:\Hello-World</source>
 	</sources>
 	<packages>
 		<package branch-rate="0" complexity="0" line-rate="0.1637" name="C:.Users.User.Documents.Github.ibllib.alf">