feat: add default global timeout (#237)

* refactor: remove unused fields * refactor: remove unused fields * refactor: camel case * feat: add default global timeout * refactor: simplify result type handling * refactor: remove unused type * feat: remove playwright's native global timeout * fix: remove incorrect test timeout
saucelabs · Oct 16, 2023 · bde42ab · bde42ab
1 parent 1d2db3c
commit bde42ab
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 97 deletions.
diff --git a/src/cucumber-runner.ts b/src/cucumber-runner.ts
@@ -2,7 +2,7 @@ import {spawn} from 'node:child_process';
 import * as path from 'node:path';
 import {prepareNpmEnv, preExec} from 'sauce-testrunner-utils';
 
-import type {CucumberRunnerConfig, Metrics, RunResult} from './types';
+import type {CucumberRunnerConfig} from './types';
 import * as utils from './utils';
 
 function buildArgs(runCfg: CucumberRunnerConfig, cucumberBin: string) {
@@ -62,7 +62,7 @@ function buildArgs(runCfg: CucumberRunnerConfig, cucumberBin: string) {
   return procArgs;
 }
 
-export async function runCucumber(nodeBin: string, runCfg: CucumberRunnerConfig): Promise<RunResult> {
+export async function runCucumber(nodeBin: string, runCfg: CucumberRunnerConfig): Promise<boolean> {
   process.env.BROWSER_NAME = runCfg.suite.browserName;
   process.env.BROWSER_OPTIONS = runCfg.suite.browserOptions;
   process.env.SAUCE_SUITE_NAME = runCfg.suite.name;
@@ -78,49 +78,40 @@ export async function runCucumber(nodeBin: string, runCfg: CucumberRunnerConfig)
   const nodeCtx = {nodePath: nodeBin, npmPath: npmBin};
 
   // Install NPM dependencies
-  const metrics: Metrics[] = [];
-  const npmMetrics = await prepareNpmEnv(runCfg, nodeCtx);
-  metrics.push(npmMetrics);
+  await prepareNpmEnv(runCfg, nodeCtx);
 
-  const startTime = new Date().toISOString();
   // Run suite preExecs
   if (!await preExec.run(runCfg.suite, runCfg.preExecTimeout)) {
-    return {
-      startTime,
-      endTime: new Date().toISOString(),
-      hasPassed: false,
-      metrics,
-    };
+    return false;
   }
 
   const cucumberBin = path.join(runCfg.projectPath, 'node_modules', '@cucumber', 'cucumber', 'bin', 'cucumber-js');
   const procArgs = buildArgs(runCfg, cucumberBin);
   const proc = spawn(nodeBin, procArgs, {stdio: 'inherit', env: process.env});
 
-  let passed = false;
-  const procPromise = new Promise<number | null>((resolve, reject) => {
-    proc.on('error', (err) => {
-      reject(err);
-    });
+  // saucectl suite.timeout is in nanoseconds, convert to seconds
+  const timeout = (runCfg.suite.timeout || 0) / 1_000_000_000 || 30 * 60; // 30min default
+
+  const timeoutPromise = new Promise<boolean>((resolve) => {
+    setTimeout(() => {
+      console.error(`Job timed out after ${timeout} seconds`);
+      resolve(false);
+    }, timeout * 1000);
+  });
+
+  const cucumberPromise = new Promise<boolean>((resolve) => {
     proc.on('close', (code) => {
-      resolve(code);
+      resolve(code === 0);
     });
   });
 
   try {
-    const exitCode = await procPromise;
-    passed = exitCode === 0;
+    return await Promise.race([timeoutPromise, cucumberPromise]);
   } catch (e) {
-    console.error(`Could not complete job. Reason: ${e}`);
+    console.error(`Failed to run Cucumber.js: ${e}`);
   }
-  const endTime = new Date().toISOString();
 
-  return {
-    startTime,
-    endTime,
-    hasPassed: passed,
-    metrics,
-  };
+  return false;
 }
 
 function buildFormatOption(cfg: CucumberRunnerConfig) {

diff --git a/src/playwright-runner.ts b/src/playwright-runner.ts
@@ -10,9 +10,7 @@ import * as convert from 'xml-js';
 import {runCucumber} from './cucumber-runner';
 import type {
   CucumberRunnerConfig,
-  Metrics,
   RunnerConfig,
-  RunResult,
 } from './types';
 import * as utils from './utils';
 
@@ -24,7 +22,7 @@ function getPlatformName(platformName: string) {
   return platformName;
 }
 
-function generateJunitfile(sourceFile: string, suiteName: string, browserName: string, platformName: string) {
+function generateJunitFile(sourceFile: string, suiteName: string, browserName: string, platformName: string) {
   if (!fs.existsSync(sourceFile)) {
     return;
   }
@@ -168,22 +166,22 @@ async function run(nodeBin: string, runCfgPath: string, suiteName: string) {
   console.log(`Sauce Playwright Runner ${packageInfo.version}`);
   console.log(`Running Playwright ${packageInfo.dependencies?.playwright || ''}`);
 
-  let result: RunResult;
+  let passed = false;
   if (runCfg.Kind === 'playwright-cucumberjs') {
-    result = await runCucumber(nodeBin, runCfg);
+    passed = await runCucumber(nodeBin, runCfg);
   } else {
-    result = await runPlaywright(nodeBin, runCfg);
+    passed = await runPlaywright(nodeBin, runCfg);
     try {
-      generateJunitfile(runCfg.junitFile, runCfg.suite.name, runCfg.suite.param.browser, runCfg.suite.platformName);
+      generateJunitFile(runCfg.junitFile, runCfg.suite.name, runCfg.suite.param.browser, runCfg.suite.platformName);
     } catch (err) {
       console.error(`Failed to generate junit file: ${err}`);
     }
   }
 
-  return result.hasPassed;
+  return passed;
 }
 
-async function runPlaywright(nodeBin: string, runCfg: RunnerConfig): Promise<RunResult> {
+async function runPlaywright(nodeBin: string, runCfg: RunnerConfig): Promise<boolean> {
   const excludeParams = ['screenshot-on-failure', 'video', 'slow-mo', 'headless', 'headed'];
 
   process.env.BROWSER_NAME = runCfg.suite.param.browserName;
@@ -272,54 +270,43 @@ async function runPlaywright(nodeBin: string, runCfg: RunnerConfig): Promise<Run
 
   utils.setEnvironmentVariables(env);
 
-  // Install NPM dependencies
-  const metrics: Metrics[] = [];
-
   // Define node/npm path for execution
   const npmBin = path.join(path.dirname(nodeBin), 'node_modules', 'npm', 'bin', 'npm-cli.js');
   const nodeCtx = { nodePath: nodeBin, npmPath: npmBin };
 
   // runCfg.path must be set for prepareNpmEnv to find node_modules. :(
-  const npmMetrics = await prepareNpmEnv(runCfg, nodeCtx);
-  metrics.push(npmMetrics);
+  await prepareNpmEnv(runCfg, nodeCtx);
 
-  const startTime = new Date().toISOString();
   // Run suite preExecs
   if (!await preExec.run(suite, runCfg.preExecTimeout)) {
-    return {
-      startTime,
-      endTime: new Date().toISOString(),
-      hasPassed: false,
-      metrics,
-    };
+    return false;
   }
 
   const playwrightProc = spawn(nodeBin, procArgs, {stdio: 'inherit', cwd: runCfg.projectPath, env});
 
-  const playwrightPromise = new Promise<number | null>((resolve, reject) => {
-    playwrightProc.on('error', (err) => {
-      reject(err);
-    });
+  // saucectl suite.timeout is in nanoseconds, convert to seconds
+  const timeout = (runCfg.suite.timeout || 0) / 1_000_000_000 || 30 * 60; // 30min default
+
+  const timeoutPromise = new Promise<boolean>((resolve) => {
+    setTimeout(() => {
+      console.error(`Job timed out after ${timeout} seconds`);
+      resolve(false);
+    }, timeout * 1000);
+  });
+
+  const playwrightPromise = new Promise<boolean>((resolve) => {
     playwrightProc.on('close', (code) => {
-      resolve(code);
+      resolve(code === 0);
     });
   });
 
-  let hasPassed = false;
   try {
-    const exitCode = await playwrightPromise;
-    hasPassed = exitCode === 0;
+    return await Promise.race([timeoutPromise, playwrightPromise]);
   } catch (e) {
-    console.error(`Could not complete job. Reason: ${e}`);
+    console.error(`Failed to run Playwright: ${e}`);
   }
-  const endTime = new Date().toISOString()
 
-  return {
-    startTime,
-    endTime,
-    hasPassed,
-    metrics,
-  };
+  return false;
 }
 
 if (require.main === module) {

diff --git a/src/sauce.config.cjs b/src/sauce.config.cjs
@@ -26,12 +26,6 @@ for (const file of configFiles) {
   }
 }
 
-// Set a default timeout of 30 minutes if one is not provided. This protects
-// against tests that hang and never finish.
-if (!userConfig.globalTimeout) {
-  userConfig.globalTimeout = 1000 * 60 * 30; // 30 minutes
-}
-
 const overrides = {
   use: {
     headless: process.env.HEADLESS === 'true',

diff --git a/src/sauce.config.mjs b/src/sauce.config.mjs
@@ -27,12 +27,6 @@ for (const file of configFiles) {
   }
 }
 
-// Set a default timeout of 30 minutes if one is not provided. This protects
-// against tests that hang and never finish.
-if (!userConfig.globalTimeout) {
-  userConfig.globalTimeout = 1000 * 60 * 30; // 30 minutes
-}
-
 const overrides = {
   use: {
     headless: process.env.HEADLESS === 'true',

diff --git a/src/types.ts b/src/types.ts
@@ -1,23 +1,5 @@
 import type { Region } from '@saucelabs/testcomposer';
 
-export type Browser = 'chromium' | 'firefox' | 'webkit' | 'chrome';
-
-export interface Metrics {
-  name: string;
-  data: {
-    install: {duration: number};
-    rebuild?: {duration: number};
-    setup: {duration: number};
-  };
-}
-
-export interface RunResult {
-  startTime: string;
-  hasPassed: boolean;
-  endTime: string;
-  metrics: Metrics[];
-}
-
 export interface RunnerConfig {
   // NOTE: Kind is serialized by saucectl with a capital 'K' ¯\_(ツ)_/¯
   Kind: 'playwright';
@@ -59,6 +41,7 @@ export interface Suite {
   env?: Record<string, string>;
   preExec: string[];
   testIgnore?: string[];
+  timeout?: number;
 }
 
 export interface SuiteConfig {
@@ -114,4 +97,5 @@ export interface CucumberSuite {
     parallel?: number;
     paths: string[];
   };
+  timeout?: number;
 }
diff --git a/tests/fixtures/local/basic-js/sauce-runner.json b/tests/fixtures/local/basic-js/sauce-runner.json
@@ -21,10 +21,9 @@
               "repeatEach": 1,
               "retries": 0,
               "shard": "1/2",
-              "timeout": 30000,
               "maxFailures": 2
             },
             "testMatch": ".*.js"
         }
     ]
-}
+}