Skip to content

Commit dcc2f96

Browse files
kamal-kaur04claude
andauthored
fix: resolve TestHub build creation from the normalised product flags (#63)
Build start was gated on a strict `enabled === true` while every other site read the same setting for truthiness, so a config supplying `"true"`, `1`, or omitting the key entirely reported reporting as enabled, created no build, and stamped every session with an empty `testhubBuildUuid`. The gate arrived in v3.7.0 and is present through 3.11.3; the same config creates a build on 3.5.0. - parse `enabled` once, in one place, accepting the boolean/string/number forms a nightwatch.conf.js, YAML or env-derived config produces - derive the build-start decision from that resolved state, so it can no longer disagree with the product map - warn when a run intended a TestHub build but ends without a uuid, which was previously silent in both the never-attempted and the failed-attempt case Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent cfc768b commit dcc2f96

4 files changed

Lines changed: 204 additions & 14 deletions

File tree

‎nightwatch/globals.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,8 @@ const cucumberPatcher = () => {
776776
}
777777
};
778778

779+
let missingTestHubBuildWarned = false;
780+
779781
const addProductMapAndbuildUuidCapability = (settings) => {
780782
try {
781783
if (!settings?.desiredCapabilities) {
@@ -793,12 +795,23 @@ const addProductMapAndbuildUuidCapability = (settings) => {
793795
percy: false
794796
};
795797

798+
const testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID || '';
799+
800+
// An empty uuid on a run that intended a TestHub build leaves every session
801+
// unlinkable, and used to be entirely silent.
802+
const testHubBuildIntended = buildProductMap.observability || buildProductMap.accessibility ||
803+
process.env.BROWSERSTACK_TESTHUB_BUILD_ATTEMPTED === 'true';
804+
if (!testhubBuildUuid && testHubBuildIntended && !missingTestHubBuildWarned) {
805+
missingTestHubBuildWarned = true;
806+
Logger.warn('No TestHub build was created for this run, so its sessions cannot be linked to test reporting or accessibility data.');
807+
}
808+
796809
if (settings.desiredCapabilities['bstack:options']) {
797810
settings.desiredCapabilities['bstack:options']['buildProductMap'] = buildProductMap;
798-
settings.desiredCapabilities['bstack:options']['testhubBuildUuid'] = process.env.BROWSERSTACK_TESTHUB_UUID ? process.env.BROWSERSTACK_TESTHUB_UUID : '' ;
811+
settings.desiredCapabilities['bstack:options']['testhubBuildUuid'] = testhubBuildUuid;
799812
} else {
800813
settings.desiredCapabilities['browserstack.buildProductMap'] = buildProductMap;
801-
settings.desiredCapabilities['browserstack.testhubBuildUuid'] = process.env.BROWSERSTACK_TESTHUB_UUID ? process.env.BROWSERSTACK_TESTHUB_UUID : '' ;
814+
settings.desiredCapabilities['browserstack.testhubBuildUuid'] = testhubBuildUuid;
802815
}
803816
} catch (error) {
804817
Logger.debug(`Error while sending productmap and build capabilities ${error}`);

‎src/testObservability.js‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,25 @@ class TestObservability {
2828
}
2929

3030
// Check for top-level testObservability or testReporting flags
31-
if (settings.testObservability === true || settings.testReporting === true) {
32-
process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'true';
33-
process.env.BROWSERSTACK_TEST_REPORTING = 'true';
34-
} else if (settings.testObservability === false || settings.testReporting === false) {
35-
process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
36-
process.env.BROWSERSTACK_TEST_REPORTING = 'false';
31+
const topLevelFlag = helper.parseBooleanSetting(
32+
helper.isUndefined(settings.testObservability) ? settings.testReporting : settings.testObservability
33+
);
34+
if (!helper.isUndefined(topLevelFlag)) {
35+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = String(topLevelFlag);
36+
process.env.BROWSERSTACK_TEST_REPORTING = String(topLevelFlag);
3737
}
3838

3939
// Check for test_observability or test_reporting configuration
4040
const observabilityConfig = this._settings.test_observability || this._settings.test_reporting;
4141
const testReportingOptions = this._settings.testReportingOptions || this._settings.testObservabilityOptions;
4242

4343
if (!helper.isUndefined(observabilityConfig) && !helper.isUndefined(observabilityConfig.enabled)) {
44-
process.env.BROWSERSTACK_TEST_OBSERVABILITY = observabilityConfig.enabled;
45-
process.env.BROWSERSTACK_TEST_REPORTING = observabilityConfig.enabled;
44+
const enabled = helper.parseBooleanSetting(observabilityConfig.enabled);
45+
if (typeof observabilityConfig.enabled !== 'boolean') {
46+
Logger.warn(`Interpreting test_observability.enabled=${JSON.stringify(observabilityConfig.enabled)} as ${enabled}. Set a boolean to remove the ambiguity.`);
47+
}
48+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = String(enabled);
49+
process.env.BROWSERSTACK_TEST_REPORTING = String(enabled);
4650
}
4751

4852
if (process.argv.includes('--disable-test-observability') || process.argv.includes('--disable-test-reporting')) {
@@ -87,6 +91,10 @@ class TestObservability {
8791
}
8892

8993
async launchTestSession() {
94+
// Records that a build was intended, so a run that ends without a uuid can say so
95+
// even though the failure path turns the product flags back off.
96+
process.env.BROWSERSTACK_TESTHUB_BUILD_ATTEMPTED = 'true';
97+
9098
// Support both old and new configuration options at different levels
9199
const testReportingOptions = this._settings.test_observability ||
92100
this._settings.test_reporting ||
@@ -100,7 +108,8 @@ class TestObservability {
100108
const accessibilityOptions = accessibility ? this._settings.accessibilityOptions || {} : {};
101109
this._gitMetadata = await helper.getGitMetaData();
102110
const fromProduct = {
103-
test_observability: this._settings.test_observability?.enabled || this._settings.test_reporting?.enabled || false,
111+
test_observability: helper.parseBooleanSetting(
112+
this._settings.test_observability?.enabled ?? this._settings.test_reporting?.enabled) ?? false,
104113
accessibility: accessibility
105114
};
106115
const data = {

‎src/utils/helper.js‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,36 @@ exports.isSuppressedFailure = (cmd) => {
106106
return !!(opts && typeof opts === 'object' && opts.suppressNotFoundErrors === true);
107107
};
108108

109+
const BOOLEAN_SETTING_TRUE = ['true', '1', 'yes', 'on'];
110+
const BOOLEAN_SETTING_FALSE = ['false', '0', 'no', 'off', ''];
111+
112+
// Config reaches the plugin from nightwatch.conf.js, YAML and env-derived wrappers, so a
113+
// flag arrives as a boolean, a string or a number. Every site must resolve it identically:
114+
// a strict `=== true` at one site and truthiness at another is what let a run report
115+
// reporting as enabled while never creating a build. undefined = setting absent.
116+
exports.parseBooleanSetting = (value) => {
117+
if (value === undefined || value === null) {
118+
return undefined;
119+
}
120+
if (typeof value === 'boolean') {
121+
return value;
122+
}
123+
if (typeof value === 'number') {
124+
return value !== 0;
125+
}
126+
if (typeof value === 'string') {
127+
const normalised = value.trim().toLowerCase();
128+
if (BOOLEAN_SETTING_TRUE.includes(normalised)) {
129+
return true;
130+
}
131+
if (BOOLEAN_SETTING_FALSE.includes(normalised)) {
132+
return false;
133+
}
134+
}
135+
136+
return Boolean(value);
137+
};
138+
109139
exports.isTestObservabilitySession = () => {
110140
return process.env.BROWSERSTACK_TEST_OBSERVABILITY === 'true' ||
111141
process.env.BROWSERSTACK_TEST_REPORTING === 'true';
@@ -211,11 +241,14 @@ exports.isAccessibilitySession = () => {
211241

212242
exports.isTestHubBuild = (pluginSettings = {}, isBuildStart = false) => {
213243
if (isBuildStart) {
214-
return pluginSettings?.test_reporting?.enabled === true || pluginSettings?.test_observability?.enabled === true || pluginSettings?.accessibility === true;
244+
// Resolved from the same normalised state the rest of the plugin reads, so build
245+
// creation cannot be skipped for a run that reports the product as enabled.
246+
return this.isTestObservabilitySession() ||
247+
this.parseBooleanSetting(pluginSettings?.accessibility) === true;
215248
}
216-
249+
217250
return this.isTestObservabilitySession() || this.isAccessibilitySession();
218-
251+
219252
};
220253

221254
exports.isAppAccessibilitySession = () => {

‎test/src/utils/booleanSettings.js‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
const {expect} = require('chai');
2+
3+
const helper = require('../../../src/utils/helper');
4+
5+
describe('parseBooleanSetting', () => {
6+
it('returns undefined when the setting is absent', () => {
7+
expect(helper.parseBooleanSetting(undefined)).to.eq(undefined);
8+
expect(helper.parseBooleanSetting(null)).to.eq(undefined);
9+
});
10+
11+
it('passes booleans through', () => {
12+
expect(helper.parseBooleanSetting(true)).to.eq(true);
13+
expect(helper.parseBooleanSetting(false)).to.eq(false);
14+
});
15+
16+
it('accepts the string forms a YAML or env-derived config produces', () => {
17+
['true', 'True', ' TRUE ', '1', 'yes', 'on'].forEach((value) => {
18+
expect(helper.parseBooleanSetting(value), value).to.eq(true);
19+
});
20+
['false', 'False', ' FALSE ', '0', 'no', 'off', ''].forEach((value) => {
21+
expect(helper.parseBooleanSetting(value), value).to.eq(false);
22+
});
23+
});
24+
25+
it('accepts numeric forms', () => {
26+
expect(helper.parseBooleanSetting(1)).to.eq(true);
27+
expect(helper.parseBooleanSetting(0)).to.eq(false);
28+
});
29+
});
30+
31+
describe('isTestHubBuild at build start', () => {
32+
const envKeys = ['BROWSERSTACK_TEST_OBSERVABILITY', 'BROWSERSTACK_TEST_REPORTING', 'BROWSERSTACK_ACCESSIBILITY'];
33+
let saved;
34+
35+
beforeEach(() => {
36+
saved = {};
37+
envKeys.forEach((key) => {
38+
saved[key] = process.env[key];
39+
delete process.env[key];
40+
});
41+
});
42+
43+
afterEach(() => {
44+
envKeys.forEach((key) => {
45+
if (saved[key] === undefined) {
46+
delete process.env[key];
47+
} else {
48+
process.env[key] = saved[key];
49+
}
50+
});
51+
});
52+
53+
it('starts a build whenever reporting resolved to enabled', () => {
54+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'true';
55+
expect(helper.isTestHubBuild({}, true)).to.eq(true);
56+
});
57+
58+
it('does not start a build when reporting resolved to disabled', () => {
59+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
60+
process.env.BROWSERSTACK_TEST_REPORTING = 'false';
61+
expect(helper.isTestHubBuild({test_observability: {enabled: true}}, true)).to.eq(false);
62+
});
63+
64+
it('starts a build for a truthy accessibility setting', () => {
65+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = 'false';
66+
process.env.BROWSERSTACK_TEST_REPORTING = 'false';
67+
expect(helper.isTestHubBuild({accessibility: 'true'}, true)).to.eq(true);
68+
});
69+
});
70+
71+
describe('TestObservability.configure normalises enabled', () => {
72+
const envKeys = ['BROWSERSTACK_TEST_OBSERVABILITY', 'BROWSERSTACK_TEST_REPORTING'];
73+
let saved;
74+
let TestObservability;
75+
76+
before(() => {
77+
TestObservability = require('../../../src/testObservability');
78+
});
79+
80+
beforeEach(() => {
81+
saved = {};
82+
envKeys.forEach((key) => {
83+
saved[key] = process.env[key];
84+
delete process.env[key];
85+
});
86+
});
87+
88+
afterEach(() => {
89+
envKeys.forEach((key) => {
90+
if (saved[key] === undefined) {
91+
delete process.env[key];
92+
} else {
93+
process.env[key] = saved[key];
94+
}
95+
});
96+
});
97+
98+
// configure() disables reporting outright when credentials are absent, so every
99+
// case below has to carry them for the `enabled` resolution to be observable.
100+
const withCredentials = (observability = {}) => Object.assign({user: 'USER', key: 'KEY'}, observability);
101+
102+
const configure = (pluginSettings) => {
103+
new TestObservability().configure({'@nightwatch/browserstack': pluginSettings});
104+
};
105+
106+
it('writes true for a string enabled', () => {
107+
configure({test_observability: withCredentials({enabled: 'true'})});
108+
expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
109+
expect(process.env.BROWSERSTACK_TEST_REPORTING).to.eq('true');
110+
});
111+
112+
it('writes true for a numeric enabled', () => {
113+
configure({test_observability: withCredentials({enabled: 1})});
114+
expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
115+
});
116+
117+
it('writes false for a string false', () => {
118+
configure({test_observability: withCredentials({enabled: 'false'})});
119+
expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('false');
120+
expect(process.env.BROWSERSTACK_TEST_REPORTING).to.eq('false');
121+
});
122+
123+
it('leaves the default on when enabled is absent', () => {
124+
configure({test_observability: withCredentials()});
125+
expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('true');
126+
});
127+
128+
it('honours a top-level string flag', () => {
129+
new TestObservability().configure({
130+
testObservability: 'false',
131+
'@nightwatch/browserstack': {test_observability: withCredentials()}
132+
});
133+
expect(process.env.BROWSERSTACK_TEST_OBSERVABILITY).to.eq('false');
134+
});
135+
});

0 commit comments

Comments
 (0)