Skip to content
Open
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
192 changes: 192 additions & 0 deletions src/JavascriptTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php

declare(strict_types=1);

namespace DrevOps\BehatSteps;

use Behat\Behat\Hook\Scope\AfterScenarioScope;
use Behat\Behat\Hook\Scope\AfterStepScope;
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
use Behat\Behat\Hook\Scope\BeforeStepScope;
use Behat\Mink\Exception\DriverException;

trait JavascriptTrait
{
private array $jsErrorRegistry = [];

private array $monitoredStepPatterns = [
'/I visit /i',
'/I go to /i',
'/I open /i',
'/I click /i',
'/I press /i',
];

public function addJsMonitorStepPattern(string $regex): void
{
$this->monitoredStepPatterns[] = $regex;
}

/** @BeforeScenario @javascript */
public function jsCollectorBeforeScenario(BeforeScenarioScope $scope): void
{
$this->jsErrorRegistry = [];
}

/** @BeforeStep @javascript */
public function jsCollectorBeforeStep(BeforeStepScope $scope): void
{
if (!$this->stepNeedsMonitoring($scope->getStep()->getText())) {
return;
}
if (!$this->driverIsJsCapable()) {
return;
}

$this->injectJsCollector();
$this->safeExecute('window.__behatJsErrors = [];');
}

/** @AfterStep @javascript */
public function jsCollectorAfterStep(AfterStepScope $scope): void
{
if (!$this->stepNeedsMonitoring($scope->getStep()->getText())) {
return;
}
if (!$this->driverIsJsCapable()) {
return;
}

$this->injectJsCollector();
$this->harvestErrorsIntoRegistry();
}

/** @AfterScenario @no-js-errors @javascript */
public function jsCollectorAssertScenarioClean(AfterScenarioScope $scope): void
{
$this->harvestErrorsIntoRegistry();
if ($this->flattenRegistry() !== []) {
throw new \RuntimeException(
"JavaScript errors detected during scenario:\n" .
json_encode($this->jsErrorRegistry, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)
);
}
}

/** @Then /^no JavaScript errors should have occurred on this page$/ */
public function assertNoJsErrorsOnCurrentPage(): void
{
if (!$this->driverIsJsCapable()) {
return;
}

$this->harvestErrorsIntoRegistry();
$path = $this->currentPath();
$errors = $this->jsErrorRegistry[$path] ?? [];

Comment on lines +79 to +86
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inject the collector before harvesting to avoid undefined global

harvestErrorsIntoRegistry() assumes that window.__behatJsErrors has already been declared.
If assertNoJsErrorsOnCurrentPage() is called on a page that has never gone through a monitored step, this variable will be undefined, triggering a JavaScript reference error in some drivers and masking real problems.

   public function assertNoJsErrorsOnCurrentPage(): void
   {
     if (!$this->driverIsJsCapable()) {
       return;
     }

+    // Ensure the collector exists even if no monitored step ran on this page.
+    $this->injectJsCollector();
     $this->harvestErrorsIntoRegistry();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!$this->driverIsJsCapable()) {
return;
}
$this->harvestErrorsIntoRegistry();
$path = $this->currentPath();
$errors = $this->jsErrorRegistry[$path] ?? [];
public function assertNoJsErrorsOnCurrentPage(): void
{
if (!$this->driverIsJsCapable()) {
return;
}
// Ensure the collector exists even if no monitored step ran on this page.
$this->injectJsCollector();
$this->harvestErrorsIntoRegistry();
$path = $this->currentPath();
$errors = $this->jsErrorRegistry[$path] ?? [];
🤖 Prompt for AI Agents
In src/JavascriptTrait.php around lines 79 to 86, the method
harvestErrorsIntoRegistry() is called before ensuring the JavaScript error
collector is injected, which can cause a JavaScript reference error if
window.__behatJsErrors is undefined. To fix this, inject the collector script
before calling harvestErrorsIntoRegistry() so that window.__behatJsErrors is
always defined when accessed.

if ($errors !== []) {
throw new \RuntimeException(
"JavaScript errors on page $path:\n" .
json_encode($errors, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)
);
}
}

private function injectJsCollector(): void
{
$js = <<<'JS'
(function () {
if (window.__behatJsCollectorInstalled) { return; }
window.__behatJsCollectorInstalled = true;
window.__behatJsErrors = window.__behatJsErrors || [];

function push(msg) {
try { window.__behatJsErrors.push(String(msg)); } catch (e) {}
}

var origConsoleError = console.error;
console.error = function () {
push([].slice.call(arguments).join(' '));
return origConsoleError.apply(console, arguments);
};

var origOnError = window.onerror;
window.onerror = function (msg, src, line, col) {
push(msg + ' @ ' + src + ':' + line + ':' + col);
if (origOnError) { return origOnError.apply(this, arguments); }
};

window.addEventListener('error', function (ev) {
if (ev.target && (ev.target.src || ev.target.href)) {
push('Resource error: ' + (ev.target.src || ev.target.href));
}
}, true);

window.addEventListener('unhandledrejection', function (ev) {
push('Unhandled rejection: ' + (ev.reason && ev.reason.message
? ev.reason.message
: ev.reason));
});
}());
JS;

$this->safeExecute($js);
}

private function harvestErrorsIntoRegistry(): void
{
try {
$errors = $this->getSession()
->evaluateScript('return window.__behatJsErrors || [];');
} catch (DriverException $e) {
return;
}

if (!is_array($errors) || $errors === []) {
return;
}

$path = $this->currentPath();
$this->jsErrorRegistry[$path] = array_merge(
$this->jsErrorRegistry[$path] ?? [],
$errors
);

$this->safeExecute('window.__behatJsErrors = [];');
}

private function stepNeedsMonitoring(string $text): bool
{
foreach ($this->monitoredStepPatterns as $rx) {
if (preg_match($rx, $text)) {
return true;
}
}
return false;
}

private function flattenRegistry(): array
{
return array_merge(...array_values($this->jsErrorRegistry ?: [[]]));
}

private function currentPath(): string
{
$url = $this->getSession()->getCurrentUrl();
return parse_url($url, PHP_URL_PATH) ?: $url;
}

private function driverIsJsCapable(): bool
{
return method_exists($this, 'getSession')
&& $this->getSession()->getDriver()->supportsJavascript();
}

private function safeExecute(string $script): void
{
try {
$this->getSession()->executeScript($script);
} catch (DriverException $e) {
}
Comment on lines +185 to +190
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Swallowing driver exceptions hides valuable diagnostics

safeExecute() silently drops every DriverException. When troubleshooting flaky tests it is extremely helpful to know why a script could not be executed (e.g., cross-origin restriction, browser crashed, etc.).

Consider at least logging the message at NOTICE/DEBUG level:

   } catch (DriverException $e) {
-  }
+      // Do not break the test flow, but surface the problem for debugging.
+      error_log(sprintf('[Behat JS-collector] DriverException: %s', $e->getMessage()));
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function safeExecute(string $script): void
{
try {
$this->getSession()->executeScript($script);
} catch (DriverException $e) {
}
private function safeExecute(string $script): void
{
try {
$this->getSession()->executeScript($script);
} catch (DriverException $e) {
// Do not break the test flow, but surface the problem for debugging.
error_log(sprintf('[Behat JS-collector] DriverException: %s', $e->getMessage()));
}
}
🤖 Prompt for AI Agents
In src/JavascriptTrait.php around lines 185 to 190, the safeExecute method
currently catches DriverException but silently ignores it, which hides important
diagnostic information. Modify the catch block to log the exception message at
NOTICE or DEBUG level using the appropriate logger, so that failures in script
execution are recorded for troubleshooting without interrupting the flow.

}
}
2 changes: 2 additions & 0 deletions tests/behat/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use DrevOps\BehatSteps\ElementTrait;
use DrevOps\BehatSteps\FieldTrait;
use DrevOps\BehatSteps\FileDownloadTrait;
use DrevOps\BehatSteps\JavascriptTrait;
use DrevOps\BehatSteps\KeyboardTrait;
use DrevOps\BehatSteps\LinkTrait;
use DrevOps\BehatSteps\PathTrait;
Expand Down Expand Up @@ -70,6 +71,7 @@ class FeatureContext extends DrupalContext {
use UserTrait;
use WaitTrait;
use WatchdogTrait;
use JavascriptTrait;

use FeatureContextTrait;

Expand Down
16 changes: 16 additions & 0 deletions tests/behat/features/javascript.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Feature: Check that JavascriptTrait works
As a Behat Steps library developer
I want to provide tools to track and verify JavaScript errors
So that users can ensure their pages are error-free

@javascript @no-js-errors
Scenario: Assert no JavaScript errors when none exist
When I go to "/"


@javascript @no-js-errors
Scenario: Assert JavaScript error is detected when one exists
When I visit "/sites/default/files/relative.html"
And the element "#js-error-trigger" should be displayed
And I click on the element "#js-error-trigger"

Comment on lines +11 to +16
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

@no-js-errors tag makes the “error expected” scenario fail

The second scenario intentionally triggers a JavaScript error, but the @no-js-errors tag instructs JavascriptTrait to assert zero errors after the scenario, resulting in an unavoidable failure.

Remove the tag (or replace it with an explicit step that asserts the presence of errors):

-  @javascript @no-js-errors
+  @javascript
   Scenario: Assert JavaScript error is detected when one exists
🤖 Prompt for AI Agents
In tests/behat/features/javascript.feature around lines 11 to 16, the scenario
that intentionally triggers a JavaScript error is incorrectly tagged with
@no-js-errors, which causes the test to fail by asserting zero errors. Remove
the @no-js-errors tag from this scenario or replace it with a step that
explicitly asserts the presence of JavaScript errors to correctly reflect the
test's intent.

5 changes: 5 additions & 0 deletions tests/behat/fixtures/relative.html
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,11 @@
Show
off-canvas overlay
</button>

<button id="js-error-trigger" onclick="throw new Error('Test JS error');">
Trigger error
</button>

<div id="overlay-off-canvas"></div>

<div id="over-container">
Expand Down