Full page screenshot support in Chrome by PiteroS678 · Pull Request #294 · wttech/aet

PiteroS678 · 2018-07-26T13:51:24Z

Description

Updated nu.validator version from 15.6.29 to 17.11.1
Updated ResolutionModifier to take full page screenshots in Chrome.
Updated HideModifier wiki
Updated suites & tests

Motivation and Context

AET supports Chrome as well as Firefox

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.

I hereby agree to the terms of the AET Contributor License Agreement.

…anges. Removed form which caused screen comparator stabilization issues.

…x/aet-integration-tests-stabilization

… into bugfix/aet-integration-tests-stabilization

…rade' into bugfix/aet-integration-tests-stabilization

…om:Cognifide/aet into bugfix/aet-integration-tests-stabilization

…Cognifide/aet into bugfix/aet-integration-tests-stabilization

…om:Cognifide/aet into bugfix/aet-integration-tests-stabilization

…fter HideModifier

ResolutionModifier - refactor

malaskowski

Please update Open module documentation. Please describe how it works, what is does with Chrome (e.g. waiting for DOM objects). What are differences between Chrome and Firefox.

malaskowski · 2018-07-30T10:52:49Z

-
-  @Deprecated
-  private boolean maximize;
+  private Optional<Integer> height;


Let's not use Optional as a type of a field. Please make it Integer.

malaskowski · 2018-07-30T10:53:26Z

 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.remote.RemoteWebDriver;
+
+import static org.mockito.Mockito.*;


Please don't use * in import.

malaskowski · 2018-07-30T10:57:40Z

 		</urls>
 	</test>

