chore(e2e): rename and refactor beforeTests, afterTest and afterTests helpers; use them properly (#5335)

* rename and refactor afterTest and afterTests hooks; use them properly

* actually save all the files..

* coverage might not be there if you're quick enough

* remove spaces

Author: lerouxb

packages/compass-e2e-tests/helpers/compass.ts

@@ -108,12 +108,10 @@ export const serverSatisfies = (
 let i = 0;
 // For the screenshots
 let j = 0;
-// For the coverage
-let k = 0;
 
 interface Coverage {
-  main: string;
-  renderer: string;
+  main?: string;
+  renderer?: string;
 }
 
 interface RenderLogEntry {
@@ -124,6 +122,7 @@ interface RenderLogEntry {
 }
 
 export class Compass {
+  name: string;
   browser: CompassBrowser;
   testPackagedApp: boolean;
   needsCloseWelcomeModal: boolean;
@@ -134,9 +133,11 @@ export class Compass {
   appName?: string;
 
   constructor(
+    name: string,
     browser: CompassBrowser,
     { testPackagedApp = false, needsCloseWelcomeModal = false } = {}
   ) {
+    this.name = name;
     this.browser = browser;
     this.testPackagedApp = testPackagedApp;
     this.needsCloseWelcomeModal = needsCloseWelcomeModal;
@@ -319,30 +320,21 @@ export class Compass {
     }
   }
 
-  async stop(test?: Mocha.Hook | Mocha.Test, step?: string): Promise<void> {
+  async stop(): Promise<void> {
     // TODO: we don't have main logs to write :(
     /*
     const mainLogs = [];
     const mainLogPath = path.join(
       LOG_PATH,
-      `electron-main.${nowFormatted}.log`
+      `electron-main.${name}.log`
     );
     debug(`Writing application main process log to ${mainLogPath}`);
     await fs.writeFile(mainLogPath, mainLogs.join('\n'));
     */
 
-    const nowFormatted = formattedDate();
-
-    // name the log files after the closest test if possible to make it easier to find
-    let name = test ? pathName(test.fullTitle()) : nowFormatted;
-
-    if (step) {
-      name = `${name}-${step}`;
-    }
-
     const renderLogPath = path.join(
       LOG_PATH,
-      `electron-render.${nowFormatted}.json`
+      `electron-render.${this.name}.json`
     );
     debug(`Writing application render process log to ${renderLogPath}`);
     await fs.writeFile(renderLogPath, JSON.stringify(this.renderLogs, null, 2));
@@ -362,22 +354,25 @@ export class Compass {
           });
         })();
       });
-      const stopIndex = ++k;
-      await fs.writeFile(
-        path.join(COVERAGE_PATH, `main.${stopIndex}.log`),
-        coverage.main
-      );
-      await fs.writeFile(
-        path.join(COVERAGE_PATH, `renderer.${stopIndex}.log`),
-        coverage.renderer
-      );
+      if (coverage.main) {
+        await fs.writeFile(
+          path.join(COVERAGE_PATH, `main.${this.name}.log`),
+          coverage.main
+        );
+      }
+      if (coverage.renderer) {
+        await fs.writeFile(
+          path.join(COVERAGE_PATH, `renderer.${this.name}.log`),
+          coverage.renderer
+        );
+      }
     }
 
     debug('Stopping Compass application');
     await this.browser.deleteSession();
 
     const compassLog = await getCompassLog(this.logPath ?? '');
-    const compassLogPath = path.join(LOG_PATH, `compass-log.${name}.log`);
+    const compassLogPath = path.join(LOG_PATH, `compass-log.${this.name}.log`);
     debug(`Writing Compass application log to ${compassLogPath}`);
     await fs.writeFile(compassLogPath, compassLog.raw);
     this.logs = compassLog.structured;
@@ -491,7 +486,10 @@ export async function runCompassOnce(args: string[], timeout = 30_000) {
   return { stdout, stderr };
 }
 
-async function startCompass(opts: StartCompassOptions = {}): Promise<Compass> {
+async function startCompass(
+  name: string,
+  opts: StartCompassOptions = {}
+): Promise<Compass> {
   const { testPackagedApp, binary } = await getCompassExecutionParameters();
   const nowFormatted = formattedDate();
   let needsCloseWelcomeModal: boolean;
@@ -710,7 +708,7 @@ async function startCompass(opts: StartCompassOptions = {}): Promise<Compass> {
     throw err;
   }
 
-  const compass = new Compass(browser, {
+  const compass = new Compass(name, browser, {
     testPackagedApp,
     needsCloseWelcomeModal,
   });
@@ -906,10 +904,15 @@ function augmentError(error: Error, stack: string) {
   error.stack = `${error.stack ?? ''}\nvia ${strippedLines.join('\n')}`;
 }
 
-export async function beforeTests(
+export async function init(
+  name?: string,
   opts: StartCompassOptions = {}
 ): Promise<Compass> {
-  const compass = await startCompass(opts);
+  // Unfortunately mocha's type is that this.test inside a test or hook is
+  // optional even though it always exists. So we have a lot of
+  // this.test?.fullTitle() and therefore we hopefully won't end up with a lot
+  // of dates in filenames in reality.
+  const compass = await startCompass(pathName(name ?? formattedDate()), opts);
 
   const { browser } = compass;
 
@@ -923,21 +926,11 @@ export async function beforeTests(
   return compass;
 }
 
-export async function afterTests(
-  compass?: Compass,
-  test?: Mocha.Hook | Mocha.Test,
-  step?: string
-): Promise<void> {
+export async function cleanup(compass?: Compass): Promise<void> {
   if (!compass) {
     return;
   }
 
-  if (test && test.state === undefined) {
-    // if there's no state, then it is probably because the before() hook failed
-    const filename = screenshotPathName(`${test.fullTitle()}-hook`);
-    await compass.capturePage(filename);
-  }
-
   let timeoutId;
   const timeoutPromise = new Promise<void>((resolve) => {
     timeoutId = setTimeout(() => {
@@ -948,7 +941,7 @@ export async function afterTests(
 
   const closePromise = (async function close(): Promise<void> {
     try {
-      await compass.stop(test, step);
+      await compass.stop();
     } catch (err) {
       debug('An error occurred while stopping compass:');
       debug(err);
@@ -983,12 +976,21 @@ export function outputFilename(filename: string): string {
   return path.join(OUTPUT_PATH, filename);
 }
 
-export async function afterTest(
+export async function screenshotIfFailed(
   compass: Compass,
   test?: Mocha.Hook | Mocha.Test
 ): Promise<void> {
-  if (test && test.state === 'failed') {
-    await compass.capturePage(screenshotPathName(test.fullTitle()));
+  // NOTE: you cannot use this inside a test because the test wouldn't be marked
+  // as failed yet. It is made for use inside an afterEach() to go with the
+  // pattern where we init() compass in a before() hook and cleanup() in an
+  // after() hook.
+  if (test) {
+    if (test.state === undefined) {
+      // if there's no state, then it is probably because the before() hook failed
+      await compass.capturePage(screenshotPathName(`${test.fullTitle()}-hook`));
+    } else if (test.state === 'failed') {
+      await compass.capturePage(screenshotPathName(test.fullTitle()));
+    }
   }
 }
 
@@ -1020,3 +1022,14 @@ function redact(value: string): string {
 
   return value;
 }
+
+export function subtestTitle(
+  test: Mocha.Runnable | undefined,
+  step: string
+): string {
+  // Sometimes we start and stop compass multiple times in the same test. In
+  // that case it is handy to give them unique names. That's what this function
+  // is for.
+  const title = test?.fullTitle() ?? formattedDate();
+  return `${title}_${step}`;
+}

packages/compass-e2e-tests/tests/atlas-login.test.ts

@@ -1,8 +1,8 @@
 import type { CompassBrowser } from '../helpers/compass-browser';
 import {
-  beforeTests,
-  afterTests,
-  afterTest,
+  init,
+  cleanup,
+  screenshotIfFailed,
   Selectors,
 } from '../helpers/compass';
 import type { Compass } from '../helpers/compass';
@@ -103,7 +103,7 @@ describe('Atlas Login', function () {
       return DEFAULT_TOKEN_PAYLOAD;
     };
 
-    compass = await beforeTests({
+    compass = await init(this.test?.fullTitle(), {
       // With this flag enabled, we are not persisting the data between tests
       firstRun: true,
     });
@@ -116,8 +116,8 @@ describe('Atlas Login', function () {
 
   afterEach(async function () {
     await browser.setFeature('browserCommandForOIDCAuth', undefined);
-    await afterTest(compass, this.currentTest);
-    await afterTests(compass, this.currentTest);
+    await screenshotIfFailed(compass, this.currentTest);
+    await cleanup(compass);
   });
 
   after(async function () {