chore(logger): expose logger._buffer={}, add tests (#69)

This commit exposes the private buffers publicly
_buffer={events=[],metrics=[],tracking=[]} on the logger instance.

In `3.x` this was exposed on `$beaver` but was removed in later
versions.

Adding this allows us much greater flexibility in testing as well as
allows consuming libraries to assert that they sent the correct events
to the buffer for logging.

This change also allows us to add a lot more tests easily (code coverage
jumped to 73%).

CONTRIBUTING.md

@@ -40,11 +40,12 @@ $ npm run-script cover
 
 ## Submitting Changes
 
-1. Ensure that no errors are generated by ESLint.
-2. Commit your changes in logical chunks, i.e. keep your changes small per single commit.
-3. Locally merge (or rebase) the upstream branch into your topic branch: `$ git pull upstream && git merge`.
-4. Push your topic branch up to your fork: `$ git push origin <topic-branch-name>`.
-5. Open a [Pull Request](https://help.github.com/articles/using-pull-requests) with a clear title and description.
+1. Don't check in your `./dist` folder. You can use this command to ignore it without modifying the .gitignore file: `git update-index —assume-unchanged dist/*.*`
+2. Ensure that no errors are generated by ESLint.
+3. Commit your changes in logical chunks, i.e. keep your changes small per single commit.
+4. Locally merge (or rebase) the upstream branch into your topic branch: `$ git pull upstream && git merge`.
+5. Push your topic branch up to your fork: `$ git push origin <topic-branch-name>`.
+6. Open a [Pull Request](https://help.github.com/articles/using-pull-requests) with a clear title and description.
 
 If you have any questions about contributing, please feel free to contact us by posting your questions on GitHub.
 

karma.conf.js

@@ -9,6 +9,13 @@ export default function configKarma(karma: Object) {
   const karmaConfig = getKarmaConfig(karma, {
     basePath: __dirname,
     webpack: WEBPACK_CONFIG_TEST,
+    // un-comment to show console.log() during tests
+    // client: {
+    //   captureConsole: true,
+    //   mocha: {
+    //     bail: true,
+    //   },
+    // },
   });
 
   karma.set(karmaConfig);

package.json

@@ -99,6 +99,7 @@
     "@commitlint/config-conventional": "^16.2.1",
     "@krakenjs/grumbler-scripts": "^8.0.4",
     "@krakenjs/sync-browser-mocks": "^3.0.0",
+    "@storybook/expect": "^27.5.2-0",
     "cross-env": "^7.0.3",
     "flow-bin": "0.135.0",
     "flow-typed": "^3.8.0",

src/logger.js

@@ -58,6 +58,16 @@ export type LoggerType = {|
 
   setTransport: (Transport) => LoggerType,
   configure: (LoggerOptions) => LoggerType,
+
+  __buffer__: {|
+    events: $ReadOnlyArray<{|
+      level: $Values<typeof LOG_LEVEL>,
+      event: string,
+      payload: Payload,
+    |}>,
+    tracking: $ReadOnlyArray<Payload>,
+    metrics: $ReadOnlyArray<Metric>,
+  |},
 |};
 
 export function Logger({
@@ -289,7 +299,7 @@ export function Logger({
     print(
       LOG_LEVEL.DEBUG,
       `metric.${metricPayload.name}`,
-      metricPayload.dimensions
+      metricPayload.dimensions || {}
     );
 
     metrics.push(metricPayload);
@@ -367,7 +377,20 @@ export function Logger({
     addHeaderBuilder,
     setTransport,
     configure,
+
+    // exposed primarily for testing
+    __buffer__: {
+      events,
+      tracking,
+      metrics,
+    },
   };
 
+  // mark the __buffer__ prop as readonly
+  Object.defineProperty(logger, "__buffer__", { writable: false });
+
+  // mark logger.__buffer__ props as readOnly, though array methods on the children still work.
+  Object.freeze(logger.__buffer__);
+
   return logger;
 }

test/index.js

@@ -1,4 +1,5 @@
 /* @flow */
 
 import "./test";
+import "./logger.test";
 import "./util.test";

test/logger.test.js

@@ -0,0 +1,217 @@
+/* eslint-disable flowtype/require-valid-file-annotation, eslint-comments/disable-enable-pair */
+
+import expect from "@storybook/expect";
+// https://jestjs.io/docs/expect
+
+import { Logger } from "../src";
+
+let logger;
+let logBuf;
+
+describe("beaver exposes the Logger() function that returns a logger instance with the following methods... ", () => {
+  beforeEach(() => {
+    // create a new logger with a new buffer
+    logger = Logger({
+      url: "/test/api/log",
+    });
+
+    logBuf = logger.__buffer__;
+
+    // verify empty (clean slate)
+    expect(logger.__buffer__.events).toHaveLength(0);
+    expect(logger.__buffer__.tracking).toHaveLength(0);
+  });
+
+  it(".info(evtName, payload?) sends an info event obj to the events[] buffer ready for logging", () => {
+    const expected = {
+      event: "testing",
+      level: "info",
+      payload: {
+        // timestamp: "1677866574862"  -- dynamic so commenting out
+      },
+    };
+
+    const expectedWithPayload = {
+      event: "testingWithPayload",
+      level: "info",
+      payload: {
+        country: "Mordor",
+      },
+    };
+
+    // 1) Without payload
+    logger.info("testing");
+    expect(logBuf.events).toHaveLength(1);
+    expect(logBuf.events[0]).toMatchObject(expected);
+
+    // 2) With payload
+    logger.info("testingWithPayload", { country: "Mordor" });
+    expect(logBuf.events).toHaveLength(2);
+    expect(logBuf.events[1]).toMatchObject(expectedWithPayload);
+  });
+
+  it(".warn(evtName, payload?) sends a warn event obj to the events[] buffer ready for logging", () => {
+    const expected = {
+      event: "testing",
+      level: "warn",
+      payload: {},
+    };
+
+    const expectedWithPayload = {
+      event: "testingWithPayload",
+      level: "warn",
+      payload: {
+        country: "France",
+      },
+    };
+
+    // 1) without payload
+    logger.warn("testing");
+    expect(logBuf.events).toHaveLength(1);
+    expect(logBuf.events[0]).toMatchObject(expected);
+
+    // 2) with payload
+    logger.warn("testingWithPayload", { country: "France" });
+    expect(logBuf.events).toHaveLength(2);
+    expect(logBuf.events[1]).toMatchObject(expectedWithPayload);
+  });
+
+  it(".error(evtName, payload?) sends an error event obj to the events[] buffer ready for logging", () => {
+    const expected = {
+      event: "house is on fire!!!",
+      level: "error",
+      payload: {},
+    };
+
+    const expectedWithPayload = {
+      event: "oh no",
+      level: "error",
+      payload: {
+        country: "DE",
+      },
+    };
+
+    // 1) without payload
+    logger.error("house is on fire!!!");
+    expect(logBuf.events).toHaveLength(1);
+    expect(logBuf.events[0]).toMatchObject(expected);
+
+    // 2) with payload
+    logger.error("oh no", { country: "DE" });
+    expect(logBuf.events).toHaveLength(2);
+    expect(logBuf.events[1]).toMatchObject(expectedWithPayload);
+  });
+
+  it(".debug(evtName, payload?) sends a debug event obj to the events[] buffer ready for logging", () => {
+    const expected = {
+      event: "gollum_personality_count",
+      level: "debug",
+      payload: {},
+    };
+
+    const expectedWithPayload = {
+      event: "frodo_personality_count",
+      level: "debug",
+      payload: {
+        how_many: 5,
+      },
+    };
+
+    // 1) without payload
+    logger.debug("gollum_personality_count");
+    expect(logBuf.events).toHaveLength(1);
+    expect(logBuf.events[0]).toMatchObject(expected);
+
+    // 2) with payload
+    logger.debug("frodo_personality_count", { how_many: 5 });
+    expect(logBuf.events).toHaveLength(2);
+    expect(logBuf.events[1]).toMatchObject(expectedWithPayload);
+  });
+
+  it(".track(payload) sends general tracking info to the tracking[] buffer and will be sent with next flush", () => {
+    const expected = {
+      token: "12345",
+      country: "FR",
+      locale: {
+        country: "FR",
+        locale: "fr_FR",
+      },
+    };
+
+    //
+    logger.track(expected);
+    expect(logBuf.tracking).toHaveLength(1);
+    expect(logBuf.tracking[0]).toMatchObject(expected);
+  });
+
+  it(".metric(payload) sends general tracking info to the metrics[] buffer and will be sent with next flush", () => {
+    const expected = {
+      name: "pp.xo.ui.lite.weasley.fallback-error",
+      metricValue: 5,
+      dimensions: {
+        country: "FR",
+        locale: "fr_FR",
+      },
+    };
+
+    // 1) Won't error on empty / missing payload props
+    logger.metric({});
+    expect(logBuf.metrics).toHaveLength(1);
+    expect(logBuf.metrics[0]).toMatchObject({});
+
+    // 2) Normal expected payload shape for .metric()
+    logger.metric(expected);
+    expect(logBuf.metrics).toHaveLength(2);
+    expect(logBuf.metrics[1]).toMatchObject(expected);
+  });
+});
+
+describe("a beaver-logger instance exposes logger.__buffer__={} prop as a readonly object that...", () => {
+  it("doesn't allow its immedate children props to be reassigned", () => {
+    const loggerInstance = Logger({
+      url: "/test/api/log",
+    });
+
+    // verify initial empty shape of buffer
+    expect(loggerInstance.__buffer__).toStrictEqual({
+      events: [],
+      metrics: [],
+      tracking: [],
+    });
+
+    // verify we disallow re-assigning __buffer__ or its arrays
+    expect(() => (loggerInstance.__buffer__ = {})).toThrow(
+      "Cannot assign to read only property '__buffer__' of object '#<Object>'"
+    );
+    expect(() => (loggerInstance.__buffer__.events = [])).toThrow(
+      "Cannot assign to read only property 'events' of object '#<Object>'"
+    );
+
+    // but using the buffer array methods directly is still allowed
+    loggerInstance.__buffer__.metrics.push(1, 2, 3);
+    loggerInstance.__buffer__.events.push(2);
+    loggerInstance.__buffer__.tracking.push(3);
+
+    expect(loggerInstance.__buffer__).toStrictEqual({
+      events: [2],
+      metrics: [1, 2, 3],
+      tracking: [3],
+    });
+  });
+});
+
+/*
+    TODO: Need to write unit tests for these exposed methods:
+    ush,
+    immediateFlush,
+    addPayloadBuilder,
+    addMetaBuilder,
+    addTrackingBuilder,
+    addHeaderBuilder,
+    setTransport,
+    configure,
+    
+    describe(' ', () => {
+  
+  });
+*/