+    <!-- Chrome shows this error in console (FF didn't) that's why this error is being filtered. TODO: Remove this test or mark it as Failed and correct bobcat assertions -->


This TODO is done?

I think it's ok to keep this JS error filter here and just remove this TODO

malaskowski · 2018-07-30T10:59:52Z

+      localHeight = Integer
+          .parseInt(js.executeScript(JAVASCRIPT_GET_BODY_HEIGHT).toString());
+      if (localHeight > MAX_SIZE) {
+        LOG.info("Height is over browser limit, changing height to " + MAX_SIZE);


Instead of string concatenation, use logger {} feature. And this should be warning.

malaskowski · 2018-07-30T11:02:41Z

  private static final String HEIGHT_PARAM = "height";

-  private static final int MAX_SIZE = 100000;
+  private static final String JAVASCRIPT_GET_BODY_HEIGHT = "return document.body.scrollHeight";


Please add those changes into resolution modifier documentation:

height param is no longer mandatory,

max page height,

how does AET detect page height,

remove maximize parameter

Updated - 5f5e26c

malaskowski · 2018-07-30T11:09:02Z

@@ -24,6 +24,7 @@
 	<test name="S-comparator-Layout">


I'm missing here some tests, that will check page with auto-height detection and fixed-height, e.g.:

there is always changing element at the bottom of a page, scenario:

F with auto-height detection

S with fixed-height (let's say 100px), because screenshot does not contain this change

Added - 0ae1f70

tkaik · 2018-07-31T14:19:40Z

 |:----------------------- |
-| In order to use this modifier it must be declared after the open module in the definition of the test suite XML. |
+| *In order to use this modifier it must be declared after the open module in the definition of the test suite XML.
+*In order to use this modifier with Resolution Modifier it must be declared before the Resolution Modifier.|


Please explain why - e.g. because the Hide modifier might affect the total height of the page (when used without leaveBlankSpace="true")

tkaik · 2018-07-31T14:24:31Z

-|:----------------------- |
-| You cannot maximize the window and specify the dimension at the same time. If you specify height param you have to also specify width param and vice versa. |
+| Note |
+| When height is not specified then it's compute by JavaScript. <br/> If the resolution is specified without height parameter it should be specified after open. |


Please fix the typo compute -> computed and make the open a link text that points to https://github.com/Cognifide/aet/wiki/Open

malaskowski · 2018-08-01T10:24:48Z

+      if (params.containsKey(HEIGHT_PARAM)) {
+        height = NumberUtils.toInt(params.get(HEIGHT_PARAM));
+        ParametersValidator
+            .checkRange(height, 1, MAX_SIZE, "Height should be greater than 0");


Let's put here more verbose info:

"Height should be greater than 0 and smaller than " + MAX_SIZE

malaskowski

Please update CHANGELOG according to AET Contributing rules

…ests-stabilization

smoczysko88 and others added 24 commits July 18, 2018 12:44

Suite files udpated for chrome - line number changes, filter rules ch…

2a8dcb9

…anges. Removed form which caused screen comparator stabilization issues.

ChromeEntirePageScreen

ae0ac30

Merge remote-tracking branch 'origin/ChromeFullPageScreen' into bugfi…

cac7773

…x/aet-integration-tests-stabilization

Chrome limit handler && disabled scrolling

8a6f67b

Refactoring

bc537b4

Tests for ResolutionModifier update

c3f9f4f

Setting resolution for chrome screen comparator

26772c8

Collect page height update

330a8a6

Merge remote-tracking branch 'origin/ChromeFullPageScreenNoScrolling'…

9ada174

… into bugfix/aet-integration-tests-stabilization

Wiki - Hide Modifier update

834a837

Changed pageLoadStrategy && updated ResolutionModifierTest

e0120aa

Merge remote-tracking branch 'origin/ChromeFullPageScreenNoScrolling'…

49dbc7f

… into bugfix/aet-integration-tests-stabilization

New chrome arg to fix headless+proxy screenshot issue

ed587b2

Merge remote-tracking branch 'origin/ChromeFullPageScreenNoScrolling'…

239dc4a

… into bugfix/aet-integration-tests-stabilization

Revert pageLoadStrategy

503089f

Merge remote-tracking branch 'origin/ChromeFullPageScreenNoScrolling'…

863341b

… into bugfix/aet-integration-tests-stabilization

Merge remote-tracking branch 'origin/task/w3c-nuvalidator-version-upg…

483662c

…rade' into bugfix/aet-integration-tests-stabilization

Merge branch 'bugfix/aet-integration-tests-stabilization' of github.c…

6a0f420

…om:Cognifide/aet into bugfix/aet-integration-tests-stabilization

Merge branch 'ChromeFullPageScreenNoScrolling' of https://github.com/…

7f3d41b

…Cognifide/aet into bugfix/aet-integration-tests-stabilization

Merge branch 'bugfix/aet-integration-tests-stabilization' of github.c…

484381b

…om:Cognifide/aet into bugfix/aet-integration-tests-stabilization

S-comparator-Layout-Hide-leaveBlankSpace-false - ResolutionModifier a…

a5fb1d9

…fter HideModifier

ResolutionModifier - refactor

a2cff67

Decreased wait timeouts for tests stabilizations

2da4b7e

ResolutionModifier - refactor

b5b0eba

tkaik changed the title ~~Bugfix/aet integration tests stabilization~~ Full page screenshot support in Chrome, Jul 27, 2018

tkaik changed the title ~~Full page screenshot support in Chrome,~~ Full page screenshot support in Chrome Jul 27, 2018

PiteroS678 and others added 4 commits July 27, 2018 10:16

ResolutionModifierTest - update & refactor

b470103

Adjustments for Chrome compatibility

63de36c

Tests adjustments for chrome.

ac0178a

ResolutionModifierTest - new test & refactor,

69de42e

ResolutionModifier - refactor

Increased cookie max age for tests stabilization

a85618f

malaskowski mentioned this pull request Jul 30, 2018

Deprecated resolution modifier's parameter maximize removed, height no longer mandatory #169

Closed

malaskowski added the enhancement label Jul 30, 2018

tkaik changed the base branch from milestone/chrome-support to master July 30, 2018 10:51

tkaik changed the base branch from master to milestone/chrome-support July 30, 2018 10:51

malaskowski suggested changes Jul 30, 2018

View reviewed changes

PiteroS678 added 4 commits July 31, 2018 12:13

ResolutionModifier - refactor

b2e2e1d

ResolutionModifierTest - update

2cafbda

ResolutionModifierTest - refactor

80da189

Wiki - ResolutionModifier update

5f5e26c

tkaik reviewed Jul 31, 2018

View reviewed changes

PiteroS678 added 4 commits August 1, 2018 09:40

layout.xml - new tests

0ae1f70

height_detection.jsp - minor fix

0af3bf3

Wiki - ResolutionModifier update

12e19a9

Wiki - HideModifier update

499442c

malaskowski suggested changes Aug 1, 2018

View reviewed changes

ResolutionModifier update

864f783

malaskowski reviewed Aug 1, 2018

View reviewed changes

plutasnyy and others added 2 commits August 2, 2018 10:03

updated changelog

0ba974f

Merge branch 'milestone/chrome-support' into bugfix/aet-integration-t…

4533475

…ests-stabilization

tkaik approved these changes Aug 2, 2018

View reviewed changes

malaskowski approved these changes Aug 3, 2018

View reviewed changes

malaskowski merged commit 5d12f34 into milestone/chrome-support Aug 3, 2018

malaskowski deleted the bugfix/aet-integration-tests-stabilization branch August 3, 2018 10:29

tkaik mentioned this pull request Aug 6, 2018

Timeout-based integration tests stabilization for Chrome #304

Merged

6 tasks

tkaik mentioned this pull request Aug 21, 2018

Milestone/chrome support #329

Merged

6 tasks

Conversation

PiteroS678 commented Jul 26, 2018

Description

Motivation and Context

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

malaskowski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PiteroS678 Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malaskowski left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

PiteroS678 Jul 31, 2018 •

edited

Loading