diff --git a/.changeset/pr-309.md b/.changeset/pr-309.md new file mode 100644 index 00000000..9df4adf0 --- /dev/null +++ b/.changeset/pr-309.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed builds being reported as timed out on Test Reporting & Analytics when a Mocha `before` / `beforeEach` / `afterEach` hook is declared outside any `describe` block. diff --git a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts index b5706ee2..c9d93cc3 100644 --- a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts +++ b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts @@ -16,6 +16,7 @@ import { TEST_ANALYTICS_ID } from '../../constants.js' export default class WdioMochaTestFramework extends TestFramework { static KEY_HOOK_LAST_STARTED = 'test_hook_last_started' static KEY_HOOK_LAST_FINISHED = 'test_hook_last_finished' + private _suiteFile?: string /** * Constructor for the TestFramework @@ -27,6 +28,10 @@ export default class WdioMochaTestFramework extends TestFramework { super(testFrameworks, testFrameworkVersions, binSessionId) } + setSuiteFile(filename?: string) { + this._suiteFile = filename + } + /** * Find instance and track any state for the test framework * @param {TestFrameworkState} testFrameworkState @@ -190,7 +195,7 @@ export default class WdioMochaTestFramework extends TestFramework { const framework = TestFramework.getState(instance, TestFrameworkConstants.KEY_TEST_FRAMEWORK_NAME) const fullTitle = getUniqueIdentifier(test, framework) const gitConfig = await getGitMetaData() - const filename = test.file // || this._suiteFile + const filename = test.file || this._suiteFile const scopes = getMochaTestHierarchy(test) @@ -388,7 +393,8 @@ export default class WdioMochaTestFramework extends TestFramework { if (hookState === HookState.PRE) { const gitConfig = await getGitMetaData() - const filename = test.file + // root-level hooks belong to mocha's root suite, which has no file + const filename = test.file || this._suiteFile const hook: Record = { key, [TestFrameworkConstants.KEY_HOOK_ID]: TestFramework.getState(instance, TestFrameworkConstants.KEY_HOOK_ID) || '', diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index 1a93ffd1..e6c1d816 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -44,9 +44,11 @@ import { AutomationFrameworkConstants } from './cli/frameworks/constants/automat import AutomationFramework from './cli/frameworks/automationFramework.js' import type AutomationFrameworkInstance from './cli/instances/automationFrameworkInstance.js' import util from 'node:util' +import { fileURLToPath } from 'node:url' import APIUtils from './cli/apiUtils.js' import { CLIUtils } from './cli/cliUtils.js' import WdioCucumberTestFramework from './cli/frameworks/wdioCucumberTestFramework.js' +import WdioMochaTestFramework from './cli/frameworks/wdioMochaTestFramework.js' import type TestHubModule from './cli/modules/testHubModule.js' export default class BrowserstackService implements Services.ServiceInstance { @@ -195,6 +197,9 @@ export default class BrowserstackService implements Services.ServiceInstance { this._browser = browser ? browser : globalThis.browser PerformanceTester.browser = this._browser + // a root-level mocha `before` runs ahead of the first beforeSuite + this._setCliSuiteFile(specs?.[0]) + // Healing Support: if (!shouldAddServiceVersion(this._config, this._options.testObservability, caps as any)) { try { @@ -379,6 +384,7 @@ export default class BrowserstackService implements Services.ServiceInstance { this._suiteTitle = suite.title this._insightsHandler?.setSuiteFile(suite.file) this._accessibilityHandler?.setSuiteFile(suite.file) + this._setCliSuiteFile(suite.file) if (suite.title && suite.title !== 'Jasmine__TopLevel__Suite') { if (!BrowserstackCLI.getInstance().isRunning() || this._config.framework !== 'mocha'){ @@ -1081,6 +1087,21 @@ export default class BrowserstackService implements Services.ServiceInstance { }) } + /** The file the CLI mocha framework gives root-level hooks, which mocha leaves without one. */ + private _setCliSuiteFile(file?: string) { + const cli = BrowserstackCLI.getInstance() + const cliFramework = cli.isRunning() ? cli.getTestFramework() : null + if (!file || !(cliFramework instanceof WdioMochaTestFramework)) { + return + } + try { + // a runner spec is a file URL; MochaAdapter converts it the same way for suite.file + cliFramework.setSuiteFile(file.startsWith('file://') ? fileURLToPath(file) : file) + } catch (err) { + BStackLogger.debug(`Could not resolve the suite file from ${file}: ${err}`) + } + } + private async _setSessionName(suiteTitle: string | undefined, test?: Frameworks.Test) { if (!this._options.setSessionName || !suiteTitle) { return diff --git a/packages/browserstack-service/tests/cli/frameworks/wdioMochaTestFramework.test.ts b/packages/browserstack-service/tests/cli/frameworks/wdioMochaTestFramework.test.ts index 8276c50b..e9e812cf 100644 --- a/packages/browserstack-service/tests/cli/frameworks/wdioMochaTestFramework.test.ts +++ b/packages/browserstack-service/tests/cli/frameworks/wdioMochaTestFramework.test.ts @@ -163,6 +163,60 @@ describe('WdioMochaTestFramework', () => { }) }) + describe('root-level mocha hooks fall back to the suite file, as on the non-CLI flow', () => { + const SPEC = '/proj/test/specs/home.spec.ts' + const ROOT_HOOK = { title: '"before each" hook: wrappedHook for "t1"' } + + const startHook = async (test: Record) => { + const data = new Map([ + ['test_hooks_started', new Map()], + ['test_hooks_finished', new Map()], + ['hook_id', 'hook-1'], + ]) + const TestFramework = await import('../../../src/cli/frameworks/testFramework.js') + vi.mocked(TestFramework.default.getState).mockImplementation((_instance, key) => data.get(key)) + const util = await import('../../../src/util.js') + vi.mocked(util.getGitMetaData).mockResolvedValue({ root: '/proj' } as any) + const instance = { updateMultipleEntries: vi.fn() } + + await framework.trackHookEvents(instance as any, TestFrameworkState.BEFORE_EACH, HookState.PRE, { test }) + return (data.get('test_hooks_started') as Map[]>).get('BEFORE_EACH')![0] + } + + it('sends the current suite file for a hook mocha gives no file', async () => { + // a beforeEach/afterEach declared outside any describe hangs off mocha's root suite, which + // has no file; without a path the binary threw, dropped the hook events, and the test's + // reference to the never-sent hook timed the build out + framework.setSuiteFile(SPEC) + + const hook = await startHook(ROOT_HOOK) + + expect(hook.hook_id).toBe('hook-1') + expect(hook.test_file_path).toBe('test/specs/home.spec.ts') + expect(hook.test_location).toBeDefined() + }) + + it('keeps the file mocha gives a hook declared inside a describe', async () => { + framework.setSuiteFile('/proj/test/specs/other.spec.ts') + + const hook = await startHook({ ...ROOT_HOOK, file: SPEC }) + + expect(hook.test_file_path).toBe('test/specs/home.spec.ts') + }) + + it('applies the same fallback to test data', async () => { + const TestFramework = await import('../../../src/cli/frameworks/testFramework.js') + vi.mocked(TestFramework.default.getState).mockReturnValue('mocha') + const util = await import('../../../src/util.js') + vi.mocked(util.getGitMetaData).mockResolvedValue({ root: '/proj' } as any) + framework.setSuiteFile(SPEC) + + const result = await framework.getTestData({} as any, { title: 't1', body: '' } as any) + + expect(result).toHaveProperty('test_file_path', 'test/specs/home.spec.ts') + }) + }) + describe('loadTestResult', () => { it('should update instance with passed result', () => { const mockInstance = { diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index 34ba6c98..b84e43c2 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -1,4 +1,5 @@ import path from 'node:path' +import { pathToFileURL } from 'node:url' import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' import got from 'got' @@ -10,6 +11,7 @@ import InsightsHandler from '../src/insights-handler.js' import * as bstackLogger from '../src/bstackLogger.js' import { BrowserstackCLI } from '../src/cli/index.js' import WdioCucumberTestFramework from '../src/cli/frameworks/wdioCucumberTestFramework.js' +import WdioMochaTestFramework from '../src/cli/frameworks/wdioMochaTestFramework.js' import { TestFrameworkState } from '../src/cli/states/testFrameworkState.js' import { HookState } from '../src/cli/states/hookState.js' import TestFramework from '../src/cli/frameworks/testFramework.js' @@ -635,6 +637,43 @@ describe('beforeScenario', () => { }) describe('beforeSuite', () => { + describe('suite file for the CLI mocha framework', () => { + const SPEC = path.resolve('/specs/root hooks.spec.ts') + let framework: WdioMochaTestFramework + + beforeEach(() => { + framework = Object.create(WdioMochaTestFramework.prototype) as WdioMochaTestFramework + vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => framework, + getAutomationFramework: () => ({ trackEvent: vi.fn() }), + } as any) + }) + + afterEach(() => { + vi.mocked(BrowserstackCLI.getInstance).mockRestore() + }) + + it('hands the suite file to the CLI mocha framework, as the non-CLI flow does to its insights handler', async () => { + await service.beforeSuite({ title: 'foobar', file: '/specs/foobar.spec.ts' } as any) + + expect(framework['_suiteFile']).toBe('/specs/foobar.spec.ts') + }) + + it('seeds it from the worker spec in before(), as a root-level `before` runs ahead of the first beforeSuite', async () => { + await service.before(service['_config'] as any, [pathToFileURL(SPEC).href], browser) + + expect(framework['_suiteFile']).toBe(SPEC) + }) + + it('keeps the spec file when a suite comes without one', async () => { + await service.before(service['_config'] as any, [pathToFileURL(SPEC).href], browser) + await service.beforeSuite({ title: 'root-level tests' } as any) + + expect(framework['_suiteFile']).toBe(SPEC) + }) + }) + it('should send request to set the session name as suite name for Mocha tests', async () => { await service.before(service['_config'] as any, [], browser) expect(service['_suiteTitle']).toBeUndefined()