Repository: elastic/synthetics (ts)

Issue:
Add screenshot options
`--screenshots` should take an option of: `off`, `on`, `only-on-failure` or `space-efficient`.

`space-efficient` be the default in heartbeat and would retain just the last step's screenshot on successful runs, and retain all steps on errors. This is important because screenshots, even with the efficiency gains we've recently gotten, just take up too much space and cost too much. Even a handful of journeys can takeup ~1g/day with those optimizations. This should cut down disk usage dramatically. 

Counterpart of https://github.com/elastic/beats/issues/25201

@liciavale  @drewpost @paulb-elastic @vigneshshanmugam thoughts on this approach? For the `space-efficient` option it'd mean that on the steps page in Uptime we'd usually have missing screenshots.

Pull request:
feat: add support for screenshot toggle
+ part of #308 
+ Extends the previous `--screenshot` option with `on|off|only-on-failure` and defaulting to `on`. 
```
on - capture screenshots for all step in a journey
off - do not capture any screenshots
only-on-failure - capture screenshots for all steps when anyone of the journey fails (any step in a journey fails). 
```
+ This is a non-breaking change and backwards compatible with previous boolean flag. 

Patch:
diff --git a/examples/todos/basic.journey.ts b/examples/todos/basic.journey.ts
index c22e26c..71f0571 100644
--- a/examples/todos/basic.journey.ts
+++ b/examples/todos/basic.journey.ts
@@ -1,5 +1,4 @@
-import { journey, step } from '@elastic/synthetics';
-import { deepStrictEqual } from 'assert';
+import { journey, step, expect } from '@elastic/synthetics';
 import { join } from 'path';
 
 journey('check if title is present', ({ page }) => {
@@ -10,7 +9,7 @@ journey('check if title is present', ({ page }) => {
 
   step('assert title', async () => {
     const header = await page.$('h1');
-    deepStrictEqual(await header.textContent(), 'todos');
+    expect(await header.textContent()).toBe('todos');
   });
 });
 
@@ -22,8 +21,7 @@ journey('check if input placeholder is correct', ({ page }) => {
 
   step('assert placeholder value', async () => {
     const input = await page.$('input.new-todo');
-    deepStrictEqual(
-      await input.getAttribute('placeholder'),
+    expect(await input.getAttribute('placeholder')).toBe(
       'What needs to be done?'
     );
   });
diff --git a/examples/todos/helpers.ts b/examples/todos/helpers.ts
index b86ed1c..4a0a542 100644
--- a/examples/todos/helpers.ts
+++ b/examples/todos/helpers.ts
@@ -1,5 +1,4 @@
-import { step, Page } from '@elastic/synthetics';
-import * as assert from 'assert';
+import { step, Page, expect } from '@elastic/synthetics';
 import { join } from 'path';
 
 export const loadAppStep = (page: Page) => {
@@ -25,7 +24,7 @@ export const findTask = async (page: Page, task: string) => {
 
 export const assertTaskListSizeStep = async (page: Page, size: number) => {
   step(`check that task list has exactly ${size} elements`, async () => {
-    assert.deepEqual((await page.$$(todosSelector)).length, size);
+    expect((await page.$$(todosSelector)).length).toBe(size);
   });
 };
 
diff --git a/src/common_types.ts b/src/common_types.ts
index 0440d46..7641596 100644
--- a/src/common_types.ts
+++ b/src/common_types.ts
@@ -114,13 +114,15 @@ export type PluginOutput = {
   metrics?: PerfMetrics;
 };
 
+export type ScreenshotOptions = 'on' | 'off' | 'only-on-failure';
+
 export type CliArgs = {
   capability?: Array<string>;
   config?: string;
   environment?: string;
   outfd?: number;
   headless?: boolean;
-  screenshots?: boolean;
+  screenshots?: ScreenshotOptions;
   ssblocks?: boolean;
   metrics?: boolean;
   filmstrips?: boolean;
diff --git a/src/core/runner.ts b/src/core/runner.ts
index a16e7fc..2872cf5 100644
--- a/src/core/runner.ts
+++ b/src/core/runner.ts
@@ -107,7 +107,7 @@ interface Events {
     JourneyResult &
     PluginOutput & {
       journey: Journey;
-      ssblocks?: boolean;
+      options: RunOptions;
     };
   'journey:end:reported': unknown;
   'step:start': { journey: Journey; step: Step };
@@ -144,6 +144,31 @@ export default class Runner extends EventEmitter {
     };
   }
 
+  async captureScreenshot(page: Driver['page'], step: Step) {
+    await page.waitForLoadState('load');
+    const buffer = await page
+      .screenshot({
+        type: 'jpeg',
+        quality: 80,
+      })
+      .catch(() => {});
+    /**
+     * Write the screenshot image buffer with additional details (step
+     * information) which could be extracted at the end of
+     * each journey without impacting the step timing information
+     */
+    if (buffer) {
+      const fileName = now().toString() + '.json';
+      writeFileSync(
+        join(this.screenshotPath, fileName),
+        JSON.stringify({
+          step,
+          data: buffer.toString('base64'),
+        })
+      );
+    }
+  }
+
   addHook(type: HookType, callback: HooksCallback) {
     this.hooks[type].push(callback);
   }
@@ -207,25 +232,8 @@ export default class Runner extends EventEmitter {
       data.error = error;
     } finally {
       data.url ??= driver.page.url();
-      if (screenshots) {
-        await driver.page.waitForLoadState('load');
-        const buffer = await driver.page.screenshot({
-          type: 'jpeg',
-          quality: 80,
-        });
-        /**
-         * Write the screenshot image buffer with additional details (step
-         * information) which could be extracted at the end of
-         * each journey without impacting the step timing information
-         */
-        const fileName = now().toString() + '.json';
-        writeFileSync(
-          join(this.screenshotPath, fileName),
-          JSON.stringify({
-            step,
-            data: buffer.toString('base64'),
-          })
-        );
+      if (screenshots && screenshots !== 'off') {
+        await this.captureScreenshot(driver.page, step);
       }
     }
     log(`Runner: end step (${step.name})`);
@@ -292,7 +300,7 @@ export default class Runner extends EventEmitter {
       params,
       start,
       end: monotonicTimeInSeconds(),
-      ssblocks: options.ssblocks,
+      options,
       ...pluginOutput,
       browserconsole: status == 'failed' ? pluginOutput.browserconsole : [],
     });
@@ -300,7 +308,7 @@ export default class Runner extends EventEmitter {
      * Wait for the all the reported events to be consumed aschronously
      * by reporter.
      */
-    if (options.reporter == 'json') {
+    if (options.reporter === 'json') {
       await once(this, 'journey:end:reported');
     }
   }
@@ -383,8 +391,8 @@ export default class Runner extends EventEmitter {
       }
       const journeyResult = await this.runJourney(journey, options);
       result[journey.name] = journeyResult;
-      await Gatherer.stop();
     }
+    await Gatherer.stop();
     await this.runAfterAllHook({
       env: options.environment,
       params: options.params,
diff --git a/src/parse_args.ts b/src/parse_args.ts
index d8f33f0..61b6788 100644
--- a/src/parse_args.ts
+++ b/src/parse_args.ts
@@ -59,7 +59,11 @@ program
     '--capability <features...>',
     'Enable capabilities through feature flags'
   )
-  .option('--screenshots', 'take screenshot for each step')
+  .addOption(
+    new Option('--screenshots [flag]', 'take screenshots at end of each step')
+      .choices(['on', 'off', 'only-on-failure'])
+      .default('on')
+  )
   .option('--network', 'capture network information for all journeys')
   .option(
     '--dry-run',
@@ -98,7 +102,7 @@ const options = command.opts() as CliArgs;
  */
 if (options.richEvents) {
   options.reporter = options.reporter ?? 'json';
-  options.screenshots = true;
+  options.screenshots = 'on';
   options.network = true;
 }
 
diff --git a/src/reporters/json.ts b/src/reporters/json.ts
index 7db8624..f169df5 100644
--- a/src/reporters/json.ts
+++ b/src/reporters/json.ts
@@ -415,42 +415,30 @@ export default class JSONReporter extends BaseReporter {
         metrics,
         status,
         error,
-        ssblocks,
+        options,
       }) => {
-        await gatherScreenshots(
-          join(CACHE_PATH, 'screenshots'),
-          async (step, data) => {
-            if (ssblocks) {
-              const { blob_mime, blocks, reference } =
-                await getScreenshotBlocks(Buffer.from(data, 'base64'));
-              for (let i = 0; i < blocks.length; i++) {
-                const block = blocks[i];
+        const { ssblocks, screenshots } = options;
+        const writeScreenshots =
+          screenshots === 'on' ||
+          (screenshots === 'only-on-failure' && status === 'failed');
+        if (writeScreenshots) {
+          await gatherScreenshots(
+            join(CACHE_PATH, 'screenshots'),
+            async (step, data) => {
+              if (ssblocks) {
+                await this.writeScreenshotBlocks(journey, step, data);
+              } else {
                 this.writeJSON({
-                  type: 'screenshot/block',
-                  _id: block.id,
-                  blob: block.blob,
-                  blob_mime,
+                  type: 'step/screenshot',
+                  journey,
+                  step,
+                  blob: data,
+                  blob_mime: 'image/jpeg',
                 });
               }
-              this.writeJSON({
-                type: 'step/screenshot_ref',
-                journey,
-                step,
-                root_fields: {
-                  screenshot_ref: reference,
-                },
-              });
-            } else {
-              this.writeJSON({
-                type: 'step/screenshot',
-                journey,
-                step,
-                blob: data,
-                blob_mime: 'image/jpeg',
-              });
             }
-          }
-        );
+          );
+        }
 
         if (networkinfo) {
           networkinfo.forEach(ni => {
@@ -504,11 +492,34 @@ export default class JSONReporter extends BaseReporter {
             status,
           },
         });
-        this.runner.emit('journey:end:reported', {});
+        process.nextTick(() => this.runner.emit('journey:end:reported', {}));
       }
     );
   }
 
+  async writeScreenshotBlocks(journey: Journey, step: Step, data: string) {
+    const { blob_mime, blocks, reference } = await getScreenshotBlocks(
+      Buffer.from(data, 'base64')
+    );
+    for (let i = 0; i < blocks.length; i++) {
+      const block = blocks[i];
+      this.writeJSON({
+        type: 'screenshot/block',
+        _id: block.id,
+        blob: block.blob,
+        blob_mime,
+      });
+    }
+    this.writeJSON({
+      type: 'step/screenshot_ref',
+      journey,
+      step,
+      root_fields: {
+        screenshot_ref: reference,
+      },
+    });
+  }
+
   writeMetrics(
     journey: Journey,
     type: string,


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index 3da1a97..f5682d0 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -66,7 +66,7 @@ describe('Run', () => {
         foo: 'bar',
       },
       headless: false,
-      screenshots: true,
+      screenshots: 'on',
       filmstrips: false,
       trace: false,
       dryRun: true,
diff --git a/__tests__/reporters/__snapshots__/json.test.ts.snap b/__tests__/reporters/__snapshots__/json.test.ts.snap
index fea6f0a..0132167 100644
--- a/__tests__/reporters/__snapshots__/json.test.ts.snap
+++ b/__tests__/reporters/__snapshots__/json.test.ts.snap
@@ -276,7 +276,7 @@ Object {
 }
 `;
 
-exports[`json reporter write screenshot block & reference docs 1`] = `
+exports[`json reporter screenshots write block & reference docs 1`] = `
 "{\\"type\\":\\"screenshot/block\\",\\"_id\\":\\"e89af7cc2cae152df54ccf091793db64b3dfe995\\",\\"@timestamp\\":1600300800000000,\\"root_fields\\":{\\"os\\":{\\"platform\\":\\"darwin\\"},\\"package\\":{\\"name\\":\\"@elastic/synthetics\\",\\"version\\":\\"0.0.1\\"}},\\"blob\\":\\"/9j/2wBDAAYEBQYFBAYGBQYHBwYIChAKCgkJChQODwwQFxQYGBcUFhYaHSUfGhsjHBYWICwgIyYnKSopGR8tMC0oMCUoKSj/2wBDAQcHBwoIChMKChMoGhYaKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCj/wAARCABaAKADASIAAhEBAxEB/8QAFQABAQAAAAAAAAAAAAAAAAAAAAj/xAAUEAEAAAAAAAAAAAAAAAAAAAAA/8QAFAEBAAAAAAAAAAAAAAAAAAAAAP/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAMAwEAAhEDEQA/AKcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB//9k=\\",\\"blob_mime\\":\\"image/jpeg\\",\\"package_version\\":\\"0.0.1\\"}
 {\\"type\\":\\"screenshot/block\\",\\"_id\\":\\"e89af7cc2cae152df54ccf091793db64b3dfe995\\",\\"@timestamp\\":1600300800000000,\\"root_fields\\":{\\"os\\":{\\"platform\\":\\"darwin\\"},\\"package\\":{\\"name\\":\\"@elastic/synthetics\\",\\"version\\":\\"0.0.1\\"}},\\"blob\\":\\"/9j/2wBDAAYEBQYFBAYGBQYHBwYIChAKCgkJChQODwwQFxQYGBcUFhYaHSUfGhsjHBYWICwgIyYnKSopGR8tMC0oMCUoKSj/2wBDAQcHBwoIChMKChMoGhYaKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCj/wAARCABaAKADASIAAhEBAxEB/8QAFQABAQAAAAAAAAAAAAAAAAAAAAj/xAAUEAEAAAAAAAAAAAAAAAAAAAAA/8QAFAEBAAAAAAAAAAAAAAAAAAAAAP/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAMAwEAAhEDEQA/AKcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB//9k=\\",\\"blob_mime\\":\\"image/jpeg\\",\\"package_version\\":\\"0.0.1\\"}
 {\\"type\\":\\"screenshot/block\\",\\"_id\\":\\"e89af7cc2cae152df54ccf091793db64b3dfe995\\",\\"@timestamp\\":1600300800000000,\\"root_fields\\":{\\"os\\":{\\"platform\\":\\"darwin\\"},\\"package\\":{\\"name\\":\\"@elastic/synthetics\\",\\"version\\":\\"0.0.1\\"}},\\"blob\\":\\"/9j/2wBDAAYEBQYFBAYGBQYHBwYIChAKCgkJChQODwwQFxQYGBcUFhYaHSUfGhsjHBYWICwgIyYnKSopGR8tMC0oMCUoKSj/2wBDAQcHBwoIChMKChMoGhYaKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCgoKCj/wAARCABaAKADASIAAhEBAxEB/8QAFQABAQAAAAAAAAAAAAAAAAAAAAj/xAAUEAEAAAAAAAAAAAAAAAAAAAAA/8QAFAEBAAAAAAAAAAAAAAAAAAAAAP/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAMAwEAAhEDEQA/AKcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB//9k=\\",\\"blob_mime\\":\\"image/jpeg\\",\\"package_version\\":\\"0.0.1\\"}
diff --git a/__tests__/reporters/base.test.ts b/__tests__/reporters/base.test.ts
index 9810693..da871d1 100644
--- a/__tests__/reporters/base.test.ts
+++ b/__tests__/reporters/base.test.ts
@@ -106,6 +106,7 @@ describe('base reporter', () => {
       error,
       start: 0,
       end: 1,
+      options: {},
     });
     runner.emit('end', 'done');
     /**
diff --git a/__tests__/reporters/json.test.ts b/__tests__/reporters/json.test.ts
index b3216aa..82e2954 100644
--- a/__tests__/reporters/json.test.ts
+++ b/__tests__/reporters/json.test.ts
@@ -35,6 +35,7 @@ import JSONReporter, {
 import * as helpers from '../../src/helpers';
 import Runner from '../../src/core/runner';
 import { NETWORK_INFO } from '../fixtures/networkinfo';
+import { StatusValue } from '../../src/common_types';
 
 /**
  * Mock package version to avoid breaking JSON payload
@@ -123,6 +124,7 @@ describe('json reporter', () => {
       status: 'succeeded',
       start: 0,
       end: 11,
+      options: {},
       filmstrips: [
         {
           blob: 'dummy',
@@ -216,6 +218,7 @@ describe('json reporter', () => {
       end: 1,
       status: 'failed',
       error: myErr,
+      options: {},
     });
 
     const journeyEnd = (await readAndCloseStreamJson()).find(
@@ -231,6 +234,7 @@ describe('json reporter', () => {
       start: 0,
       end: 1,
       status: 'skipped',
+      options: {},
     });
 
     const journeyEnd = (await readAndCloseStreamJson()).find(
@@ -268,49 +272,83 @@ describe('json reporter', () => {
     expect(screenshot1).toEqual(screenshot2);
   });
 
-  it('write screenshot blob data', async () => {
+  describe('screenshots', () => {
     const sourceDir = join(FIXTURES_DIR, 'screenshots');
     const destDir = join(helpers.CACHE_PATH, 'screenshots');
-    mkdirSync(destDir, { recursive: true });
-    fs.copyFileSync(
-      join(sourceDir, 'content.json'),
-      join(destDir, 'content.json')
-    );
-    runner.emit('journey:end', {
-      journey: j1,
-      start: 0,
-      status: 'failed',
-      ssblocks: false,
+    beforeAll(() => {
+      mkdirSync(destDir, { recursive: true });
+      fs.copyFileSync(
+        join(sourceDir, 'content.json'),
+        join(destDir, 'content.json')
+      );
     });
-    const stepEnd = (await readAndCloseStreamJson()).find(
-      json => json.type == 'step/screenshot'
-    );
-    expect(stepEnd).toMatchObject({
-      step: {
-        name: 'launch app',
-        index: 1,
-      },
-      blob: expect.any(String),
-      blob_mime: 'image/jpeg',
+
+    afterAll(() => {
+      fs.rmdirSync(destDir, { recursive: true });
     });
-    fs.rmdirSync(destDir, { recursive: true });
-  });
 
-  it('write screenshot block & reference docs', async () => {
-    const sourceDir = join(FIXTURES_DIR, 'screenshots');
-    const destDir = join(helpers.CACHE_PATH, 'screenshots');
-    mkdirSync(destDir, { recursive: true });
-    fs.copyFileSync(
-      join(sourceDir, 'content.json'),
-      join(destDir, 'content.json')
-    );
-    runner.emit('journey:end', {
-      journey: j1,
-      start: 0,
-      status: 'failed',
-      ssblocks: true,
+    const emitEnd = (options, status = 'failed' as StatusValue) =>
+      runner.emit('journey:end', {
+        journey: j1,
+        start: 0,
+        status,
+        options,
+      });
+
+    it('write whole blobs data ', async () => {
+      emitEnd({
+        screenshots: 'on',
+        ssblocks: false,
+      });
+      const screenshotJson = (await readAndCloseStreamJson()).find(
+        json => json.type == 'step/screenshot'
+      );
+      expect(screenshotJson).toMatchObject({
+        step: {
+          name: 'launch app',
+          index: 1,
+        },
+        blob: expect.any(String),
+        blob_mime: 'image/jpeg',
+      });
+    });
+
+    it('write block & reference docs', async () => {
+      emitEnd({
+        screenshots: 'on',
+        ssblocks: true,
+      });
+      expect((await readAndCloseStream()).toString()).toMatchSnapshot();
+    });
+
+    it('dont write on only-on-failure for successful journey', async () => {
+      emitEnd(
+        {
+          screenshots: 'only-on-failure',
+        },
+        'succeeded'
+      );
+      const screenshotJson = (await readAndCloseStreamJson()).find(
+        json => json.type == 'step/screenshot'
+      );
+      expect(screenshotJson).not.toBeDefined();
+    });
+
+    it('write on only-on-failure for failed journey', async () => {
+      emitEnd({
+        screenshots: 'only-on-failure',
+      });
+      const screenshotJson = (await readAndCloseStreamJson()).find(
+        json => json.type == 'step/screenshot'
+      );
+      expect(screenshotJson).toMatchObject({
+        step: {
+          name: 'launch app',
+          index: 1,
+        },
+        blob: expect.any(String),
+        blob_mime: 'image/jpeg',
+      });
     });
-    expect((await readAndCloseStream()).toString()).toMatchSnapshot();
-    fs.rmdirSync(destDir, { recursive: true });
   });
 });
diff --git a/__tests__/reporters/junit.test.ts b/__tests__/reporters/junit.test.ts
index f1e6201..375c3a4 100644
--- a/__tests__/reporters/junit.test.ts
+++ b/__tests__/reporters/junit.test.ts
@@ -83,6 +83,7 @@ describe('junit reporter', () => {
       journey: j1,
       start: 0,
       status: 'failed',
+      options: {},
     });
     runner.emit('end', 'done');
     /**
@@ -116,6 +117,7 @@ describe('junit reporter', () => {
       journey: j1,
       start: 0,
       status: 'failed',
+      options: {},
     });
     runner.emit('end', 'done');
     stream.end();