feat: add timeout option for both install and test phase (#769)

Co-authored-by: Zirro <[email protected]>

README.md

@@ -143,6 +143,7 @@ For syntax, see [lookup.json](./lib/lookup.json), the available attributes are:
 "useGitClone": true          Use a shallow git clone instead of downloading the module
 "ignoreGitHead":             Ignore the gitHead field if it exists and fallback to using github tags
 "yarn":                      Install and test the project using yarn instead of npm
+"timeout":                   Number of milliseconds before timeout. Applies separately to `install` and `test`
 ```
 
 If you want to pass options to npm, eg `--registry`, you can usually define an

bin/citgm-all.js

@@ -68,7 +68,7 @@ const options = {
   failFlaky: app.failFlaky,
   level: app.verbose,
   npmLevel: app.npmLoglevel,
-  timeoutLength: app.timeout,
+  timeout: app.timeout,
   tmpDir: app.tmpDir,
   customTest: app.customTest,
   yarn: app.yarn,

bin/citgm.js

@@ -45,7 +45,7 @@ const options = {
   testPath: app.testPath,
   level: app.verbose,
   npmLevel: app.npmLoglevel,
-  timeoutLength: app.timeout,
+  timeout: app.timeout,
   sha: app.sha,
   tmpDir: app.tmpDir,
   customTest: app.customTest,

lib/common-args.js

@@ -73,8 +73,7 @@ module.exports = function commonArgs(app) {
     .option('timeout', {
       alias: 'o',
       type: 'number',
-      description: 'Set timeout for npm install',
-      default: 1000 * 60 * 10
+      description: 'Set timeout for `install` and `test` phases'
     })
     .option('yarn', {
       alias: 'y',

lib/grab-project.js

@@ -38,7 +38,7 @@ async function grabProject(context) {
     // Default timeout to 10 minutes if not provided
     const timeout = setTimeout(
       cleanup,
-      context.options.timeoutLength || 1000 * 60 * 10
+      context.options.timeout || 1000 * 60 * 10
     );
 
     function cleanup() {

lib/lookup.js

@@ -162,6 +162,9 @@ function resolve(context) {
       if (rep.yarn) {
         context.module.useYarn = true;
       }
+      if (rep.timeout) {
+        context.module.timeout = rep.timeout;
+      }
       context.module.flaky = context.options.failFlaky
         ? false
         : isMatch(rep.flaky);

lib/package-manager/install.js

@@ -43,6 +43,7 @@ function install(packageManager, context) {
 
     const proc = spawn(packageManagerBin, args, options);
     const finish = timeout(
+      packageManager,
       context,
       proc,
       (err) => {

lib/package-manager/test.js

@@ -105,7 +105,14 @@ async function test(packageManager, context) {
       );
 
       const proc = spawn(bin, args, options);
-      const finish = timeout(context, proc, runScript, 'Test');
+      const finish = timeout(
+        packageManager,
+        context,
+        proc,
+        runScript,
+        'Test',
+        context.module.timeout
+      );
 
       proc.stdout.on('data', (data) => {
         context.testOutput.append(data);

lib/timeout.js

@@ -3,9 +3,17 @@
 // Default timeout to 10 minutes if not provided
 const kDefaultTimeout = 1000 * 60 * 10;
 
-function timeout(context, proc, next, step) {
+function timeout(
+  packageManager,
+  context,
+  proc,
+  next,
+  step,
+  moduleConfigTimeout
+) {
   let hasRun = false;
-  const delay = context.options.timeoutLength || kDefaultTimeout;
+  const delay =
+    context.options.timeout || moduleConfigTimeout || kDefaultTimeout;
   // Third arg === `true` is the way to signal `finish` that this is a timeout.
   // Otherwise it acts like a "regular" callback, i.e. `(err, ret) => {}`.
   // `if (timedOut)` will overwrite `err` & `ret`, so first 2 args are ignored.
@@ -28,8 +36,8 @@ function timeout(context, proc, next, step) {
       context.emit(
         'data',
         'error',
-        `${context.module.name} npm:`,
-        `npm-${step.toLowerCase()} Timed Out`
+        `${context.module.name} ${packageManager}:`,
+        `${packageManager}-${step.toLowerCase()} Timed Out`
       );
       proc.kill();
       err = new Error(`${step} Timed Out`);

test/npm/test-npm-install.js

@@ -89,7 +89,7 @@ test('npm-install: timeout', async (t) => {
     sandbox,
     {
       npmLevel: 'silly',
-      timeoutLength: 100
+      timeout: 100
     }
   );
   try {

test/npm/test-npm-test.js

@@ -126,7 +126,7 @@ test('npm-test: timeout', async (t) => {
     sandbox,
     {
       npmLevel: 'silly',
-      timeoutLength: 100
+      timeout: 100
     }
   );
   try {

test/test-grab-project.js

@@ -159,7 +159,7 @@ test('grab-project: timeout', async (t) => {
     meta: {},
     options: {
       npmLevel: 'silly',
-      timeoutLength: 10
+      timeout: 10
     }
   };
   try {

test/test-timeout.js

@@ -24,7 +24,7 @@ test('timeout:', (t) => {
     sandbox,
     {
       npmLevel: 'silly',
-      timeoutLength: 100
+      timeout: 100
     }
   );
   const proc = {
@@ -39,7 +39,7 @@ test('timeout:', (t) => {
     err = e;
     ret = r;
   };
-  const finish = timeout(context, proc, next, 'Tap');
+  const finish = timeout('npm', context, proc, next, 'Tap');
   setTimeout(() => {
     t.notOk(context.module.flaky, 'Time out should not mark module flaky');
     t.equals(proc.killed, 1);
@@ -61,7 +61,7 @@ test('timeout:', (t) => {
     sandbox,
     {
       npmLevel: 'silly',
-      timeoutLength: 100
+      timeout: 100
     }
   );
   const proc = {
@@ -80,7 +80,7 @@ test('timeout:', (t) => {
     ret = r;
     return sentinel1;
   };
-  const finish = timeout(context, proc, next, 'Tap');
+  const finish = timeout('npm', context, proc, next, 'Tap');
   const r = finish(sentinel2, sentinel3);
   t.equals(r, sentinel1);
   t.equals(proc.killed, 0);

test/yarn/test-yarn-install.js

@@ -65,7 +65,7 @@ test('yarn-install: timeout', async (t) => {
     packageManagers,
     sandbox,
     {
-      timeoutLength: 100
+      timeout: 100
     }
   );
   try {

test/yarn/test-yarn-test.js

@@ -121,7 +121,7 @@ test('yarn-test: timeout', async (t) => {
     packageManagers,
     sandbox,
     {
-      timeoutLength: 100
+      timeout: 100
     }
   );
   try {