Remove duplicate Playwright helper initialization (drop constructor setTimeout)#5206
Conversation
…engines in constructor
|
All users running CodeceptJS 3.7.5 with the Playwright helper may see noisy log warnings like: This comes from the helper invoking _init() twice (constructor + container lifecycle). It is harmless (registration is idempotent in practice) but confusing and risks future race conditions if more logic is added. This PR removes the redundant deferred init so only one registration path remains. A quick release would prevent confusion in newly upgraded projects. |
There was a problem hiding this comment.
Pull Request Overview
Removes a redundant asynchronous initialization call from the Playwright helper constructor that was causing duplicate execution of the _init() method. The initialization is already properly handled by the CodeceptJS container lifecycle, making the deferred constructor call unnecessary and potentially problematic.
- Eliminates duplicate
_init()method executions that occurred during framework bootstrap - Removes race condition potential between container-managed and constructor-initiated initialization
- Simplifies initialization flow to rely solely on the standard CodeceptJS container lifecycle
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
The fails seems flaky and unrelated to my PR 🤔 |
Playwright Helper: Remove duplicate async initialization causing race conditions
Summary
This PR removes an unnecessary
setTimeout(... _init())call from thePlaywrighthelper constructor that caused_init()to run twice in parallel during framework bootstrap. Initialization is already performed by the CodeceptJS container (container.createHelpers -> helper._init()), so the deferred constructor call was redundant and risky.Background
After merging PR #5089 (custom locator strategies + selector engine registration improvements), the constructor still contained (or reintroduced) an asynchronous fire‑and‑forget initialization pattern:
At runtime this led to two near-simultaneous executions:
global.createHelpers -> Playwright._initsetTimeoutResult: duplicate selector engine registration attempts and potential race conditions (especially if future logic adds non-idempotent side effects).
Problem Details
Stack traces showed two
_initexecutions for a singlecodecept init/ test catalog discovery phase. Although selector engine registration was wrapped intry/catch, this produced noisy logs and increased risk of subtle state bugs (e.g. shared sets likeregisteredCustomLocatorStrategies).Changes
setTimeoutcall from thePlaywrighthelper constructor._init()lifecycle._startBrowser()already awaits_init()explicitly.)Rationale
What This Does NOT Change
Reproduction Before Fix
_initstack traces and selector registration attempts.After Fix
Only one
_initcall—triggered by the container—appears in traces.Testing & Safety
_initinvocation during bootstrap.Potential Follow-Up (Optional)
If future contributions add asynchronous prep stages that must be serialized:
_initialized/_initPromiseguards (idempotence) inside_init().Migration
No action required for end users or plugin authors.
Risk Assessment
Low. The removed code path was redundant; the retained path is the canonical lifecycle hook.
Changelog Entry
Playwright Helper: Remove redundant deferred
_init()invocation to prevent duplicate initialization and potential race conditions.Applicable helpers:
Applicable plugins:
Type of change
Checklist:
npm run docs)npm run lint)npm test)