Simplify handling of internal values a bit

The weak map that I previously talked about [1] wasn't as I intended it
to be, in this case the map only ever contain a single item.

My intention was to keep a map outside of the "test's runtime", as I am
doing with this change, but I now realize that strings aren't legal
WeakMap keys.

Furthremore, I like that the reference contained in the Cypress
environment is a primitive value. Less surprises this way.

Lastly, because the value known to Cypress is a primitive value and not
something where we're messing with method overloading, I am more
comfortable with reomving the opt-out feature of this.

[1] #908 (comment)

Author: badeball

features/issues/908.feature

@@ -6,40 +6,18 @@ Feature: hide internals from cypress environment
       """
       Feature: a feature
         Scenario: hide internal state by default
-          * the internal state should not be visible
-
-        @reportInternalCucumberState
-        Scenario: expose internal state with tag
-          * the internal state should be visible
+          Then the visible internal state should be a mere reference
       """
     And a file named "cypress/support/step_definitions/steps.js" with:
       """
-      const { inspect } = require('util');
       const { Then } = require("@badeball/cypress-cucumber-preprocessor");
       const { INTERNAL_SPEC_PROPERTIES } = require("@badeball/cypress-cucumber-preprocessor/lib/constants");
-
-      Then("the internal state should not be visible", () => {
+      // From https://github.com/rfrench/chai-uuid/blob/master/index.js.
+      const UUID_V4_EXPR = /^[0-9A-F]{8}-[0-9A-F]{4}-[4][0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
+      Then("the visible internal state should be a mere reference", () => {
         const properties = Cypress.env(INTERNAL_SPEC_PROPERTIES);
-        const serializedProperties = JSON.stringify(properties);
-        const inspectedProperties = inspect(properties, {depth: 0});
-
-        expect(properties).not.to.be.undefined;
-        expect(properties.pickle).to.be.undefined;
-        expect(serializedProperties).to.be.undefined;
-        expect(inspectedProperties).to.be.equal('{}');
-      });
-
-      Then("the internal state should be visible", () => {
-        const properties = Cypress.env(INTERNAL_SPEC_PROPERTIES);
-        const serializedProperties = JSON.stringify(properties);
-        const inspectedProperties = inspect(properties, {depth: 0});
-
-        expect(properties).not.to.be.undefined;
-        expect(properties.pickle).not.to.be.undefined;
-        expect(serializedProperties).not.to.be.undefined;
-        // Must be more than an empty object, cannot test the exact string because
-        // it contains random values in a random order.
-        expect(inspectedProperties).to.have.length.of.at.least(3);
+        expect(properties).to.be.a("string");
+        expect(properties).to.match(UUID_V4_EXPR);
       });
       """
     When I run cypress

lib/assertions.ts

@@ -2,12 +2,16 @@ import { isString } from "./type-guards";
 
 import { homepage } from "../package.json";
 
-export function fail(message: string) {
-  throw new Error(
+export function createError(message: string) {
+  return new Error(
     `${message} (this might be a bug, please report at ${homepage})`
   );
 }
 
+export function fail(message: string) {
+  throw createError(message);
+}
+
 export function assert(value: unknown, message: string): asserts value {
   if (value != null) {
     return;

lib/create-tests.ts

@@ -114,48 +114,29 @@ export interface InternalSuiteProperties {
   isEventHandlersAttached?: boolean;
 }
 
-class InternalSpecPropertiesReference {
-  private static referenceMap = new WeakMap<
-    InternalSpecPropertiesReference,
-    InternalSpecProperties
-  >();
-
-  private constructor() {
-    // Empty constructor just to set private visibility
-  }
-
-  public static new(
-    scenarioName: string,
-    properties: InternalSpecProperties
-  ): InternalSpecPropertiesReference {
-    const reference = new this();
-
-    this.referenceMap.set(reference, properties);
-
-    return reference;
-  }
-
-  public dereference(): InternalSpecProperties {
-    // Non-null assertion is safe because an instance of reference is only given out after it is included
-    // in the reference map.
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    return InternalSpecPropertiesReference.referenceMap.get(this)!;
-  }
-
-  public toJSON(): void {
-    // Noop override to hide the reference from serialization.
-    // Any property pointing to a reference will be ignored by JSON.stringify
-  }
+const internalSpecProperties = new Map<string, InternalSpecProperties>();
+
+function createInternalSpecProperties(
+  properties: InternalSpecProperties
+): string {
+  const reference = uuid();
+  internalSpecProperties.set(reference, properties);
+  return reference;
 }
 
 export function retrieveInternalSpecProperties(): InternalSpecProperties {
-  const envValue = Cypress.env(INTERNAL_SPEC_PROPERTIES);
+  const reference = Cypress.env(INTERNAL_SPEC_PROPERTIES) as string;
 
-  if (envValue instanceof InternalSpecPropertiesReference) {
-    return envValue.dereference();
-  }
+  return assertAndReturn(
+    internalSpecProperties.get(reference),
+    `Expected to find internal spec properties with reference = ${reference}`
+  );
+}
 
-  return envValue;
+function updateInternalSpecProperties(
+  newProperties: Partial<InternalSpecProperties>
+): void {
+  Object.assign(retrieveInternalSpecProperties(), newProperties);
 }
 
 function retrieveInternalSuiteProperties():
@@ -361,9 +342,8 @@ function createPickle(
   };
 
   const internalEnv = {
-    [INTERNAL_SPEC_PROPERTIES]: tags.includes("@reportInternalCucumberState")
-      ? internalProperties
-      : InternalSpecPropertiesReference.new(scenarioName, internalProperties),
+    [INTERNAL_SPEC_PROPERTIES]:
+      createInternalSpecProperties(internalProperties),
   };
 
   const suiteOptions = tags
@@ -819,8 +799,10 @@ export default function createTests(
     /**
      * Repopulate internal properties in case previous test is retried.
      */
-    properties.testCaseStartedId = uuid();
-    properties.remainingSteps = [...properties.allSteps];
+    updateInternalSpecProperties({
+      testCaseStartedId: uuid(),
+      remainingSteps: [...properties.allSteps],
+    });
   });
 
   after(function () {

lib/methods.ts

@@ -1,16 +1,19 @@
+import { Pickle } from "@cucumber/messages";
+
 import parse from "@cucumber/tag-expressions";
+
 import { fromByteArray } from "base64-js";
-import { assertAndReturn } from "./assertions";
+
+import { createError } from "./assertions";
+
 import { collectTagNames } from "./ast-helpers";
 
 import {
   INTERNAL_SPEC_PROPERTIES,
   TASK_CREATE_STRING_ATTACHMENT,
 } from "./constants";
-import {
-  InternalSpecProperties,
-  retrieveInternalSpecProperties,
-} from "./create-tests";
+
+import { retrieveInternalSpecProperties } from "./create-tests";
 
 import { runStepWithLogGroup } from "./cypress";
 
@@ -123,10 +126,13 @@ export const NOT_FEATURE_ERROR =
   "Expected to find internal properties, but didn't. This is likely because you're calling doesFeatureMatch() in a non-feature spec. Use doesFeatureMatch() in combination with isFeature() if you have both feature and non-feature specs";
 
 function doesFeatureMatch(expression: string) {
-  const { pickle } = assertAndReturn(
-    retrieveInternalSpecProperties(),
-    NOT_FEATURE_ERROR
-  ) as InternalSpecProperties;
+  let pickle: Pickle;
+
+  try {
+    pickle = retrieveInternalSpecProperties().pickle;
+  } catch {
+    throw createError(NOT_FEATURE_ERROR);
+  }
 
   return parse(expression).evaluate(collectTagNames(pickle.tags));
 }