From 99e225c72962a576c6fe5a66eae480a13ad67934 Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Thu, 11 Oct 2018 16:50:04 +0200 Subject: [PATCH 1/7] Add algorithm to enable taking long screenshots without resolution-sleep-resolution workaround --- .../resolution/ResolutionModifier.java | 38 +++++++-- .../common/modifiers/sleep/SleepModifier.java | 7 +- .../aet/job/common/utils/CurrentThread.java | 28 +++++++ .../aet/job/common/utils/Sampler.java | 79 +++++++++++++++++++ .../aet/job/common/utils/SamplerTest.java | 64 +++++++++++++++ .../layout/long_expanding_page.jsp | 55 +++++++++++++ .../sanity/functional/HomePageTilesTest.java | 8 +- .../test/resources/features/filtering.feature | 2 +- .../test-suite/partials/layout.xml | 37 +++++++-- 9 files changed, 293 insertions(+), 25 deletions(-) create mode 100644 core/jobs/src/main/java/com/cognifide/aet/job/common/utils/CurrentThread.java create mode 100644 core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java create mode 100644 core/jobs/src/test/java/com/cognifide/aet/job/common/utils/SamplerTest.java create mode 100644 integration-tests/sample-site/src/main/webapp/sanity/comparators/layout/long_expanding_page.jsp diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java index 6d15442be..68327e5fd 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java @@ -20,7 +20,11 @@ import com.cognifide.aet.job.api.collector.CollectorJob; import com.cognifide.aet.job.api.exceptions.ParametersException; import com.cognifide.aet.job.api.exceptions.ProcessingException; +import com.cognifide.aet.job.common.utils.Sampler; +import java.util.Arrays; +import java.util.LinkedList; import java.util.Map; +import java.util.function.Supplier; import org.apache.commons.lang3.math.NumberUtils; import org.openqa.selenium.Dimension; import org.openqa.selenium.JavascriptExecutor; @@ -39,6 +43,8 @@ public class ResolutionModifier implements CollectorJob { private static final String HEIGHT_PARAM = "height"; + private static final String SAMPLING_PERIOD_PARAM = "samplingPeriod"; + private static final String JAVASCRIPT_GET_BODY_HEIGHT = "return document.body.scrollHeight"; private static final int MAX_SIZE = 35000; @@ -47,12 +53,20 @@ public class ResolutionModifier implements CollectorJob { private static final int HEIGHT_NOT_DEFINED = 0; + private static final int DEFAULT_SAMPLING_WAIT_PERIOD = 100; + + private static final int MAX_SAMPLES_THRESHOLD = 15; + + public static final int SAMPLE_QUEUE_SIZE = 3; + private final WebDriver webDriver; private int width; private int height; + private int samplingPeriod; + public ResolutionModifier(WebDriver webDriver) { this.webDriver = webDriver; } @@ -72,7 +86,11 @@ public void setParameters(Map params) throws ParametersException if (params.containsKey(HEIGHT_PARAM)) { height = NumberUtils.toInt(params.get(HEIGHT_PARAM)); ParametersValidator - .checkRange(height, 1, MAX_SIZE, "Height should be greater than 0 and smaller than " + MAX_SIZE); + .checkRange(height, 1, MAX_SIZE, + "Height should be greater than 0 and smaller than " + MAX_SIZE); + } else { + samplingPeriod = params.containsKey(SAMPLING_PERIOD_PARAM) ? NumberUtils + .toInt(params.get(SAMPLING_PERIOD_PARAM)) : DEFAULT_SAMPLING_WAIT_PERIOD; } } else { throw new ParametersException("You have to specify width, height parameter is optional"); @@ -80,18 +98,24 @@ public void setParameters(Map params) throws ParametersException } private void setResolution(WebDriver webDriver) { - Window window = webDriver.manage().window(); if (height == HEIGHT_NOT_DEFINED) { - window.setSize(new Dimension(width, INITIAL_HEIGHT)); - JavascriptExecutor js = (JavascriptExecutor) webDriver; - height = Integer - .parseInt(js.executeScript(JAVASCRIPT_GET_BODY_HEIGHT).toString()); + height = calculateWindowHeight(webDriver); if (height > MAX_SIZE) { LOG.warn("Height is over browser limit, changing height to {}", MAX_SIZE); height = MAX_SIZE; } } LOG.info("Setting resolution to {}x{} ", width, height); - window.setSize(new Dimension(width, height)); + webDriver.manage().window().setSize(new Dimension(width, height)); + } + + private int calculateWindowHeight(WebDriver webDriver) { + Window window = webDriver.manage().window(); + window.setSize(new Dimension(width, INITIAL_HEIGHT)); + + Supplier heightSupplier = () -> Integer.parseInt( + ((JavascriptExecutor) webDriver).executeScript(JAVASCRIPT_GET_BODY_HEIGHT).toString()); + return Sampler + .waitForValue(heightSupplier, samplingPeriod, SAMPLE_QUEUE_SIZE, MAX_SAMPLES_THRESHOLD); } } diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/sleep/SleepModifier.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/sleep/SleepModifier.java index 22a12e8af..7a8aecc19 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/sleep/SleepModifier.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/sleep/SleepModifier.java @@ -20,6 +20,7 @@ import com.cognifide.aet.job.api.collector.CollectorJob; import com.cognifide.aet.job.api.exceptions.ParametersException; import com.cognifide.aet.job.api.exceptions.ProcessingException; +import com.cognifide.aet.job.common.utils.CurrentThread; import java.util.Map; import org.apache.commons.lang3.math.NumberUtils; import org.slf4j.Logger; @@ -53,11 +54,7 @@ public void setParameters(Map params) throws ParametersException } private void sleepInMillis(int ms) { - try { - Thread.sleep(ms); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } + CurrentThread.sleep(ms); } } diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/CurrentThread.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/CurrentThread.java new file mode 100644 index 000000000..880fa7f93 --- /dev/null +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/CurrentThread.java @@ -0,0 +1,28 @@ +/** + * AET + * + * Copyright (C) 2013 Cognifide Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package com.cognifide.aet.job.common.utils; + +public final class CurrentThread { + + public static void sleep(int ms) { + try { + Thread.sleep(ms); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } +} diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java new file mode 100644 index 000000000..e8a0ad927 --- /dev/null +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java @@ -0,0 +1,79 @@ +/** + * AET + * + * Copyright (C) 2013 Cognifide Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.cognifide.aet.job.common.utils; + +import com.cognifide.aet.job.common.modifiers.resolution.ResolutionModifier; +import java.util.LinkedList; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class Sampler { + + private static final Logger LOG = LoggerFactory.getLogger(ResolutionModifier.class); + + /** + * Collects values from supplier in specified periods of time, and compares last n samples every + * iteration. If all n samples are equal(), returns the value. If last n samples don't match + * before max iterations threshold is reached, returns last collected sample. + * + * @param samplesSupplier supplier of value to wait for, + * @param samplingPeriod milliseconds period between taking each sample, + * @param sampleQueueSize defines the last n elements that are to be compared, + * @param maxSamplesThreshold max number of samples before return + * @return true if pattern is empty or if its match with given value, false otherwise + */ + public static T waitForValue(Supplier samplesSupplier, int samplingPeriod, + int sampleQueueSize, int maxSamplesThreshold) { + LinkedList samplesQueue = new LinkedList<>(); + + int samplesTaken = 0; + while (!isThresholdReached(samplesTaken, maxSamplesThreshold) && + !areAllSamplesEqual(samplesQueue, sampleQueueSize)) { + + CurrentThread.sleep(samplingPeriod); + + T nextSample = samplesSupplier.get(); + removeLast(samplesQueue, sampleQueueSize); + samplesQueue.addFirst(nextSample); + ++samplesTaken; + } + return samplesQueue.getFirst(); + } + + private static boolean isThresholdReached(int samplesTaken, int maxSamplesThreshold) { + if (samplesTaken >= maxSamplesThreshold) { + LOG.warn("Sampling reached threshold"); + return true; + } + return false; + } + + private static boolean areAllSamplesEqual(LinkedList samplesQueue, int sampleSize) { + return isQueueFull(samplesQueue, sampleSize) && + samplesQueue.stream().allMatch(sample -> samplesQueue.get(0).equals(sample)); + } + + private static boolean isQueueFull(LinkedList samplesQueue, int queueSize) { + return samplesQueue.size() == queueSize; + } + + private static void removeLast(LinkedList samplesQueue, int sampleQueueSize) { + if (isQueueFull(samplesQueue, sampleQueueSize)) { + samplesQueue.removeLast(); + } + } +} diff --git a/core/jobs/src/test/java/com/cognifide/aet/job/common/utils/SamplerTest.java b/core/jobs/src/test/java/com/cognifide/aet/job/common/utils/SamplerTest.java new file mode 100644 index 000000000..4fb39a847 --- /dev/null +++ b/core/jobs/src/test/java/com/cognifide/aet/job/common/utils/SamplerTest.java @@ -0,0 +1,64 @@ +/** + * AET + * + * Copyright (C) 2013 Cognifide Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.cognifide.aet.job.common.utils; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Random; +import java.util.function.Supplier; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class SamplerTest { + + private static final int MAX_SAMPLES_THRESHOLD = 5; + private static final int SAMPLE_QUEUE_SIZE = 3; + private static final int SAMPLING_PERIOD = 1; + + @Mock + private Supplier supplier; + + @Test + public void sampleChangingValueTest_AllSamplesMatch() { + when(supplier.get()).thenReturn(0); + + Integer finalSample = Sampler + .waitForValue(supplier, SAMPLING_PERIOD, SAMPLE_QUEUE_SIZE, MAX_SAMPLES_THRESHOLD); + + assertEquals((int) finalSample, 0); + } + + @Test + public void sampleChangingValueTest_AllSamplesDiffer_ThresholdReached() { + Random random = new Random(); + when(supplier.get()) + .thenReturn(random.nextInt()) + .thenReturn(random.nextInt()) + .thenReturn(random.nextInt()) + .thenReturn(random.nextInt()) + .thenReturn(random.nextInt()); + + Sampler.waitForValue(supplier, 1, 3, 5); + + verify(supplier, times(5)).get(); + } +} \ No newline at end of file diff --git a/integration-tests/sample-site/src/main/webapp/sanity/comparators/layout/long_expanding_page.jsp b/integration-tests/sample-site/src/main/webapp/sanity/comparators/layout/long_expanding_page.jsp new file mode 100644 index 000000000..c9ccb8404 --- /dev/null +++ b/integration-tests/sample-site/src/main/webapp/sanity/comparators/layout/long_expanding_page.jsp @@ -0,0 +1,55 @@ +<%-- + + AET + + Copyright (C) 2013 Cognifide Limited + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +--%> + +<%-- + This page simulates a page that takes a long time to load. + The final height of the page is above 12k pixels. +--%> + + + +<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %> +<%@ include file="/includes/header.jsp" %> +
+<%@ include file="dynamic_content.jsp" %> + diff --git a/integration-tests/sanity-functional/src/test/java/com/cognifide/aet/sanity/functional/HomePageTilesTest.java b/integration-tests/sanity-functional/src/test/java/com/cognifide/aet/sanity/functional/HomePageTilesTest.java index 92c56c999..2e0198ae4 100644 --- a/integration-tests/sanity-functional/src/test/java/com/cognifide/aet/sanity/functional/HomePageTilesTest.java +++ b/integration-tests/sanity-functional/src/test/java/com/cognifide/aet/sanity/functional/HomePageTilesTest.java @@ -29,15 +29,15 @@ @Modules(GuiceModule.class) public class HomePageTilesTest { - private static final int TESTS = 138; + private static final int TESTS = 140; - private static final int EXPECTED_TESTS_SUCCESS = 78; + private static final int EXPECTED_TESTS_SUCCESS = 79; - private static final int EXPECTED_TESTS_CONDITIONALLY_PASSED = 10; + private static final int EXPECTED_TESTS_CONDITIONALLY_PASSED = 11; private static final int EXPECTED_TESTS_WARN = 5; - private static final int EXPECTED_TESTS_FAIL = 55; + private static final int EXPECTED_TESTS_FAIL = 56; @Inject private ReportHomePage page; diff --git a/integration-tests/sanity-functional/src/test/resources/features/filtering.feature b/integration-tests/sanity-functional/src/test/resources/features/filtering.feature index 4971783ea..ce39f5143 100644 --- a/integration-tests/sanity-functional/src/test/resources/features/filtering.feature +++ b/integration-tests/sanity-functional/src/test/resources/features/filtering.feature @@ -38,7 +38,7 @@ Feature: Tests Results Filtering Given I have opened sample tests report page When I search for tests containing "layout" Then There are 37 tiles visible - And Statistics text contains "37 ( 16 / 0 / 21 (10) / 0 )" + And Statistics text contains "39 ( 17 / 0 / 22 (11) / 0 )" Scenario: Filtering Tests Results: jserrors Given I have opened sample tests report page diff --git a/integration-tests/test-suite/partials/layout.xml b/integration-tests/test-suite/partials/layout.xml index 3d3734bf2..c00480ecd 100644 --- a/integration-tests/test-suite/partials/layout.xml +++ b/integration-tests/test-suite/partials/layout.xml @@ -479,10 +479,6 @@ - - - @@ -497,10 +493,6 @@ - - - @@ -511,5 +503,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 597d913aefab08b2a95f52fd3d6b12d145a7876e Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Mon, 15 Oct 2018 13:35:17 +0200 Subject: [PATCH 2/7] Update documentation --- CHANGELOG.md | 1 + .../src/main/wiki/ResolutionModifier.md | 35 +++---------------- documentation/src/main/wiki/UpgradeNotes.md | 5 --- 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52edcbe30..4067df561 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to AET will be documented in this file. **List of changes that are finished but not yet released in any final version.** - [PR-403](https://github.com/Cognifide/aet/pull/403) Conditionally passed tests can be accepted ([#400](https://github.com/Cognifide/aet/issues/400)) - [PR-387](https://github.com/Cognifide/aet/pull/387) Set max allowed page screenshot height to 35k pixels. +- [PR-397](https://github.com/Cognifide/aet/pull/397) Add algorithm to enable taking long screenshots without resolution-sleep-resolution workaround ## Version 3.0.1 diff --git a/documentation/src/main/wiki/ResolutionModifier.md b/documentation/src/main/wiki/ResolutionModifier.md index 239af1e28..1ca0fc4cf 100644 --- a/documentation/src/main/wiki/ResolutionModifier.md +++ b/documentation/src/main/wiki/ResolutionModifier.md @@ -14,10 +14,14 @@ Module name: **resolution** | --------- | ----- | ----------- | --------- | | `width` | int (1 to 35000) | Window width | yes | | `height` | int (1 to 35000) | Window height | no | +| `samplingPeriod` | int (milliseconds) | Used when `height` is not defined. Defaults to 100ms (see notes below) | no | + +TODO: Change 15000 to 35000 when (https://github.com/Cognifide/aet/pull/387) is merged. | Note | |:------ | -| When height is not specified then it's computed by JavaScript (using `document.body.scrollHeight` property). | +| When `height` is not specified then it's computed by JavaScript (using `document.body.scrollHeight` property). | +| For very long pages, it may take some time to render the page in order to get its full height, so AET is using an algorithm that samples the page's height over some specified period of time. `samplingPeriod` specifies the amount of time between taking each sample. If defined number of samples would match (3 last samples) or when the max number of samples is reached (15), the acquired valued is used as `height` resolution for screenshot.| | **If the resolution is specified without height parameter it should be specified after [`open`](https://github.com/Cognifide/aet/wiki/Open)** and after all modifiers which may affect the page height (e.g. [`hide`](https://github.com/Cognifide/aet/wiki/HideModifier)) | ##### Example Usage @@ -47,35 +51,6 @@ Module name: **resolution** ``` -##### Known issues - -[#357](https://github.com/Cognifide/aet/issues/357) - If you're using the auto-height calculation feature of [[Resolution Modifier|ResolutionModifier]], it may happen that the -height of collected screenshot is different every time you run the suite, which results in failures on the report. -Currently you can use one of following workarounds to fix this issues: -* specify the `height` parameter manually with a value which is equal or greater than the height of page you want to test, e.g.: - ```$xml - - - - ``` -* use an additional `resolution` modifier with any `height` (value doesn't matter) before the `open` phase - to ensure that - the page will be opened with desired `width` and the 2nd `resolution` will only compute and change the height. - ```$xml - - - - - ``` -* use two `resolution` modifiers with the same `width` attribute (the first one may also have `height` attribute with any value) - and a `sleep` modifier between them, e.g: - ```$xml - - - - - - ``` - #### Tips and tricks In order to make sure that your screenshots have the resolution you expect them to have you need to test it first. diff --git a/documentation/src/main/wiki/UpgradeNotes.md b/documentation/src/main/wiki/UpgradeNotes.md index 1a9bed72e..87e94cfdf 100644 --- a/documentation/src/main/wiki/UpgradeNotes.md +++ b/documentation/src/main/wiki/UpgradeNotes.md @@ -27,11 +27,6 @@ the width of collected screenshot could be different that the width set in the ` (see "Notes" section in [[Resolution Modifier|ResolutionModifier]] wiki). This issue doesn't occur when using AET 3.0 with Chrome browser - make sure to adjust the resolution `width` value when updating your suite from previous AET versions. -##### Known issues - -* [#357](https://github.com/Cognifide/aet/issues/357) - see Known issues section in [[Resolution Modifier|ResolutionModifier]] wiki -for possible workarounds. - #### `aet-maven-plugin` marked as deprecated That means it will be no longer supported after release of this version and expect it will be removed soon. Please use [[client script|ClientScripts]] instead or simply communicate with AET Web API to schedule your suite. From 4c3689183baab3d081acca64b0f8b91ca8f9f495 Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Wed, 17 Oct 2018 15:46:30 +0200 Subject: [PATCH 3/7] Fix logger creation --- .../main/java/com/cognifide/aet/job/common/utils/Sampler.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java index e8a0ad927..657ed3be6 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java @@ -15,7 +15,6 @@ */ package com.cognifide.aet.job.common.utils; -import com.cognifide.aet.job.common.modifiers.resolution.ResolutionModifier; import java.util.LinkedList; import java.util.function.Supplier; import org.slf4j.Logger; @@ -23,7 +22,7 @@ public class Sampler { - private static final Logger LOG = LoggerFactory.getLogger(ResolutionModifier.class); + private static final Logger LOG = LoggerFactory.getLogger(Sampler.class); /** * Collects values from supplier in specified periods of time, and compares last n samples every From 675de4c00eefd1b962e28a1e03a6ad9b2fe429cc Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Thu, 18 Oct 2018 13:26:15 +0200 Subject: [PATCH 4/7] Use CircularFifoBuffer instead of LinkedList in algorithm --- .../aet/job/common/utils/Sampler.java | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java index 657ed3be6..1a4e0ffe8 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java @@ -17,6 +17,7 @@ import java.util.LinkedList; import java.util.function.Supplier; +import org.apache.commons.collections.buffer.CircularFifoBuffer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,24 +34,23 @@ public class Sampler { * @param samplingPeriod milliseconds period between taking each sample, * @param sampleQueueSize defines the last n elements that are to be compared, * @param maxSamplesThreshold max number of samples before return - * @return true if pattern is empty or if its match with given value, false otherwise + * @return last collected sample */ public static T waitForValue(Supplier samplesSupplier, int samplingPeriod, int sampleQueueSize, int maxSamplesThreshold) { - LinkedList samplesQueue = new LinkedList<>(); + CircularFifoBuffer samplesQueue = new CircularFifoBuffer(sampleQueueSize); int samplesTaken = 0; while (!isThresholdReached(samplesTaken, maxSamplesThreshold) && - !areAllSamplesEqual(samplesQueue, sampleQueueSize)) { + !areAllSamplesEqual(samplesQueue)) { CurrentThread.sleep(samplingPeriod); T nextSample = samplesSupplier.get(); - removeLast(samplesQueue, sampleQueueSize); - samplesQueue.addFirst(nextSample); + samplesQueue.add(nextSample); ++samplesTaken; } - return samplesQueue.getFirst(); + return (T) samplesQueue.get(); } private static boolean isThresholdReached(int samplesTaken, int maxSamplesThreshold) { @@ -61,18 +61,9 @@ private static boolean isThresholdReached(int samplesTaken, int maxSamplesThresh return false; } - private static boolean areAllSamplesEqual(LinkedList samplesQueue, int sampleSize) { - return isQueueFull(samplesQueue, sampleSize) && - samplesQueue.stream().allMatch(sample -> samplesQueue.get(0).equals(sample)); - } - - private static boolean isQueueFull(LinkedList samplesQueue, int queueSize) { - return samplesQueue.size() == queueSize; - } - - private static void removeLast(LinkedList samplesQueue, int sampleQueueSize) { - if (isQueueFull(samplesQueue, sampleQueueSize)) { - samplesQueue.removeLast(); - } + private static boolean areAllSamplesEqual(CircularFifoBuffer samplesQueue) { + return samplesQueue.isFull() && + samplesQueue.stream().allMatch(sample -> samplesQueue.get() != null && + samplesQueue.get().equals(sample)); } } From d94624e36a5339db44a9fa56fc2489824abd256c Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Mon, 22 Oct 2018 16:23:55 +0200 Subject: [PATCH 5/7] Fix review issues --- .../modifiers/resolution/ResolutionModifier.java | 10 +++++----- .../com/cognifide/aet/job/common/utils/Sampler.java | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java index 68327e5fd..cac6cbf02 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java @@ -21,9 +21,8 @@ import com.cognifide.aet.job.api.exceptions.ParametersException; import com.cognifide.aet.job.api.exceptions.ProcessingException; import com.cognifide.aet.job.common.utils.Sampler; -import java.util.Arrays; -import java.util.LinkedList; import java.util.Map; +import java.util.Optional; import java.util.function.Supplier; import org.apache.commons.lang3.math.NumberUtils; import org.openqa.selenium.Dimension; @@ -57,7 +56,7 @@ public class ResolutionModifier implements CollectorJob { private static final int MAX_SAMPLES_THRESHOLD = 15; - public static final int SAMPLE_QUEUE_SIZE = 3; + private static final int SAMPLE_QUEUE_SIZE = 3; private final WebDriver webDriver; @@ -89,8 +88,9 @@ public void setParameters(Map params) throws ParametersException .checkRange(height, 1, MAX_SIZE, "Height should be greater than 0 and smaller than " + MAX_SIZE); } else { - samplingPeriod = params.containsKey(SAMPLING_PERIOD_PARAM) ? NumberUtils - .toInt(params.get(SAMPLING_PERIOD_PARAM)) : DEFAULT_SAMPLING_WAIT_PERIOD; + samplingPeriod = Optional.ofNullable(params.get(SAMPLING_PERIOD_PARAM)) + .map(NumberUtils::toInt) + .orElse(DEFAULT_SAMPLING_WAIT_PERIOD); } } else { throw new ParametersException("You have to specify width, height parameter is optional"); diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java index 1a4e0ffe8..64ae574e4 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/utils/Sampler.java @@ -15,7 +15,6 @@ */ package com.cognifide.aet.job.common.utils; -import java.util.LinkedList; import java.util.function.Supplier; import org.apache.commons.collections.buffer.CircularFifoBuffer; import org.slf4j.Logger; From 16d944fa35bc43891f4dba9c3fb93279211d5ed2 Mon Sep 17 00:00:00 2001 From: Maciej Laskowski Date: Tue, 23 Oct 2018 11:04:23 +0200 Subject: [PATCH 6/7] Return default sampling wait time when conversion fails --- .../job/common/modifiers/resolution/ResolutionModifier.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java index cac6cbf02..1ec155d93 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java @@ -88,9 +88,8 @@ public void setParameters(Map params) throws ParametersException .checkRange(height, 1, MAX_SIZE, "Height should be greater than 0 and smaller than " + MAX_SIZE); } else { - samplingPeriod = Optional.ofNullable(params.get(SAMPLING_PERIOD_PARAM)) - .map(NumberUtils::toInt) - .orElse(DEFAULT_SAMPLING_WAIT_PERIOD); + samplingPeriod = NumberUtils + .toInt(params.get(SAMPLING_PERIOD_PARAM), DEFAULT_SAMPLING_WAIT_PERIOD); } } else { throw new ParametersException("You have to specify width, height parameter is optional"); From f9acd40bf6218bba3538d43691fd7e5ded3fea1e Mon Sep 17 00:00:00 2001 From: Jakub Izbicki Date: Tue, 23 Oct 2018 11:30:48 +0200 Subject: [PATCH 7/7] Check range for samplingPeriod --- .../resolution/ResolutionModifier.java | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java index 1ec155d93..3ae9ea85a 100644 --- a/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java +++ b/core/jobs/src/main/java/com/cognifide/aet/job/common/modifiers/resolution/ResolutionModifier.java @@ -19,10 +19,8 @@ import com.cognifide.aet.job.api.ParametersValidator; import com.cognifide.aet.job.api.collector.CollectorJob; import com.cognifide.aet.job.api.exceptions.ParametersException; -import com.cognifide.aet.job.api.exceptions.ProcessingException; import com.cognifide.aet.job.common.utils.Sampler; import java.util.Map; -import java.util.Optional; import java.util.function.Supplier; import org.apache.commons.lang3.math.NumberUtils; import org.openqa.selenium.Dimension; @@ -46,7 +44,7 @@ public class ResolutionModifier implements CollectorJob { private static final String JAVASCRIPT_GET_BODY_HEIGHT = "return document.body.scrollHeight"; - private static final int MAX_SIZE = 35000; + private static final int HEIGHT_MAX_SIZE = 35000; private static final int INITIAL_HEIGHT = 300; @@ -58,6 +56,8 @@ public class ResolutionModifier implements CollectorJob { private static final int SAMPLE_QUEUE_SIZE = 3; + private static final int MAX_SAMPLING_PERIOD = 10000; + private final WebDriver webDriver; private int width; @@ -72,7 +72,7 @@ public ResolutionModifier(WebDriver webDriver) { @Override - public CollectorStepResult collect() throws ProcessingException { + public CollectorStepResult collect() { setResolution(this.webDriver); return CollectorStepResult.newModifierResult(); } @@ -81,27 +81,39 @@ public CollectorStepResult collect() throws ProcessingException { public void setParameters(Map params) throws ParametersException { if (params.containsKey(WIDTH_PARAM)) { width = NumberUtils.toInt(params.get(WIDTH_PARAM)); - ParametersValidator.checkRange(width, 1, MAX_SIZE, "Width should be greater than 0"); + ParametersValidator.checkRange(width, 1, HEIGHT_MAX_SIZE, "Width should be greater than 0"); if (params.containsKey(HEIGHT_PARAM)) { - height = NumberUtils.toInt(params.get(HEIGHT_PARAM)); - ParametersValidator - .checkRange(height, 1, MAX_SIZE, - "Height should be greater than 0 and smaller than " + MAX_SIZE); + setHeight(params); } else { - samplingPeriod = NumberUtils - .toInt(params.get(SAMPLING_PERIOD_PARAM), DEFAULT_SAMPLING_WAIT_PERIOD); + setHeightSamplingPeriod(params); } } else { throw new ParametersException("You have to specify width, height parameter is optional"); } } + private void setHeight(Map params) throws ParametersException { + height = NumberUtils.toInt(params.get(HEIGHT_PARAM)); + ParametersValidator + .checkRange(height, 1, HEIGHT_MAX_SIZE, + "Height should be greater than 0 and smaller than " + HEIGHT_MAX_SIZE); + } + + private void setHeightSamplingPeriod(Map params) throws ParametersException { + samplingPeriod = NumberUtils + .toInt(params.get(SAMPLING_PERIOD_PARAM), DEFAULT_SAMPLING_WAIT_PERIOD); + ParametersValidator + .checkRange(samplingPeriod, 0, MAX_SAMPLING_PERIOD, + "samplingPeriod should be greater than or equal 0 and smaller or equal " + + MAX_SAMPLING_PERIOD); + } + private void setResolution(WebDriver webDriver) { if (height == HEIGHT_NOT_DEFINED) { height = calculateWindowHeight(webDriver); - if (height > MAX_SIZE) { - LOG.warn("Height is over browser limit, changing height to {}", MAX_SIZE); - height = MAX_SIZE; + if (height > HEIGHT_MAX_SIZE) { + LOG.warn("Height is over browser limit, changing height to {}", HEIGHT_MAX_SIZE); + height = HEIGHT_MAX_SIZE; } } LOG.info("Setting resolution to {}x{} ", width, height);