feat: collect logs for successful journey as well (#817)

* PR feedback

* revert

* update

* add tests and fix algo

* Update src/plugins/browser-console.ts

* Update src/plugins/browser-console.ts

---------

Co-authored-by: vigneshshanmugam <[email protected]>

Author: shahzad31

__tests__/plugins/browser-console.test.ts

@@ -24,7 +24,8 @@
  */
 
 import { Gatherer } from '../../src/core/gatherer';
-import { BrowserConsole } from '../../src/plugins';
+import { BrowserConsole, filterBrowserMessages } from '../../src/plugins';
+import { BrowserMessage } from '../../src/common_types';
 import { Server } from '../utils/server';
 import { wsEndpoint } from '../utils/test-config';
 
@@ -122,4 +123,59 @@ describe('BrowserConsole', () => {
     expect(unhandledError?.step).toEqual(currentStep);
     expect(unhandledError?.error?.stack).toContain('Error: Boom');
   });
+
+  describe('Filtering', () => {
+    function getBrowserMessages({ errors = 100, warnings = 100, log = 100 }) {
+      const messages: BrowserMessage[] = [];
+      for (let i = 0; i < errors; i++) {
+        messages.push({ type: 'error', text: `error ${i}`, timestamp: 0 });
+      }
+      for (let i = 0; i < warnings; i++) {
+        messages.push({ type: 'warning', text: `warning ${i}`, timestamp: 0 });
+      }
+      for (let i = 0; i < log; i++) {
+        messages.push({ type: 'log', text: `log ${i}`, timestamp: 0 });
+      }
+      return messages;
+    }
+
+    it('skipped journey', () => {
+      expect(
+        filterBrowserMessages(getBrowserMessages({}), 'skipped').length
+      ).toEqual(0);
+    });
+
+    it('failed journey', () => {
+      expect(
+        filterBrowserMessages(getBrowserMessages({}), 'failed').length
+      ).toEqual(300);
+    });
+
+    it('successful journey', () => {
+      expect(
+        filterBrowserMessages(
+          getBrowserMessages({ errors: 10, warnings: 10, log: 10 }),
+          'succeeded'
+        ).length
+      ).toEqual(30);
+
+      expect(
+        filterBrowserMessages(getBrowserMessages({}), 'succeeded').length
+      ).toEqual(100);
+
+      const withoutLog = filterBrowserMessages(
+        getBrowserMessages({ errors: 10, log: 200 }),
+        'succeeded'
+      );
+      expect(withoutLog.every(msg => msg.type !== 'log')).toBe(true);
+      expect(withoutLog.length).toEqual(100);
+
+      const withLog = filterBrowserMessages(
+        getBrowserMessages({ errors: 5, warnings: 5 }),
+        'succeeded'
+      );
+      expect(withLog.some(msg => msg.type === 'log')).toBe(true);
+      expect(withLog.length).toEqual(100);
+    });
+  });
 });

src/core/runner.ts

@@ -46,8 +46,11 @@ import {
   StepResult,
   PushOptions,
 } from '../common_types';
-import { PluginManager } from '../plugins';
-import { PerformanceManager } from '../plugins';
+import {
+  PluginManager,
+  PerformanceManager,
+  filterBrowserMessages,
+} from '../plugins';
 import { Gatherer } from './gatherer';
 import { log } from './logger';
 import { Monitor, MonitorConfig } from '../dsl/monitor';
@@ -310,7 +313,10 @@ export default class Runner {
       timestamp: getTimestamp(),
       options,
       ...pluginOutput,
-      browserconsole: status == 'failed' ? pluginOutput.browserconsole : [],
+      browserconsole: filterBrowserMessages(
+        pluginOutput.browserconsole,
+        status
+      ),
     });
     // clear screenshots cache after each journey
     await rm(Runner.screenshotPath, { recursive: true, force: true });

src/plugins/browser-console.ts

@@ -24,12 +24,15 @@
  */
 
 import type { ConsoleMessage } from 'playwright-core';
-import { BrowserMessage, Driver } from '../common_types';
+import { BrowserMessage, Driver, StatusValue } from '../common_types';
 import { log } from '../core/logger';
 import { Step } from '../dsl';
 import { getTimestamp } from '../helpers';
 
-const defaultMessageLimit = 1000;
+const DEFAULT_MSG_LIMIt = 1000;
+const SUCCESSFUL_MSG_LIMIT = 100;
+
+const allowedTypes = ['error', 'warning', 'log'];
 
 export class BrowserConsole {
   private messages: BrowserMessage[] = [];
@@ -42,7 +45,7 @@ export class BrowserConsole {
       return;
     }
     const type = msg.type();
-    if (type === 'error' || type === 'warning') {
+    if (allowedTypes.includes(type)) {
       const { name, index } = this._currentStep;
       this.messages.push({
         timestamp: getTimestamp(),
@@ -72,7 +75,7 @@ export class BrowserConsole {
   };
 
   private enforceMessagesLimit() {
-    if (this.messages.length > defaultMessageLimit) {
+    if (this.messages.length > DEFAULT_MSG_LIMIt) {
       this.messages.splice(0, 1);
     }
   }
@@ -90,3 +93,30 @@ export class BrowserConsole {
     return this.messages;
   }
 }
+
+export function filterBrowserMessages(
+  messages: BrowserMessage[],
+  status: StatusValue
+) {
+  if (status == 'skipped') {
+    return [];
+  } else if (status === 'failed' || messages.length <= SUCCESSFUL_MSG_LIMIT) {
+    return messages;
+  }
+  // collect 100 messages from the browser console when the test is successful,
+  // giving priority to errors and warnings
+  const result = messages.filter(msg => msg.type === 'error');
+  if (result.length >= SUCCESSFUL_MSG_LIMIT) {
+    return result.slice(-SUCCESSFUL_MSG_LIMIT);
+  }
+
+  // collect warnings
+  result.push(...messages.filter(msg => msg.type === 'warning'));
+  if (result.length >= SUCCESSFUL_MSG_LIMIT) {
+    return result.slice(-SUCCESSFUL_MSG_LIMIT);
+  }
+
+  // collect logs
+  result.push(...messages.filter(msg => msg.type === 'log'));
+  return result.slice(-SUCCESSFUL_MSG_LIMIT);
+}