From 95a1575b897ed87ceef39574911ddcc35bd339be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erg=C3=BCn=20Erdo=C4=9Fmu=C5=9F?= <erdogmusergun@gmail.com>
Date: Wed, 29 Jun 2022 11:00:39 +0200
Subject: [PATCH] fix: close browser after run and throw errors (#211)

* We need to close the browser so that process
exits at the end of the runs
* We need to throw the error so that process
exits with exit code 1 when the replay
was not successful
---
 src/CLIUtils.ts  | 12 ++++++++----
 test/cli.test.ts | 39 +++++++++++++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/CLIUtils.ts b/src/CLIUtils.ts
index ec27583b..34a72011 100644
--- a/src/CLIUtils.ts
+++ b/src/CLIUtils.ts
@@ -20,6 +20,7 @@ import { join, isAbsolute, extname } from 'path';
 import { pathToFileURL } from 'url';
 import { cwd } from 'process';
 import { PuppeteerRunnerOwningBrowserExtension } from '../lib/main.js';
+import { Browser } from 'puppeteer';
 
 export function getJSONFilesFromFolder(path: string): string[] {
   return readdirSync(path)
@@ -80,8 +81,10 @@ export async function runFiles(
     log: false,
     headless: true,
   }
-): Promise<boolean> {
+): Promise<void> {
   let Extension = PuppeteerRunnerOwningBrowserExtension;
+  let browser: Browser | undefined;
+
   if (opts.extension) {
     const module = await import(
       pathToFileURL(
@@ -99,7 +102,7 @@ export async function runFiles(
       const object = JSON.parse(content);
       const recording = parse(object);
       const { default: puppeteer } = await import('puppeteer');
-      const browser = await puppeteer.launch({
+      browser = await puppeteer.launch({
         headless: opts.headless,
       });
       const page = await browser.newPage();
@@ -109,8 +112,9 @@ export async function runFiles(
       opts.log && console.log(`Finished running ${file}`);
     } catch (err) {
       opts.log && console.error(`Error running ${file}`, err);
-      return false;
+      throw err;
+    } finally {
+      await browser?.close();
     }
   }
-  return true;
 }
diff --git a/test/cli.test.ts b/test/cli.test.ts
index 2ec75db5..51ebf8f0 100644
--- a/test/cli.test.ts
+++ b/test/cli.test.ts
@@ -26,6 +26,26 @@ import url from 'url';
 
 const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
 
+// TODO(ergunsh): There was an issue with Eslint saying enums are not used
+// After fixing it, update this to be an enum
+const Status = {
+  Success: 1,
+  Error: 0,
+} as const;
+
+async function getStatus(
+  asyncFn: () => Promise<unknown>
+): Promise<typeof Status['Success'] | typeof Status['Error']> {
+  let error = undefined;
+  try {
+    await asyncFn();
+  } catch (err) {
+    error = err;
+  }
+
+  return error ? Status.Error : Status.Success;
+}
+
 describe('cli', () => {
   describe('getHeadlessEnvVar', () => {
     it('extracts the headless parameter from process.argv', () => {
@@ -42,33 +62,36 @@ describe('cli', () => {
 
   describe('runFiles', () => {
     it('is able to run successfully', async () => {
-      assert.isTrue(
-        await runFiles([path.join(__dirname, 'resources', 'replay.json')])
+      const result = await getStatus(() =>
+        runFiles([path.join(__dirname, 'resources', 'replay.json')])
       );
+      assert.strictEqual(result, Status.Success);
     });
 
     it('is not able to run', async () => {
-      assert.isFalse(
-        await runFiles([path.join(__dirname, 'resources', 'replay-fail.json')])
+      const result = await getStatus(() =>
+        runFiles([path.join(__dirname, 'resources', 'replay-fail.json')])
       );
+      assert.strictEqual(result, Status.Error);
     });
 
     it('is able to run able to run folder of recordings', async () => {
       const recordings = getJSONFilesFromFolder(
         path.join(__dirname, 'resources')
       );
-
-      assert.isFalse(await runFiles([...recordings]));
+      const result = await getStatus(() => runFiles([...recordings]));
+      assert.strictEqual(result, Status.Error);
     });
 
     it('is able to run successfully with an extension', async () => {
-      assert.isTrue(
-        await runFiles([path.join(__dirname, 'resources', 'replay.json')], {
+      const result = await getStatus(() =>
+        runFiles([path.join(__dirname, 'resources', 'replay.json')], {
           extension: path.join('examples', 'cli-extension', 'extension.js'),
           headless: true,
           log: false,
         })
       );
+      assert.strictEqual(result, Status.Success);
     });
   });