diff --git a/nightwatch/globals.js b/nightwatch/globals.js index 7eb160f..d7f1417 100644 --- a/nightwatch/globals.js +++ b/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}`); diff --git a/src/testObservability.js b/src/testObservability.js index 582d37e..d01a2e3 100644 --- a/src/testObservability.js +++ b/src/testObservability.js @@ -28,12 +28,12 @@ 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 @@ -41,8 +41,12 @@ class TestObservability { 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 = { diff --git a/src/utils/helper.js b/src/utils/helper.js index d6d17b4..0b908d9 100644 --- a/src/utils/helper.js +++ b/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 = () => { diff --git a/test/src/utils/booleanSettings.js b/test/src/utils/booleanSettings.js new file mode 100644 index 0000000..2143438 --- /dev/null +++ b/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'); + }); +});