Repository: elastic/synthetics
Pull request #272 (2021-05-06 23:36:06)

feat: expose params in all hooks
+ Exposes the suite params and params passed via `--config` flag to all our hooks `before, beforeAll, after, afterAll`
+ Params in hooks cannot be modified, throws error otherwise - added a test in `cli.test.ts`. 

+ Follow up of the config support PR #271 

apmmachine: 
## :green_heart: Build Succeeded


<!-- BUILD BADGES-->
> _the below badges are clickable and redirect to their specific view in the CI or DOCS_
[![Pipeline View](https://img.shields.io/badge/pipeline-pipeline%20-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-272/detail/PR-272/1//pipeline) [![Test View](https://img.shields.io/badge/test-test-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-272/detail/PR-272/1//tests) [![Changes](https://img.shields.io/badge/changes-changes-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-272/detail/PR-272/1//changes) [![Artifacts](https://img.shields.io/badge/artifacts-artifacts-yellow)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-272/detail/PR-272/1//artifacts) [![preview](https://img.shields.io/badge/docs-preview-yellowgreen)](http://synthetics_272.docs-preview.app.elstc.co/diff)

<!-- BUILD SUMMARY-->
<details><summary>Expand to view the summary</summary>
<p>

#### Build stats


* Build Cause: `Pull request #272 opened`



* Start Time: 2021-05-06T23:36:22.550+0000


* Duration: 13 min 17 sec


* Commit: d66f7b7d7636a55d7cd8924439bca7cd53f399a0



#### Test stats :test_tube:

| Test         | Results                         |
| ------------ | :-----------------------------: |
| Failed       | 0  |
| Passed       | 88  |
| Skipped      | 0 |
| Total        | 88   |


#### Trends :test_tube:


![Image of Build Times](https://apm-ci.elastic.co/job/apm-agent-rum/job/elastic-synthetics/job/PR-272/buildTimeGraph/png)


![Image of Tests](https://apm-ci.elastic.co/job/apm-agent-rum/job/elastic-synthetics/job/PR-272/1/testReport/history/countGraph/png?start=0&amp;end=25)


</p>
</details>

<!-- TEST RESULTS IF ANY-->


<!-- STEPS ERRORS IF ANY -->












<!--COMMENT_GENERATED_WITH_ID_comment.id-->

Patch:
diff --git a/src/cli.ts b/src/cli.ts
index 2ac7f2c..29ec09d 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -174,7 +174,8 @@ async function prepareSuites(inputs: string[]) {
   const reporter = options.json ? 'json' : options.reporter;
 
   const results = await run({
-    params: Object.freeze({ ...params, environment }),
+    params: Object.freeze(params),
+    environment,
     reporter,
     headless: options.headless,
     screenshots: options.screenshots,
diff --git a/src/common_types.ts b/src/common_types.ts
index b6f7c69..a9b27dc 100644
--- a/src/common_types.ts
+++ b/src/common_types.ts
@@ -28,6 +28,12 @@ import { Step } from './dsl';
 import { reporters } from './reporters';
 
 export type VoidCallback = () => void;
+export type Params = Record<string, unknown>;
+export type HooksArgs = {
+  env: string;
+  params: Params;
+};
+export type HooksCallback = (args: HooksArgs) => void;
 export type StatusValue = 'succeeded' | 'failed' | 'skipped';
 export type Reporters = keyof typeof reporters;
 
diff --git a/src/core/index.ts b/src/core/index.ts
index 9afd6a5..46d1e1f 100644
--- a/src/core/index.ts
+++ b/src/core/index.ts
@@ -25,7 +25,7 @@
 
 import { Journey, JourneyCallback, JourneyOptions } from '../dsl';
 import Runner from './runner';
-import { VoidCallback } from '../common_types';
+import { VoidCallback, HooksCallback } from '../common_types';
 import { log } from './logger';
 
 /**
@@ -57,22 +57,22 @@ export const step = (name: string, callback: VoidCallback) => {
   return runner.currentJourney?.addStep(name, callback);
 };
 
-export const beforeAll = (callback: VoidCallback) => {
+export const beforeAll = (callback: HooksCallback) => {
   runner.addHook('beforeAll', callback);
 };
 
-export const afterAll = (callback: VoidCallback) => {
+export const afterAll = (callback: HooksCallback) => {
   runner.addHook('afterAll', callback);
 };
 
-export const before = (callback: VoidCallback) => {
+export const before = (callback: HooksCallback) => {
   if (!runner.currentJourney) {
     throw new Error('before is called outside of the journey context');
   }
   return runner.currentJourney.addHook('before', callback);
 };
 
-export const after = (callback: VoidCallback) => {
+export const after = (callback: HooksCallback) => {
   if (!runner.currentJourney) {
     throw new Error('after is called outside of the journey context');
   }
diff --git a/src/core/runner.ts b/src/core/runner.ts
index 0d1442b..ddceb57 100644
--- a/src/core/runner.ts
+++ b/src/core/runner.ts
@@ -32,8 +32,10 @@ import {
   StatusValue,
   FilmStrip,
   NetworkInfo,
-  VoidCallback,
+  HooksCallback,
+  Params,
   CliArgs,
+  HooksArgs,
 } from '../common_types';
 import { BrowserMessage, PluginManager } from '../plugins';
 import { PerformanceManager, Metrics } from '../plugins';
@@ -49,16 +51,13 @@ export type RunOptions = Omit<
   | 'require'
   | 'suiteParams'
   | 'reporter'
-  | 'environment'
 > & {
-  params?: RunParamaters;
+  params?: Params;
   reporter?: CliArgs['reporter'] | Reporter;
 };
 
-type RunParamaters = { environment: string } & Record<string, unknown>;
-
 type BaseContext = {
-  params?: RunParamaters;
+  params?: Params;
   start: number;
   end?: number;
 };
@@ -84,7 +83,7 @@ type JourneyResult = {
 type RunResult = Record<string, JourneyResult>;
 
 type HookType = 'beforeAll' | 'afterAll';
-export type SuiteHooks = Record<HookType, Array<VoidCallback>>;
+export type SuiteHooks = Record<HookType, Array<HooksCallback>>;
 
 interface Events {
   start: { numJourneys: number };
@@ -94,7 +93,7 @@ interface Events {
   'journey:start': {
     journey: Journey;
     timestamp: number;
-    params: RunParamaters;
+    params: Params;
   };
   'journey:end': BaseContext &
     JourneyResult & {
@@ -132,7 +131,7 @@ export default class Runner {
     };
   }
 
-  addHook(type: HookType, callback: VoidCallback) {
+  addHook(type: HookType, callback: HooksCallback) {
     this.hooks[type].push(callback);
   }
 
@@ -150,24 +149,24 @@ export default class Runner {
     this.eventEmitter.on(e, cb);
   }
 
-  async runBeforeAllHook() {
+  async runBeforeAllHook(args: HooksArgs) {
     log(`Runner: beforeAll hooks`);
-    await runParallel(this.hooks.beforeAll);
+    await runParallel(this.hooks.beforeAll, args);
   }
 
-  async runAfterAllHook() {
+  async runAfterAllHook(args: HooksArgs) {
     log(`Runner: afterAll hooks`);
-    await runParallel(this.hooks.afterAll);
+    await runParallel(this.hooks.afterAll, args);
   }
 
-  async runBeforeHook(journey: Journey) {
+  async runBeforeHook(journey: Journey, args: HooksArgs) {
     log(`Runner: before hooks for (${journey.name})`);
-    await runParallel(journey.hooks.before);
+    await runParallel(journey.hooks.before, args);
   }
 
-  async runAfterHook(journey: Journey) {
+  async runAfterHook(journey: Journey, args: HooksArgs) {
     log(`Runner: after hooks for (${journey.name})`);
-    await runParallel(journey.hooks.after);
+    await runParallel(journey.hooks.after, args);
   }
 
   async runStep(
@@ -266,11 +265,8 @@ export default class Runner {
 
   async endJourney(journey, result: JourneyContext & JourneyResult) {
     const { pluginManager, start, params, status, error } = result;
-    const {
-      filmstrips,
-      networkinfo,
-      browserconsole,
-    } = await pluginManager.output();
+    const { filmstrips, networkinfo, browserconsole } =
+      await pluginManager.output();
     this.emit('journey:end', {
       journey,
       status,
@@ -292,7 +288,11 @@ export default class Runner {
     const context = await Runner.createContext(options);
     try {
       this.registerJourney(journey, context);
-      await this.runBeforeHook(journey);
+      const hookArgs = {
+        env: options.environment,
+        params: options.params,
+      };
+      await this.runBeforeHook(journey, hookArgs);
       const stepResults = await this.runSteps(journey, context, options);
       /**
        * Mark journey as failed if any intermediate step fails
@@ -303,7 +303,7 @@ export default class Runner {
           result.error = stepResult.error;
         }
       }
-      await this.runAfterHook(journey);
+      await this.runAfterHook(journey, hookArgs);
     } catch (e) {
       result.status = 'failed';
       result.error = e;
@@ -332,7 +332,10 @@ export default class Runner {
         : reporters[reporter] || reporters['default'];
     new Reporter(this, { fd: outfd });
     this.emit('start', { numJourneys: this.journeys.length });
-    await this.runBeforeAllHook();
+    await this.runBeforeAllHook({
+      env: options.environment,
+      params: options.params,
+    });
     for (const journey of this.journeys) {
       /**
        * Used by heartbeat to gather all registered journeys
@@ -349,7 +352,10 @@ export default class Runner {
       result[journey.name] = journeyResult;
     }
     await Gatherer.stop();
-    await this.runAfterAllHook();
+    await this.runAfterAllHook({
+      env: options.environment,
+      params: options.params,
+    });
     this.reset();
     this.emit('end', {});
     return result;
diff --git a/src/dsl/journey.ts b/src/dsl/journey.ts
index 78e077b..352947b 100644
--- a/src/dsl/journey.ts
+++ b/src/dsl/journey.ts
@@ -25,7 +25,7 @@
 
 import { Browser, Page, BrowserContext, CDPSession } from 'playwright-chromium';
 import { Step } from './step';
-import { VoidCallback } from '../common_types';
+import { VoidCallback, HooksCallback, Params } from '../common_types';
 
 export type JourneyOptions = {
   name: string;
@@ -33,14 +33,13 @@ export type JourneyOptions = {
 };
 
 type HookType = 'before' | 'after';
-export type Hooks = Record<HookType, Array<VoidCallback>>;
-
+export type Hooks = Record<HookType, Array<HooksCallback>>;
 export type JourneyCallback = (options: {
   page: Page;
   context: BrowserContext;
   browser: Browser;
   client: CDPSession;
-  params: Record<string, any>;
+  params: Params;
 }) => void;
 
 export class Journey {
@@ -62,7 +61,7 @@ export class Journey {
     return step;
   }
 
-  addHook(type: HookType, callback: VoidCallback) {
+  addHook(type: HookType, callback: HooksCallback) {
     this.hooks[type].push(callback);
   }
 }
diff --git a/src/helpers.ts b/src/helpers.ts
index 74b7fe6..d0923af 100644
--- a/src/helpers.ts
+++ b/src/helpers.ts
@@ -29,6 +29,7 @@ import { resolve, join, dirname } from 'path';
 import fs, { statSync } from 'fs';
 import { promisify } from 'util';
 import { performance } from 'perf_hooks';
+import { HooksArgs, HooksCallback } from './common_types';
 
 const statAsync = promisify(fs.lstat);
 const readAsync = promisify(fs.readdir);
@@ -90,10 +91,13 @@ export function now() {
 }
 
 /**
- * Execute all the callbacks in parallel using Promise.all
+ * Execute all the hooks callbacks in parallel using Promise.all
  */
-export async function runParallel(callbacks) {
-  const promises = callbacks.map(cb => cb());
+export async function runParallel(
+  callbacks: Array<HooksCallback>,
+  args: HooksArgs
+) {
+  const promises = callbacks.map(cb => cb(args));
   return await Promise.all(promises);
 }
 


Test patch:
diff --git a/__tests__/cli.test.ts b/__tests__/cli.test.ts
index 77e6deb..a7e083e 100644
--- a/__tests__/cli.test.ts
+++ b/__tests__/cli.test.ts
@@ -61,12 +61,25 @@ describe('CLI', () => {
       '-e',
       'testing',
     ]);
-    await cli.waitFor('fake journey');
+    await cli.waitFor('journey/start');
     const output = cli.output();
+    expect(await cli.exitCode).toBe(0);
     expect(JSON.parse(output).payload).toMatchObject({
-      params: { url: 'non-dev', environment: 'testing' },
+      params: { url: 'non-dev' },
+    });
+  });
+
+  it('throw error on modifying params', async () => {
+    const cli = new CLIMock([
+      join(FIXTURES_DIR, 'params-error.journey.ts'),
+      '-j',
+    ]);
+    expect(await cli.exitCode).toBe(1);
+    const output = cli.output();
+    expect(JSON.parse(output).error).toMatchObject({
+      name: 'TypeError',
+      message: 'Cannot add property foo, object is not extensible',
     });
-    expect(await cli.exitCode).toBe(0);
   });
 });
 
diff --git a/__tests__/core/runner.test.ts b/__tests__/core/runner.test.ts
index c8f7d5c..6375d4d 100644
--- a/__tests__/core/runner.test.ts
+++ b/__tests__/core/runner.test.ts
@@ -349,6 +349,43 @@ describe('runner', () => {
     ]);
   });
 
+  it('run - expose params in all hooks', async () => {
+    const result = [];
+    runner.addHook('beforeAll', ({ params, env }) =>
+      result.push({ name: 'beforeAll', params, env })
+    );
+    runner.addHook('afterAll', ({ params }) =>
+      result.push({ name: 'afterAll', params })
+    );
+    const j1 = new Journey({ name: 'j1' }, noop);
+    j1.addHook('before', ({ params }) => {
+      result.push({ name: 'before', params });
+    });
+    j1.addHook('after', ({ params, env }) => {
+      result.push({ name: 'after', params, env });
+    });
+    j1.addStep('s1', () => result.push('step1'));
+    runner.addJourney(j1);
+
+    const params = {
+      url: 'http://local.dev',
+    };
+    await runner.run({
+      wsEndpoint,
+      params,
+      environment: 'testing',
+      outfd: fs.openSync(dest, 'w'),
+    });
+
+    expect(result).toEqual([
+      { name: 'beforeAll', params, env: 'testing' },
+      { name: 'before', params },
+      'step1',
+      { name: 'after', params, env: 'testing' },
+      { name: 'afterAll', params },
+    ]);
+  });
+
   it('run - supports custom reporters', async () => {
     let reporter;
     class Reporter {
diff --git a/__tests__/fixtures/params-error.journey.ts b/__tests__/fixtures/params-error.journey.ts
new file mode 100644
index 0000000..1d80bfe
--- /dev/null
+++ b/__tests__/fixtures/params-error.journey.ts
@@ -0,0 +1,33 @@
+/**
+ * MIT License
+ *
+ * Copyright (c) 2020-present, Elastic NV
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ */
+
+import { journey, step, before } from '../../src';
+
+journey('journey 1', ({}) => {
+  before(({ params }) => {
+    params.foo = 'bar';
+  });
+  step('step1', () => {});
+});
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index f58359a..a0ae8df 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -61,8 +61,9 @@ describe('Run', () => {
       .mockImplementation(() => Promise.resolve({}));
 
     const runParams: RunOptions = {
+      environment: 'debug',
       params: {
-        environment: 'debug',
+        foo: 'bar',
       },
       headless: false,
       screenshots: true,