New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show JS errors in html report #3821

Merged
merged 5 commits into from Dec 26, 2016

Conversation

Projects
None yet
4 participants
@ngraf

ngraf commented Dec 7, 2016

As a tester
I want to see JavaScript errors in HTML reports of failing tests
to make troubleshooting easier.

Usage

Set js_error_logging to true for WebDriver module (opt-in).

log_js_errors_configuration

How it looks like

js_error_in_html

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
if ($logType === 'browser' && $this->config['log_js_errors']
&& ($test instanceof Cest || $test instanceof Cept)
) {

This comment has been minimized.

@Nitpick-CI

Nitpick-CI Dec 7, 2016

Expected 1 newline after opening brace; 2 found

@Nitpick-CI

Nitpick-CI Dec 7, 2016

Expected 1 newline after opening brace; 2 found

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
';
$errorFound = false;
foreach ($logEntries as $logEntry) {

This comment has been minimized.

@Nitpick-CI

Nitpick-CI Dec 7, 2016

Expected 1 newline after opening brace; 2 found

@Nitpick-CI

Nitpick-CI Dec 7, 2016

Expected 1 newline after opening brace; 2 found

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
$errorFound = false;
foreach ($logEntries as $logEntry) {
if ($this->isJSError($logEntry['level'],$logEntry['message'])) {

This comment has been minimized.

@Nitpick-CI

Nitpick-CI Dec 7, 2016

No space found after comma in function call

@Nitpick-CI

Nitpick-CI Dec 7, 2016

No space found after comma in function call

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Dec 7, 2016

Member

Thanks, looks great

Member

DavertMik commented Dec 7, 2016

Thanks, looks great

@DavertMik

Thanks for your work, however I have some remarks about implementation.

I think HTML should not be part of logging process, as this should not be restricted to HTML reporter only. I think if it would be printed to console as it is (inside a comment) it would be just enough. This step information can also be taken into an HTML report. I think this nice color:red font:serif formatting is not that important.

Could you update?

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
@@ -21,9 +21,12 @@
use Codeception\PHPUnit\Constraint\Page as PageConstraint;
use Codeception\PHPUnit\Constraint\WebDriver as WebDriverConstraint;
use Codeception\PHPUnit\Constraint\WebDriverNot as WebDriverConstraintNot;
use Codeception\Test\Cept;

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

not needed

@DavertMik

DavertMik Dec 10, 2016

Member

not needed

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
@@ -21,9 +21,12 @@
use Codeception\PHPUnit\Constraint\Page as PageConstraint;
use Codeception\PHPUnit\Constraint\WebDriver as WebDriverConstraint;
use Codeception\PHPUnit\Constraint\WebDriverNot as WebDriverConstraintNot;
use Codeception\Test\Cept;
use Codeception\Test\Cest;

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

not needed

@DavertMik

DavertMik Dec 10, 2016

Member

not needed

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
if (empty($logEntries)) {
$this->debugSection("Selenium {$logType} Logs", " EMPTY ");
continue;
}
$this->debugSection("Selenium {$logType} Logs", "\n" . $this->formatLogEntries($logEntries));
if ($logType === 'browser' && $this->config['log_js_errors']
&& ($test instanceof Cest || $test instanceof Cept)

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

$test instanceof ScenarioDriven

@DavertMik

DavertMik Dec 10, 2016

Member

$test instanceof ScenarioDriven

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
* @param array $logEntries
* @return string
*/
protected function formatLogEntriesToHtml(array $logEntries)

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

actually, why HTML? Can't this be also useful to have it inside console?
probably we shouldn't put view details and formatting inside the module?

@DavertMik

DavertMik Dec 10, 2016

Member

actually, why HTML? Can't this be also useful to have it inside console?
probably we shouldn't put view details and formatting inside the module?

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
return '';
}
$formattedLogs = '

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

probably that HTML part is too specific and should be removed

@DavertMik

DavertMik Dec 10, 2016

Member

probably that HTML part is too specific and should be removed

Show outdated Hide outdated src/Codeception/Module/WebDriver.php
$time = date('H:i:s', $logEntry['timestamp'] / 1000) .
// Append the milliseconds to the end of the time string
'.' . ($logEntry['timestamp'] % 1000);
$formattedLogs .= "{$time} {$logEntry['level']} - {$logEntry['message']}<br/>";

This comment has been minimized.

@DavertMik

DavertMik Dec 10, 2016

Member

why not just:

foreach ($errors as $error) {
   // time formatting happens here....
    $test->getScenario()->comment($error)
}
@DavertMik

DavertMik Dec 10, 2016

Member

why not just:

foreach ($errors as $error) {
   // time formatting happens here....
    $test->getScenario()->comment($error)
}
Show outdated Hide outdated src/Codeception/Module/WebDriver.php
protected function logJSErrors(ScenarioDriven $test, array $browserLogEntries)
{
foreach ($browserLogEntries as $logEntry) {
if ( true === isset($logEntry['level'])

This comment has been minimized.

@Nitpick-CI

Nitpick-CI Dec 12, 2016

Expected 0 spaces after opening bracket; 3 found

@Nitpick-CI

Nitpick-CI Dec 12, 2016

Expected 0 spaces after opening bracket; 3 found

@ngraf

This comment has been minimized.

Show comment
Hide comment
@ngraf

ngraf Dec 12, 2016

Thanks for your comments @DavertMik
I followed your advices and removed the HTML specific stuff. The feature is now more universal.

This is how it looks now in HTML:

js_errors_in_html_v2

This is how it looks now in Console:

js_errors_in_console_v2

ngraf commented Dec 12, 2016

Thanks for your comments @DavertMik
I followed your advices and removed the HTML specific stuff. The feature is now more universal.

This is how it looks now in HTML:

js_errors_in_html_v2

This is how it looks now in Console:

js_errors_in_console_v2

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Dec 26, 2016

Member

Thanks for your PR!

Member

DavertMik commented Dec 26, 2016

Thanks for your PR!

@DavertMik DavertMik merged commit e978795 into Codeception:2.2 Dec 26, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
@bimmelino

This comment has been minimized.

Show comment
Hide comment
@bimmelino

bimmelino Dec 30, 2016

thanks for code review and merging it

thanks for code review and merging it

chris1312 added a commit to chris1312/Codeception that referenced this pull request Jun 16, 2017

show JS errors in html report (#3821)
* optional feature for webdriver: log JavaScript errors in html

* optional feature for webdriver: log JavaScript errors in html

* optional feature for webdriver: log JavaScript errors in html

* optional feature for webdriver: log JavaScript errors

* optional feature for webdriver: log JavaScript errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment