Fix: resolve TestHub build creation from the normalised product flags

nightwatch/globals.js

@@ -776,6 +776,8 @@ const cucumberPatcher = () => {
   }
 };
 
+let missingTestHubBuildWarned = false;
+
 const addProductMapAndbuildUuidCapability = (settings) => {
   try {
     if (!settings?.desiredCapabilities) {
@@ -793,12 +795,23 @@ const addProductMapAndbuildUuidCapability = (settings) => {
       percy: false
     };
 
+    const testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID || '';
+
+    // An empty uuid on a run that intended a TestHub build leaves every session
+    // unlinkable, and used to be entirely silent.
+    const testHubBuildIntended = buildProductMap.observability || buildProductMap.accessibility ||
+      process.env.BROWSERSTACK_TESTHUB_BUILD_ATTEMPTED === 'true';
+    if (!testhubBuildUuid && testHubBuildIntended && !missingTestHubBuildWarned) {
+      missingTestHubBuildWarned = true;
+      Logger.warn('No TestHub build was created for this run, so its sessions cannot be linked to test reporting or accessibility data.');
+    }
+
     if (settings.desiredCapabilities['bstack:options']) {
       settings.desiredCapabilities['bstack:options']['buildProductMap'] = buildProductMap;
-      settings.desiredCapabilities['bstack:options']['testhubBuildUuid'] = process.env.BROWSERSTACK_TESTHUB_UUID ? process.env.BROWSERSTACK_TESTHUB_UUID : '' ;
+      settings.desiredCapabilities['bstack:options']['testhubBuildUuid'] = testhubBuildUuid;
     } else {
       settings.desiredCapabilities['browserstack.buildProductMap'] = buildProductMap;
-      settings.desiredCapabilities['browserstack.testhubBuildUuid'] = process.env.BROWSERSTACK_TESTHUB_UUID ? process.env.BROWSERSTACK_TESTHUB_UUID : '' ;
+      settings.desiredCapabilities['browserstack.testhubBuildUuid'] = testhubBuildUuid;
     }
   } catch (error) {
     Logger.debug(`Error while sending productmap and build capabilities ${error}`);

src/testObservability.js

@@ -28,21 +28,25 @@ class TestObservability {
     }
 
     // Check for top-level testObservability or testReporting flags
-    if (settings.testObservability === true || settings.testReporting === true) {
-      process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'true';
-      process.env.BROWSERSTACK_TEST_REPORTING = 'true';
-    } else if (settings.testObservability === false || settings.testReporting === false) {
-      process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
-      process.env.BROWSERSTACK_TEST_REPORTING = 'false';
+    const topLevelFlag = helper.parseBooleanSetting(
+      helper.isUndefined(settings.testObservability) ? settings.testReporting : settings.testObservability
+    );
+    if (!helper.isUndefined(topLevelFlag)) {
+      process.env.BROWSERSTACK_TEST_OBSERVABILITY = String(topLevelFlag);
+      process.env.BROWSERSTACK_TEST_REPORTING = String(topLevelFlag);
     }
 
     // Check for test_observability or test_reporting configuration
     const observabilityConfig = this._settings.test_observability || this._settings.test_reporting;
     const testReportingOptions = this._settings.testReportingOptions || this._settings.testObservabilityOptions;
 
     if (!helper.isUndefined(observabilityConfig) && !helper.isUndefined(observabilityConfig.enabled)) {
-      process.env.BROWSERSTACK_TEST_OBSERVABILITY = observabilityConfig.enabled;
-      process.env.BROWSERSTACK_TEST_REPORTING = observabilityConfig.enabled;
+      const enabled = helper.parseBooleanSetting(observabilityConfig.enabled);
+      if (typeof observabilityConfig.enabled !== 'boolean') {
+        Logger.warn(`Interpreting test_observability.enabled=${JSON.stringify(observabilityConfig.enabled)} as ${enabled}. Set a boolean to remove the ambiguity.`);
+      }
+      process.env.BROWSERSTACK_TEST_OBSERVABILITY = String(enabled);
+      process.env.BROWSERSTACK_TEST_REPORTING = String(enabled);
     }
 
     if (process.argv.includes('--disable-test-observability') || process.argv.includes('--disable-test-reporting')) {
@@ -87,6 +91,10 @@ class TestObservability {
   }
 
   async launchTestSession() {
+    // Records that a build was intended, so a run that ends without a uuid can say so
+    // even though the failure path turns the product flags back off.
+    process.env.BROWSERSTACK_TESTHUB_BUILD_ATTEMPTED = 'true';
+
     // Support both old and new configuration options at different levels
     const testReportingOptions = this._settings.test_observability ||
                    this._settings.test_reporting ||
@@ -100,7 +108,8 @@ class TestObservability {
     const accessibilityOptions = accessibility ? this._settings.accessibilityOptions || {} : {};
     this._gitMetadata = await helper.getGitMetaData();
     const fromProduct = {
-      test_observability: this._settings.test_observability?.enabled || this._settings.test_reporting?.enabled || false,
+      test_observability: helper.parseBooleanSetting(
+        this._settings.test_observability?.enabled ?? this._settings.test_reporting?.enabled) ?? false,
       accessibility: accessibility
     };
     const data = {

src/utils/helper.js

@@ -106,6 +106,36 @@ exports.isSuppressedFailure = (cmd) => {
   return !!(opts && typeof opts === 'object' && opts.suppressNotFoundErrors === true);
 };
 
+const BOOLEAN_SETTING_TRUE = ['true', '1', 'yes', 'on'];
+const BOOLEAN_SETTING_FALSE = ['false', '0', 'no', 'off', ''];
+
+// Config reaches the plugin from nightwatch.conf.js, YAML and env-derived wrappers, so a
+// flag arrives as a boolean, a string or a number. Every site must resolve it identically:
+// a strict `=== true` at one site and truthiness at another is what let a run report
+// reporting as enabled while never creating a build. undefined = setting absent.
+exports.parseBooleanSetting = (value) => {
+  if (value === undefined || value === null) {
+    return undefined;
+  }
+  if (typeof value === 'boolean') {
+    return value;
+  }
+  if (typeof value === 'number') {
+    return value !== 0;
+  }
+  if (typeof value === 'string') {
+    const normalised = value.trim().toLowerCase();
+    if (BOOLEAN_SETTING_TRUE.includes(normalised)) {
+      return true;
+    }
+    if (BOOLEAN_SETTING_FALSE.includes(normalised)) {
+      return false;
+    }
+  }
+
+  return Boolean(value);
+};
+
 exports.isTestObservabilitySession = () => {
   return process.env.BROWSERSTACK_TEST_OBSERVABILITY === 'true' || 
          process.env.BROWSERSTACK_TEST_REPORTING === 'true';
@@ -211,11 +241,14 @@ exports.isAccessibilitySession = () => {
 
 exports.isTestHubBuild = (pluginSettings = {}, isBuildStart = false) => {
   if (isBuildStart) {
-    return pluginSettings?.test_reporting?.enabled === true ||  pluginSettings?.test_observability?.enabled === true || pluginSettings?.accessibility === true;
+    // Resolved from the same normalised state the rest of the plugin reads, so build
+    // creation cannot be skipped for a run that reports the product as enabled.
+    return this.isTestObservabilitySession() ||
+      this.parseBooleanSetting(pluginSettings?.accessibility) === true;
   }
-  
+
   return this.isTestObservabilitySession() || this.isAccessibilitySession();
-  
+
 };
 
 exports.isAppAccessibilitySession = () => {

test/src/utils/booleanSettings.js

@@ -0,0 +1,135 @@
+const {expect} = require('chai');
+
+const helper = require('../../../src/utils/helper');
+
+describe('parseBooleanSetting', () => {
+  it('returns undefined when the setting is absent', () => {
+    expect(helper.parseBooleanSetting(undefined)).to.eq(undefined);
+    expect(helper.parseBooleanSetting(null)).to.eq(undefined);
+  });
+
+  it('passes booleans through', () => {
+    expect(helper.parseBooleanSetting(true)).to.eq(true);
+    expect(helper.parseBooleanSetting(false)).to.eq(false);
+  });
+
+  it('accepts the string forms a YAML or env-derived config produces', () => {
+    ['true', 'True', ' TRUE ', '1', 'yes', 'on'].forEach((value) => {
+      expect(helper.parseBooleanSetting(value), value).to.eq(true);
+    });
+    ['false', 'False', ' FALSE ', '0', 'no', 'off', ''].forEach((value) => {
+      expect(helper.parseBooleanSetting(value), value).to.eq(false);
+    });
+  });
+
+  it('accepts numeric forms', () => {
+    expect(helper.parseBooleanSetting(1)).to.eq(true);
+    expect(helper.parseBooleanSetting(0)).to.eq(false);
+  });
+});
+
+describe('isTestHubBuild at build start', () => {
+  const envKeys = ['BROWSERSTACK_TEST_OBSERVABILITY', 'BROWSERSTACK_TEST_REPORTING', 'BROWSERSTACK_ACCESSIBILITY'];
+  let saved;
+
+  beforeEach(() => {
+    saved = {};
+    envKeys.forEach((key) => {
+      saved[key] = process.env[key];
+      delete process.env[key];
+    });
+  });
+
+  afterEach(() => {
+    envKeys.forEach((key) => {
+      if (saved[key] === undefined) {
+        delete process.env[key];
+      } else {
+        process.env[key] = saved[key];
+      }
+    });
+  });
+
+  it('starts a build whenever reporting resolved to enabled', () => {
+    process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'true';
+    expect(helper.isTestHubBuild({}, true)).to.eq(true);
+  });
+
+  it('does not start a build when reporting resolved to disabled', () => {
+    process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
+    process.env.BROWSERSTACK_TEST_REPORTING = 'false';
+    expect(helper.isTestHubBuild({test_observability: {enabled: true}}, true)).to.eq(false);
+  });
+
+  it('starts a build for a truthy accessibility setting', () => {
+    process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
+    process.env.BROWSERSTACK_TEST_REPORTING = 'false';
+    expect(helper.isTestHubBuild({accessibility: 'true'}, true)).to.eq(true);
+  });
+});
+
+describe('TestObservability.configure normalises enabled', () => {
+  const envKeys = ['BROWSERSTACK_TEST_OBSERVABILITY', 'BROWSERSTACK_TEST_REPORTING'];
+  let saved;
+  let TestObservability;
+
+  before(() => {
+    TestObservability = require('../../../src/testObservability');
+  });
+
+  beforeEach(() => {
+    saved = {};
+    envKeys.forEach((key) => {
+      saved[key] = process.env[key];
+      delete process.env[key];
+    });
+  });
+
+  afterEach(() => {
+    envKeys.forEach((key) => {
+      if (saved[key] === undefined) {
+        delete process.env[key];
+      } else {
+        process.env[key] = saved[key];
+      }
+    });
+  });
+
+  // configure() disables reporting outright when credentials are absent, so every
+  // case below has to carry them for the `enabled` resolution to be observable.
+  const withCredentials = (observability = {}) => Object.assign({user: 'USER', key: 'KEY'}, observability);
+
+  const configure = (pluginSettings) => {
+    new TestObservability().configure({'@nightwatch/browserstack': pluginSettings});
+  };
+
+  it('writes true for a string enabled', () => {
+    configure({test_observability: withCredentials({enabled: 'true'})});
+    expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
+    expect(process.env.BROWSERSTACK_TEST_REPORTING).to.eq('true');
+  });
+
+  it('writes true for a numeric enabled', () => {
+    configure({test_observability: withCredentials({enabled: 1})});
+    expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
+  });
+
+  it('writes false for a string false', () => {
+    configure({test_observability: withCredentials({enabled: 'false'})});
+    expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('false');
+    expect(process.env.BROWSERSTACK_TEST_REPORTING).to.eq('false');
+  });
+
+  it('leaves the default on when enabled is absent', () => {
+    configure({test_observability: withCredentials()});
+    expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
+  });
+
+  it('honours a top-level string flag', () => {
+    new TestObservability().configure({
+      testObservability: 'false',
+      '@nightwatch/browserstack': {test_observability: withCredentials()}
+    });
+    expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('false');
+  });
+});