Skip to content

Commit 0ff2e80

Browse files
author
ScriptedAlchemy
committed
refactor(bdd): consolidate uiAgent slot machinery per simplify review
One UiAgentSlot class now serves both the worker-scoped and per-World agent lifetimes (shared in-flight promise, retry after failure with a compare-before-clear guard, teardown that awaits in-flight creation), so the two paths cannot drift. importPlatformPackage only translates genuine resolution failures of the platform package itself into the install hint — an installed-but-broken package rethrows untouched. Single unknown-type guard in config validation, exhaustiveness tripwire on UI_TARGET_TYPES, deprecated no-op fields (screenshotResizeScale, testId) dropped from the new public types, harmony gains the yaml env's appNameMapping, and the README scopes its yaml-parity claim honestly and documents worker-scope retry and cache-id semantics.
1 parent 66219cf commit 0ff2e80

8 files changed

Lines changed: 225 additions & 172 deletions

File tree

packages/bdd/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ interface BddConfig {
212212
}
213213
```
214214

215-
`uiAgent` is a flat union discriminated on `type` — field names match the corresponding [yaml automation](https://midscenejs.com/automate-with-scripts-in-yaml) env blocks, so configs translate 1:1:
215+
`uiAgent` is a flat union discriminated on `type` — field names match the corresponding [yaml automation](https://midscenejs.com/automate-with-scripts-in-yaml) env vocabulary (each row below lists exactly what its target supports; the web target covers the launcher basics, not yaml's serve/bridge modes):
216216

217217
| `type` | Fields | Notes |
218218
| --- | --- | --- |
@@ -226,7 +226,9 @@ interface BddConfig {
226226
Every target also accepts `scope?: 'scenario' | 'worker'`:
227227

228228
- `'scenario'` (default) — a fresh agent per scenario: full isolation, one Midscene report per scenario. Cheap for browsers.
229-
- `'worker'` — one agent per cucumber worker, reused across scenarios and destroyed when the worker finishes. Use this for device targets where reconnecting per scenario is expensive. Note the report semantics differ: scenarios share one rolling Midscene report (the same path is attached to each scenario) instead of one report per scenario. Factory configs are always scenario-scoped.
229+
- `'worker'` — one agent per cucumber worker, reused across scenarios and destroyed when the worker finishes. Use this for device targets where reconnecting per scenario is expensive. Note the report semantics differ: scenarios share one rolling Midscene report (the same path is attached to each scenario) instead of one report per scenario. If creation fails, the slot clears and the next scenario retries. Factory configs are always scenario-scoped.
230+
231+
`uiAgentOptions.cache: true` uses one shared cache id (`'default'`) for the whole suite — unlike the yaml runner, which derives a per-file cache id. Set `cache: { id: '...' }` if you want finer granularity.
230232

231233
The android/ios/harmony/computer platform packages are **optional peer dependencies** — install the one your target needs, e.g.:
232234

packages/bdd/src/agents/ui-agent.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* defaults to true.
1212
*/
1313
import path from 'node:path';
14+
import { getDebug } from '@midscene/shared/logger';
1415
import {
1516
type AndroidUiTarget,
1617
type ComputerUiTarget,
@@ -19,6 +20,7 @@ import {
1920
type IOSUiTarget,
2021
type InterfaceUiTarget,
2122
type ResolvedBddConfig,
23+
UI_TARGET_TYPES,
2224
type UiAgent,
2325
type UiAgentOptions,
2426
type WebUiTarget,
@@ -87,28 +89,42 @@ export async function createUiAgent(
8789
default: {
8890
const unreachable: never = uiAgent;
8991
throw new Error(
90-
`${ERROR_PREFIX} unhandled uiAgent.type ${JSON.stringify(unreachable)}`,
92+
`${ERROR_PREFIX} unhandled uiAgent.type '${String(
93+
(unreachable as { type?: unknown }).type,
94+
)}' — valid types: ${UI_TARGET_TYPES.join(', ')}`,
9195
);
9296
}
9397
}
9498
}
9599

