Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pr-238.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@wdio/browserstack-service": patch
---

- Added a warning when `BROWSERSTACK_USERNAME`/`BROWSERSTACK_ACCESS_KEY` or `testObservabilityOptions.user` point to a different BrowserStack account than the WebdriverIO `user`/`key`. Such runs send test results to a different account than their sessions.
9 changes: 9 additions & 0 deletions packages/browserstack-service/src/launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import {
isTrue,
getBrowserStackUser,
getBrowserStackKey,
getCredentialMismatchWarning,
isBrowserstackInfra,
uploadLogs,
ObjectsAreEqual, getBasicAuthHeader,
isValidCapsForHealing,
Expand Down Expand Up @@ -274,6 +276,13 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
// edge-1 conflict, but browserStackConfig.app was copied earlier in the constructor.
this.browserStackConfig.app = this._options.app

if (isBrowserstackInfra(config as BrowserstackConfig & Options.Testrunner, capabilities as Capabilities.BrowserStackCapabilities)) {
const credentialMismatchWarning = getCredentialMismatchWarning(this._options, config)
if (credentialMismatchWarning) {
BStackLogger.warn(credentialMismatchWarning)
}
}

// Send Funnel start request
await sendStart(this.browserStackConfig)

Expand Down
32 changes: 32 additions & 0 deletions packages/browserstack-service/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,38 @@ export function getBrowserStackKey(config: Options.Testrunner) {
return config.key
}

// Sessions authenticate with config.user, but the CLI / Test Reporting build prefers the
// env credentials, then testObservabilityOptions.user — a mismatch splits one run across two accounts.
Comment on lines +1453 to +1454

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if creds are present in the env vars then why are we using the creds from the config file? shouldn't we follow this order of picking up the properties: cli > env > yml

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the history. Device-session creds have always come from the wdio config: WebdriverIO core builds the /session Basic auth from config.user/config.key and never reads BROWSERSTACK_USERNAME. See webdriver 8.40.0 build/request/index.js:121 and 9.32.0 build/node.js:2230; there are no env refs in any core package. No @wdio/browserstack-service release from 6.12.1 to 9.36.2 copies the env creds into config.user/key.

The cli > env > yml order only ever applied to the service's own calls: Test Reporting (getObservabilityUser, and since 9.21.0 the CLI binary's setFinalCaps), Percy and the log upload. So the service and WebdriverIO have always resolved credentials differently. Making the session follow env too would mean overwriting config.user/key in onPrepare. That silently moves sessions to another account for anyone with a stray env var today, which is why this PR only warns.

export function getCredentialMismatchWarning(options: BrowserstackConfig & Options.Testrunner, config: Options.Testrunner): string | undefined {
const hubUser = config.user
if (typeof hubUser !== 'string' || hubUser.length === 0) {
return undefined
}

let source: string | undefined
let reportingUser: string | undefined
for (const envVar of ['BROWSERSTACK_USERNAME', 'BROWSERSTACK_USER_NAME']) {
if (process.env[envVar]) {
source = `the ${envVar} environment variable`
reportingUser = process.env[envVar]
break
}
}
if (!source && options.testObservabilityOptions?.user) {
source = 'testObservabilityOptions.user'
reportingUser = options.testObservabilityOptions.user
}

if (!source || reportingUser === hubUser) {
return undefined
}

return `BrowserStack credential mismatch: the \`user\` in your WebdriverIO config and ${source} point to different BrowserStack accounts. ` +
`Test sessions are created with the config \`user\`, but Test Reporting & Analytics builds are created with ${source}, ` +
'so this run\'s test results will not appear under the same account as its sessions. ' +
`Use the same BrowserStack credentials in both places (or remove ${source}) to see them together.`
}

export function isUndefined(value: unknown) {
let res = (value === undefined || value === null)
if (typeof value === 'string') {
Expand Down
27 changes: 27 additions & 0 deletions packages/browserstack-service/tests/launcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,33 @@ describe('onPrepare', () => {
vi.spyOn(thUtils, 'getProductMap').mockImplementation(() => productMap)
})

it('warns when BROWSERSTACK_USERNAME points to a different account than config.user', async () => {
const warnSpy = vi.spyOn(bstackLogger.BStackLogger, 'warn')
process.env.BROWSERSTACK_USERNAME = 'another-account'
try {
const service = new BrowserstackLauncher({ testObservability: false } as any, caps, config)
await service.onPrepare(config, caps)
} finally {
delete process.env.BROWSERSTACK_USERNAME
}

expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('BrowserStack credential mismatch'))
})

it('does not warn about a credential mismatch when not running on BrowserStack', async () => {
const warnSpy = vi.spyOn(bstackLogger.BStackLogger, 'warn')
process.env.BROWSERSTACK_USERNAME = 'another-account'
const nonBstackConfig = { ...config, hostname: 'localhost' }
try {
const service = new BrowserstackLauncher({ testObservability: false } as any, caps, nonBstackConfig)
await service.onPrepare(nonBstackConfig, caps)
} finally {
delete process.env.BROWSERSTACK_USERNAME
}

expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('BrowserStack credential mismatch'))
})

it('should not try to upload app is app is undefined', async () => {
const service = new BrowserstackLauncher({ testObservability: false } as any, caps, config)
await service.onPrepare(config, caps)
Expand Down
45 changes: 45 additions & 0 deletions packages/browserstack-service/tests/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
isTrue,
uploadLogs,
getObservabilityProduct,
getCredentialMismatchWarning,
isUndefined,
processTestObservabilityResponse,
processAccessibilityResponse,
Expand Down Expand Up @@ -1147,6 +1148,50 @@ describe('getObservabilityUser', () => {
})
})

describe('getCredentialMismatchWarning', () => {
const envVars = ['BROWSERSTACK_USERNAME', 'BROWSERSTACK_USER_NAME']
beforeEach(() => envVars.forEach((v) => delete process.env[v]))
afterEach(() => envVars.forEach((v) => delete process.env[v]))

it('warns when BROWSERSTACK_USERNAME differs from config.user', () => {
process.env.BROWSERSTACK_USERNAME = 'other-account'
const warning = getCredentialMismatchWarning({} as any, { user: 'hub-account' })
expect(warning).toContain('BROWSERSTACK_USERNAME environment variable')
expect(warning).not.toContain('hub-account')
expect(warning).not.toContain('other-account')
})

it('warns when BROWSERSTACK_USER_NAME differs from config.user', () => {
process.env.BROWSERSTACK_USER_NAME = 'other-account'
expect(getCredentialMismatchWarning({} as any, { user: 'hub-account' })).toContain('BROWSERSTACK_USER_NAME environment variable')
})

it('warns when testObservabilityOptions.user differs from config.user', () => {
const warning = getCredentialMismatchWarning({ testObservabilityOptions: { user: 'other-account' } } as any, { user: 'hub-account' })
expect(warning).toContain('testObservabilityOptions.user')
})

it('names the env var when it takes precedence over testObservabilityOptions.user', () => {
process.env.BROWSERSTACK_USERNAME = 'other-account'
const warning = getCredentialMismatchWarning({ testObservabilityOptions: { user: 'hub-account' } } as any, { user: 'hub-account' })
expect(warning).toContain('BROWSERSTACK_USERNAME environment variable')
})

it('does not warn when the env var matches config.user', () => {
process.env.BROWSERSTACK_USERNAME = 'hub-account'
expect(getCredentialMismatchWarning({ testObservabilityOptions: { user: 'other-account' } } as any, { user: 'hub-account' })).toBeUndefined()
})

it('does not warn when no alternative credential source is set', () => {
expect(getCredentialMismatchWarning({} as any, { user: 'hub-account' })).toBeUndefined()
})

it('does not warn when config.user is not set', () => {
process.env.BROWSERSTACK_USERNAME = 'other-account'
expect(getCredentialMismatchWarning({} as any, {})).toBeUndefined()
})
})

describe('getObservabilityKey', () => {
it('get env var', () => {
process.env.BROWSERSTACK_ACCESS_KEY = 'try'
Expand Down
Loading