From 34dfdfa34bb8b40baffd39f8fa3a3bce607c99c9 Mon Sep 17 00:00:00 2001 From: Zirro Date: Mon, 4 Nov 2019 18:04:40 +0100 Subject: [PATCH 1/6] feat: add timeoutLength option --- README.md | 1 + lib/common-args.js | 3 +-- lib/lookup.js | 3 +++ lib/package-manager/install.js | 1 + lib/package-manager/test.js | 9 ++++++++- lib/timeout.js | 16 ++++++++++++---- test/test-timeout.js | 4 ++-- 7 files changed, 28 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 6b12faad7..afeb47e1d 100644 --- a/README.md +++ b/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 +"timeoutLength": Number of milliseconds before timeout ``` If you want to pass options to npm, eg `--registry`, you can usually define an diff --git a/lib/common-args.js b/lib/common-args.js index 6df75001b..1516f466f 100644 --- a/lib/common-args.js +++ b/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 npm install' }) .option('yarn', { alias: 'y', diff --git a/lib/lookup.js b/lib/lookup.js index e98aa02d3..8514fcd64 100644 --- a/lib/lookup.js +++ b/lib/lookup.js @@ -162,6 +162,9 @@ function resolve(context) { if (rep.yarn) { context.module.useYarn = true; } + if (rep.timeoutLength) { + context.module.timeoutLength = rep.timeoutLength; + } context.module.flaky = context.options.failFlaky ? false : isMatch(rep.flaky); diff --git a/lib/package-manager/install.js b/lib/package-manager/install.js index 0fd994ad6..fe89e6b8b 100644 --- a/lib/package-manager/install.js +++ b/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) => { diff --git a/lib/package-manager/test.js b/lib/package-manager/test.js index 2e923ee30..0648f5855 100644 --- a/lib/package-manager/test.js +++ b/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.timeoutLength + ); proc.stdout.on('data', (data) => { context.testOutput.append(data); diff --git a/lib/timeout.js b/lib/timeout.js index 7b4a0f2cf..a2ba90ec6 100644 --- a/lib/timeout.js +++ b/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.timeoutLength || 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`); diff --git a/test/test-timeout.js b/test/test-timeout.js index e04be6a6b..849b73f65 100644 --- a/test/test-timeout.js +++ b/test/test-timeout.js @@ -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); @@ -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); From 1940ca76e659d3124660c87077c0218de6918257 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 2 May 2020 11:45:36 +0200 Subject: [PATCH 2/6] Update README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël Zasso --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index afeb47e1d..003617eb7 100644 --- a/README.md +++ b/README.md @@ -143,7 +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 -"timeoutLength": Number of milliseconds before timeout +"testTimeout": Number of milliseconds before timeout ``` If you want to pass options to npm, eg `--registry`, you can usually define an From 4ee01cd1b62c4baac90fb21a37d50fa2a55c83da Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 9 May 2020 12:44:56 +0200 Subject: [PATCH 3/6] complete rename --- bin/citgm-all.js | 2 +- bin/citgm.js | 2 +- lib/grab-project.js | 2 +- lib/lookup.js | 4 ++-- lib/package-manager/test.js | 2 +- lib/timeout.js | 2 +- test/npm/test-npm-install.js | 2 +- test/npm/test-npm-test.js | 2 +- test/test-grab-project.js | 2 +- test/test-timeout.js | 4 ++-- test/yarn/test-yarn-install.js | 2 +- test/yarn/test-yarn-test.js | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/bin/citgm-all.js b/bin/citgm-all.js index 6f5d41981..520313fa7 100755 --- a/bin/citgm-all.js +++ b/bin/citgm-all.js @@ -68,7 +68,7 @@ const options = { failFlaky: app.failFlaky, level: app.verbose, npmLevel: app.npmLoglevel, - timeoutLength: app.timeout, + testTimeout: app.timeout, tmpDir: app.tmpDir, customTest: app.customTest, yarn: app.yarn, diff --git a/bin/citgm.js b/bin/citgm.js index 5ec6a181b..6711989c0 100755 --- a/bin/citgm.js +++ b/bin/citgm.js @@ -45,7 +45,7 @@ const options = { testPath: app.testPath, level: app.verbose, npmLevel: app.npmLoglevel, - timeoutLength: app.timeout, + testTimeout: app.timeout, sha: app.sha, tmpDir: app.tmpDir, customTest: app.customTest, diff --git a/lib/grab-project.js b/lib/grab-project.js index 25f46451c..cae038572 100644 --- a/lib/grab-project.js +++ b/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.testTimeout || 1000 * 60 * 10 ); function cleanup() { diff --git a/lib/lookup.js b/lib/lookup.js index 8514fcd64..43ea3c7dd 100644 --- a/lib/lookup.js +++ b/lib/lookup.js @@ -162,8 +162,8 @@ function resolve(context) { if (rep.yarn) { context.module.useYarn = true; } - if (rep.timeoutLength) { - context.module.timeoutLength = rep.timeoutLength; + if (rep.testTimeout) { + context.module.testTimeout = rep.testTimeout; } context.module.flaky = context.options.failFlaky ? false diff --git a/lib/package-manager/test.js b/lib/package-manager/test.js index 0648f5855..45fd892cc 100644 --- a/lib/package-manager/test.js +++ b/lib/package-manager/test.js @@ -111,7 +111,7 @@ async function test(packageManager, context) { proc, runScript, 'Test', - context.module.timeoutLength + context.module.testTimeout ); proc.stdout.on('data', (data) => { diff --git a/lib/timeout.js b/lib/timeout.js index a2ba90ec6..d37b3c7a9 100644 --- a/lib/timeout.js +++ b/lib/timeout.js @@ -13,7 +13,7 @@ function timeout( ) { let hasRun = false; const delay = - context.options.timeoutLength || moduleConfigTimeout || kDefaultTimeout; + context.options.testTimeout || 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. diff --git a/test/npm/test-npm-install.js b/test/npm/test-npm-install.js index b6ef23513..04d6fe575 100644 --- a/test/npm/test-npm-install.js +++ b/test/npm/test-npm-install.js @@ -89,7 +89,7 @@ test('npm-install: timeout', async (t) => { sandbox, { npmLevel: 'silly', - timeoutLength: 100 + testTimeout: 100 } ); try { diff --git a/test/npm/test-npm-test.js b/test/npm/test-npm-test.js index 164cb0f67..3d08a2e92 100644 --- a/test/npm/test-npm-test.js +++ b/test/npm/test-npm-test.js @@ -126,7 +126,7 @@ test('npm-test: timeout', async (t) => { sandbox, { npmLevel: 'silly', - timeoutLength: 100 + testTimeout: 100 } ); try { diff --git a/test/test-grab-project.js b/test/test-grab-project.js index 2692adcbd..f52efa5d2 100644 --- a/test/test-grab-project.js +++ b/test/test-grab-project.js @@ -159,7 +159,7 @@ test('grab-project: timeout', async (t) => { meta: {}, options: { npmLevel: 'silly', - timeoutLength: 10 + testTimeout: 10 } }; try { diff --git a/test/test-timeout.js b/test/test-timeout.js index 849b73f65..dd5d29129 100644 --- a/test/test-timeout.js +++ b/test/test-timeout.js @@ -24,7 +24,7 @@ test('timeout:', (t) => { sandbox, { npmLevel: 'silly', - timeoutLength: 100 + testTimeout: 100 } ); const proc = { @@ -61,7 +61,7 @@ test('timeout:', (t) => { sandbox, { npmLevel: 'silly', - timeoutLength: 100 + testTimeout: 100 } ); const proc = { diff --git a/test/yarn/test-yarn-install.js b/test/yarn/test-yarn-install.js index 244025d4c..081c89a90 100644 --- a/test/yarn/test-yarn-install.js +++ b/test/yarn/test-yarn-install.js @@ -65,7 +65,7 @@ test('yarn-install: timeout', async (t) => { packageManagers, sandbox, { - timeoutLength: 100 + testTimeout: 100 } ); try { diff --git a/test/yarn/test-yarn-test.js b/test/yarn/test-yarn-test.js index 3f082f71b..f18f76f1a 100644 --- a/test/yarn/test-yarn-test.js +++ b/test/yarn/test-yarn-test.js @@ -121,7 +121,7 @@ test('yarn-test: timeout', async (t) => { packageManagers, sandbox, { - timeoutLength: 100 + testTimeout: 100 } ); try { From 5af84bb850fab0b817da9644f0c2d19fdfb5c972 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 9 May 2020 12:58:02 +0200 Subject: [PATCH 4/6] alignment --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 003617eb7..e681254d1 100644 --- a/README.md +++ b/README.md @@ -143,7 +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 -"testTimeout": Number of milliseconds before timeout +"testTimeout": Number of milliseconds before timeout ``` If you want to pass options to npm, eg `--registry`, you can usually define an From 0bdef0271c9ede2fbd9a6de489f445cc8e25c94f Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 9 May 2020 13:05:00 +0200 Subject: [PATCH 5/6] call it timeout --- README.md | 2 +- bin/citgm-all.js | 2 +- bin/citgm.js | 2 +- lib/grab-project.js | 2 +- lib/lookup.js | 4 ++-- lib/package-manager/test.js | 2 +- lib/timeout.js | 2 +- test/npm/test-npm-install.js | 2 +- test/npm/test-npm-test.js | 2 +- test/test-grab-project.js | 2 +- test/test-timeout.js | 4 ++-- test/yarn/test-yarn-install.js | 2 +- test/yarn/test-yarn-test.js | 2 +- 13 files changed, 15 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e681254d1..aa874d41e 100644 --- a/README.md +++ b/README.md @@ -143,7 +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 -"testTimeout": Number of milliseconds before timeout +"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 diff --git a/bin/citgm-all.js b/bin/citgm-all.js index 520313fa7..9f8362df7 100755 --- a/bin/citgm-all.js +++ b/bin/citgm-all.js @@ -68,7 +68,7 @@ const options = { failFlaky: app.failFlaky, level: app.verbose, npmLevel: app.npmLoglevel, - testTimeout: app.timeout, + timeout: app.timeout, tmpDir: app.tmpDir, customTest: app.customTest, yarn: app.yarn, diff --git a/bin/citgm.js b/bin/citgm.js index 6711989c0..199093f5b 100755 --- a/bin/citgm.js +++ b/bin/citgm.js @@ -45,7 +45,7 @@ const options = { testPath: app.testPath, level: app.verbose, npmLevel: app.npmLoglevel, - testTimeout: app.timeout, + timeout: app.timeout, sha: app.sha, tmpDir: app.tmpDir, customTest: app.customTest, diff --git a/lib/grab-project.js b/lib/grab-project.js index cae038572..4dadbf597 100644 --- a/lib/grab-project.js +++ b/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.testTimeout || 1000 * 60 * 10 + context.options.timeout || 1000 * 60 * 10 ); function cleanup() { diff --git a/lib/lookup.js b/lib/lookup.js index 43ea3c7dd..c4543e0ec 100644 --- a/lib/lookup.js +++ b/lib/lookup.js @@ -162,8 +162,8 @@ function resolve(context) { if (rep.yarn) { context.module.useYarn = true; } - if (rep.testTimeout) { - context.module.testTimeout = rep.testTimeout; + if (rep.timeout) { + context.module.timeout = rep.timeout; } context.module.flaky = context.options.failFlaky ? false diff --git a/lib/package-manager/test.js b/lib/package-manager/test.js index 45fd892cc..c88e0a381 100644 --- a/lib/package-manager/test.js +++ b/lib/package-manager/test.js @@ -111,7 +111,7 @@ async function test(packageManager, context) { proc, runScript, 'Test', - context.module.testTimeout + context.module.timeout ); proc.stdout.on('data', (data) => { diff --git a/lib/timeout.js b/lib/timeout.js index d37b3c7a9..c595a8de0 100644 --- a/lib/timeout.js +++ b/lib/timeout.js @@ -13,7 +13,7 @@ function timeout( ) { let hasRun = false; const delay = - context.options.testTimeout || moduleConfigTimeout || kDefaultTimeout; + 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. diff --git a/test/npm/test-npm-install.js b/test/npm/test-npm-install.js index 04d6fe575..9a743969c 100644 --- a/test/npm/test-npm-install.js +++ b/test/npm/test-npm-install.js @@ -89,7 +89,7 @@ test('npm-install: timeout', async (t) => { sandbox, { npmLevel: 'silly', - testTimeout: 100 + timeout: 100 } ); try { diff --git a/test/npm/test-npm-test.js b/test/npm/test-npm-test.js index 3d08a2e92..b6ac92ce8 100644 --- a/test/npm/test-npm-test.js +++ b/test/npm/test-npm-test.js @@ -126,7 +126,7 @@ test('npm-test: timeout', async (t) => { sandbox, { npmLevel: 'silly', - testTimeout: 100 + timeout: 100 } ); try { diff --git a/test/test-grab-project.js b/test/test-grab-project.js index f52efa5d2..b55f4129c 100644 --- a/test/test-grab-project.js +++ b/test/test-grab-project.js @@ -159,7 +159,7 @@ test('grab-project: timeout', async (t) => { meta: {}, options: { npmLevel: 'silly', - testTimeout: 10 + timeout: 10 } }; try { diff --git a/test/test-timeout.js b/test/test-timeout.js index dd5d29129..5b1e4736c 100644 --- a/test/test-timeout.js +++ b/test/test-timeout.js @@ -24,7 +24,7 @@ test('timeout:', (t) => { sandbox, { npmLevel: 'silly', - testTimeout: 100 + timeout: 100 } ); const proc = { @@ -61,7 +61,7 @@ test('timeout:', (t) => { sandbox, { npmLevel: 'silly', - testTimeout: 100 + timeout: 100 } ); const proc = { diff --git a/test/yarn/test-yarn-install.js b/test/yarn/test-yarn-install.js index 081c89a90..23f48ccfb 100644 --- a/test/yarn/test-yarn-install.js +++ b/test/yarn/test-yarn-install.js @@ -65,7 +65,7 @@ test('yarn-install: timeout', async (t) => { packageManagers, sandbox, { - testTimeout: 100 + timeout: 100 } ); try { diff --git a/test/yarn/test-yarn-test.js b/test/yarn/test-yarn-test.js index f18f76f1a..b56cadf4c 100644 --- a/test/yarn/test-yarn-test.js +++ b/test/yarn/test-yarn-test.js @@ -121,7 +121,7 @@ test('yarn-test: timeout', async (t) => { packageManagers, sandbox, { - testTimeout: 100 + timeout: 100 } ); try { From fd3af73952f6f2ddfc4c78a696686acb1e7ad85d Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 9 May 2020 13:06:41 +0200 Subject: [PATCH 6/6] udpate description --- lib/common-args.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common-args.js b/lib/common-args.js index 1516f466f..c6979607f 100644 --- a/lib/common-args.js +++ b/lib/common-args.js @@ -73,7 +73,7 @@ module.exports = function commonArgs(app) { .option('timeout', { alias: 'o', type: 'number', - description: 'Set timeout for npm install' + description: 'Set timeout for `install` and `test` phases' }) .option('yarn', { alias: 'y',