96100
/**
97101
* Lazy platform import with a pointed error when the optional peer
98-
* dependency `@midscene/<targetType>` is not installed.
102+
* dependency `@midscene/<targetType>` is not installed. Only genuine
103+
* resolution failures OF THAT PACKAGE get the install hint — an installed
104+
* package that fails to load (broken transitive dep, native build failure)
105+
* rethrows untouched so the real error stays the headline.
99106
*/
100-
async function importPlatformPackage<T>(
107+
export async function importPlatformPackage<T>(
101108
targetType: string,
102109
importer: () => Promise<T>,
103110
): Promise<T> {
111+
const packageName = `@midscene/${targetType}`;
104112
try {
105113
return await importer();
106114
} catch (error) {
107-
const packageName = `@midscene/${targetType}`;
115+
const code = (error as NodeJS.ErrnoException).code;
116+
// The QUOTED name is what Node puts around the unresolvable specifier
117+
// ("Cannot find package '@midscene/x' …"); a transitive dep's failure
118+
// only mentions our package unquoted inside the "imported from" path.
119+
const notFound =
120+
(code === 'ERR_MODULE_NOT_FOUND' || code === 'MODULE_NOT_FOUND') &&
121+
error instanceof Error &&
122+
error.message.includes(`'${packageName}'`);
123+
if (!notFound) {
124+
throw error;
125+
}
108126
throw new Error(
109-
`${ERROR_PREFIX} uiAgent type '${targetType}' requires the optional peer dependency '${packageName}' — install it (e.g. \`pnpm add -D ${packageName}\`). Original error: ${
110-
error instanceof Error ? error.message : String(error)
111-
}`,
127+
`${ERROR_PREFIX} uiAgent type '${targetType}' requires the optional peer dependency '${packageName}' — install it (e.g. \`pnpm add -D ${packageName}\`).`,
112128
{ cause: error },
113129
);
114130
}
@@ -250,6 +266,9 @@ async function createInterfaceUiAgent(
250266
path.isAbsolute(target.module)
251267
? path.resolve(baseDir, target.module)
252268
: target.module;
269+
getDebug('bdd:ui-agent')(
270+
`interface module '${target.module}' resolved to '${specifier}'`,
271+
);
253272

254273
let importedModule: Record<string, unknown>;
255274
try {
@@ -274,6 +293,9 @@ async function createInterfaceUiAgent(
274293
);
275294
}
276295

296+
// Lazy not because core is optional (it's a hard dep) but to keep the
297+
// unit-test mock surface small and module load cheap for non-interface
298+
// targets.
277299
const { createAgent } = await import('@midscene/core/agent');
278300
const device = new (
279301
DeviceClass as new (

packages/bdd/src/config.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ function configError(message: string): Error {
2121

2222
function validateUiTarget(target: Record<string, unknown>): void {
2323
const type = target.type as UiTarget['type'];
24-
if (!UI_TARGET_TYPES.includes(type)) {
25-
throw configError(
26-
`uiAgent.type '${String(type)}' is unknown — valid types: ${UI_TARGET_TYPES.join(
27-
', ',
28-
)} (or pass a factory function)`,
29-
);
30-
}
3124

3225
if (
3326
target.scope !== undefined &&
@@ -57,9 +50,15 @@ function validateUiTarget(target: Record<string, unknown>): void {
5750
case 'computer':
5851
// Everything is optional: deviceId/launch/displayId default sensibly.
5952
break;
53+
// One guard for unknown types: the `never` binding keeps the switch
54+
// exhaustive at compile time, the throw handles unvalidated JS configs.
6055
default: {
6156
const unreachable: never = type;
62-
throw configError(`unhandled uiAgent.type ${String(unreachable)}`);
57+
throw configError(
58+
`uiAgent.type '${String(unreachable)}' is unknown — valid types: ${UI_TARGET_TYPES.join(
59+
', ',
60+
)} (or pass a factory function)`,
61+
);
6362
}
6463
}
6564
}

packages/bdd/src/types.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@ export interface WebUiTarget extends UiTargetCommon {
8686
/** Field vocabulary mirrors the yaml `android:` env (deviceId, launch, ...). */
8787
export interface AndroidUiTarget
8888
extends UiTargetCommon,
89-
Omit<AndroidDeviceOpt, 'customActions'> {
89+
// customActions is callback-typed (not config material); the resize
90+
// scale is deprecated-and-ignored in core — a new API has no reason to
91+
// accept either.
92+
Omit<AndroidDeviceOpt, 'customActions' | 'screenshotResizeScale'> {
9093
type: 'android';
9194
/** ADB device id; defaults to the first connected device. */
9295
deviceId?: string;
@@ -106,12 +109,14 @@ export interface IOSUiTarget
106109
/** Field vocabulary mirrors the yaml `harmony:` env (deviceId, launch, ...). */
107110
export interface HarmonyUiTarget
108111
extends UiTargetCommon,
109-
Omit<HarmonyDeviceOpt, 'customActions'> {
112+
Omit<HarmonyDeviceOpt, 'customActions' | 'screenshotResizeScale'> {
110113
type: 'harmony';
111114
/** HDC device id; defaults to the first connected device. */
112115
deviceId?: string;
113116
/** App package to launch after connecting (optional). */
114117
launch?: string;
118+
/** App-name → bundle-name mapping for launch, as in the yaml env. */
119+
appNameMapping?: Record<string, string>;
115120
}
116121

117122
/** Field vocabulary mirrors the yaml `computer:` env. */
@@ -144,6 +149,16 @@ export const UI_TARGET_TYPES = [
144149
'interface',
145150
] as const satisfies readonly UiTarget['type'][];
146151

152+
// Completeness tripwire: `satisfies` above rejects typos but not omissions —
153+
// this fails to compile until a new union member is added to the array.
154+
type MissingTargetTypes = Exclude<
155+
UiTarget['type'],
156+
(typeof UI_TARGET_TYPES)[number]
157+
>;
158+
const _allTargetTypesListed: MissingTargetTypes extends never ? true : never =
159+
true;
160+
void _allTargetTypesListed;
161+
147162
/**
148163
* Declarative UI target — one flat object per platform, discriminated on
149164
* `type`. android/ios/harmony/computer need their optional peer package
@@ -160,9 +175,10 @@ export type UiTarget =
160175
/**
161176
* Agent construction options shared by every target type — mirrors the yaml
162177
* `agent:` block (generateReport, reportFileName, groupName, cache, ...).
163-
* `generateReport` defaults to true.
178+
* `generateReport` defaults to true. (`testId` is a deprecated yaml alias a
179+
* new API has no reason to accept.)
164180
*/
165-
export type UiAgentOptions = MidsceneYamlScriptAgentOpt;
181+
export type UiAgentOptions = Omit<MidsceneYamlScriptAgentOpt, 'testId'>;
166182

167183
export type UiAgentFactory = () => Promise<{
168184
agent: UiAgent;

0 commit comments

Comments
 (0)