Feature/exclude #303

PiteroS678 · 2018-08-03T11:17:49Z

Description

Merged with treshold
Added new parameter exclude-elements to screen collector.

Motivation and Context

We want to compare screenshots without specified elements.

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.

…t comparator, created new status conditionally passed

malaskowski · 2018-08-03T13:38:39Z

core/jobs/src/main/java/com/cognifide/aet/job/common/collectors/screen/ScreenCollector.java

@@ -97,6 +127,13 @@ public void setParameters(Map<String, String> params) throws ParametersException
        .isNotBlank(params.get(CSS_PARAM))) {
      setElementParams(params);
    }
+    if(params.containsKey(EXCLUDE_ELEMENT_PARAM)) {
+      if (StringUtils.isNotBlank(params.get(EXCLUDE_ELEMENT_PARAM))) {
+        namesOfExcludeElements = params.get("exclude-elements");


Please use constant also here.

malaskowski

Please update CHANGELOG according to AET Contributing rules

malaskowski · 2018-08-03T13:42:22Z

core/jobs/src/main/java/com/cognifide/aet/job/common/comparators/layout/LayoutComparator.java

+      mask = new ByteArrayInputStream(baos.toByteArray());
+      String maskArtifactId = artifactsDAO.saveArtifact(properties, mask, CONTENT_TYPE);
+
+      if (isMaskWithoutDifference(imageComparisonResult) && !excludeFunctionIsOn) {


Switching those arguments will be more performant.

malaskowski · 2018-08-03T13:45:32Z

...communication-api/src/main/java/com/cognifide/aet/communication/api/metadata/StepResult.java

@@ -32,14 +32,22 @@
  private static final long serialVersionUID = 7758484589529051815L;


After updating Serializable class, please update this property.

tkaik · 2018-08-06T10:36:37Z

core/jobs/src/main/java/com/cognifide/aet/job/common/collectors/screen/ScreenCollector.java

@@ -56,12 +62,27 @@

  private final CollectorProperties properties;

+  private String namesOfExcludeElements;


Consider changing the name of this field - it hold a CSS selector of elements to exclude, so maybe something like excludeCssSelector?

tkaik · 2018-08-06T10:48:24Z

...nication-api/src/main/java/com/cognifide/aet/communication/api/metadata/exclude/Exclude.java

+import java.io.Serializable;
+import java.util.List;
+
+public class Exclude implements Serializable {


Please change the class name to something more meaningful - it should be a noun, not a verb

malaskowski · 2018-08-07T08:19:01Z

After merging #293 please change the target branch to master.

malaskowski · 2018-08-07T08:24:10Z

core/jobs/src/main/java/com/cognifide/aet/job/common/comparators/layout/LayoutComparator.java

@@ -55,14 +59,27 @@

  private Double percentageThreshold;

+  private boolean excludeFunctionIsOn = false;


You don't have to initialise boolean with false, its default value.

malaskowski · 2018-08-07T08:26:37Z

core/jobs/src/main/java/com/cognifide/aet/job/common/collectors/screen/ScreenCollector.java

  ScreenCollector(CollectorProperties properties, WebDriver webDriver, ArtifactsDAO artifactsDAO) {
    this.properties = properties;
    this.webDriver = webDriver;
    this.artifactsDAO = artifactsDAO;
  }

+  private List<ExcludedElement> getExcludeElementsFromWebElements(
+      List<WebElement> webElements) {
+    List<ExcludedElement> excludeExcludedElements = new LinkedList<>();


Please use ArrayList here to have better performance. You may estimate it's size with webElements.size().

malaskowski · 2018-08-07T08:35:40Z

documentation/src/main/wiki/ScreenCollector.md

 ##### Parameters

 | Parameter | Value | Description | Mandatory |
 | --------- | ----- | ----------- | --------- |
 | `xpath` | xpath_to_element | Xpath to element(s) | optional (either xpath or css) |
 | `css` | css_selector_to_element | css selector to element(s)| optional (either xpath or css) |
+| `exclude-elements` | css_selector_to_element | If you provide elements in exclude-elements, they won't be checked by layout comparator, the difference won't be included for `pixelThreshold` and `percentageThreshold` and you won't see the difference on the mask. | no |


How about that description:

Elements found with that selector will be ignored by layout comparator (they won't affect its results) but will be rendered on the report as captured.

tkaik

Please update Changelog.md

…e/exclude # Conflicts: # integration-tests/sanity-functional/src/test/java/com/cognifide/aet/sanity/functional/HomePageTilesTest.java # integration-tests/sanity-functional/src/test/resources/features/filtering.feature # integration-tests/test-suite/partials/layout.xml

PiteroS678 and others added 30 commits August 3, 2018 10:39

Preparing prototype

9a2a0c5

created threshold, moved logic from image comparision result to layou…

60f3205

…t comparator, created new status conditionally passed

created tests for threshold

f013b93

created status conditionally passed in frontend

f769083

created documentation for threshold parameters

aa31764

Preparing prototype

645fb72

merge with treshold

2b53717

Preparing prototype

5e82351

updated documentation

ddebdaf

Update SuiteReportLayoutCase.md

d3bccfe

Refactor

bff0d18

added sum of passed and conditionally passed in side panel

7039644

rafactor saveArtifact

972a64a

added integration tests for exclude

b8d98fe

updated photo

6d087df

added documentation about exclude elements

49527b4

removed blank line

55b1f12

updated documentation

0a31c80

centered button

27c45c5

added integrations tests

4386af1

fixed wrong class in dynamic content

ccc8720

fixed typo in test name

2ae9496

removed unused incrementation

23e4997

Refactor

2c82913

Show difference in test - fix

3930e29

ScreenCollector refactor

a47a732

Wiki - move exclude-elements parameter to ScreenCollector

77e96a9

ScreenCollector refactor

41ba6cb

Added height to tests

bc02476

href update

a08bacd

malaskowski changed the base branch from master to newfeature/layout_comparator_with_treshold August 3, 2018 13:18

malaskowski added the enhancement label Aug 3, 2018

malaskowski reviewed Aug 3, 2018

View reviewed changes

malaskowski suggested changes Aug 3, 2018

View reviewed changes

PiteroS678 added 3 commits August 6, 2018 08:50

StepResult - serialVersionUID update

282dc0f

LayoutComparator - update

a5b09de

Payload & ScreenCollector - refactor

adc9e4e

tkaik reviewed Aug 6, 2018

View reviewed changes

ScreenCollector - refactor

5385992

Exclude feature - refactor

54fc03d

malaskowski suggested changes Aug 7, 2018

View reviewed changes

PiteroS678 added 3 commits August 7, 2018 13:09

Exclude feature - refactor

6b7db4d

ScreenCollector - refactor

8427fc9

Wiki - ScreenCollector update

8d42fd3

malaskowski approved these changes Aug 8, 2018

View reviewed changes

plutasnyy force-pushed the newfeature/layout_comparator_with_treshold branch 4 times, most recently from 6847792 to f2dbc05 Compare August 10, 2018 07:15

plutasnyy changed the base branch from newfeature/layout_comparator_with_treshold to master August 14, 2018 07:17

plutasnyy and others added 2 commits August 14, 2018 12:30

merge with master

c113b3d

Merge branch 'master' into feature/exclude

09d18c0

tkaik suggested changes Aug 21, 2018

View reviewed changes

tkaik and others added 2 commits August 21, 2018 17:11

CHANGELOG update

70e120d

tkaik approved these changes Aug 22, 2018

View reviewed changes

tkaik merged commit 29d8ec3 into master Aug 22, 2018

tkaik deleted the feature/exclude branch August 22, 2018 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/exclude #303

Feature/exclude #303

PiteroS678 commented Aug 3, 2018 •

edited

Loading

malaskowski Aug 3, 2018

malaskowski left a comment

malaskowski Aug 3, 2018

malaskowski Aug 3, 2018

tkaik Aug 6, 2018

tkaik Aug 6, 2018

malaskowski commented Aug 7, 2018

malaskowski Aug 7, 2018

malaskowski Aug 7, 2018

malaskowski Aug 7, 2018

tkaik left a comment

		@@ -32,14 +32,22 @@
		private static final long serialVersionUID = 7758484589529051815L;

		@@ -56,12 +62,27 @@

		private final CollectorProperties properties;

		private String namesOfExcludeElements;

		@@ -55,14 +59,27 @@

		private Double percentageThreshold;

		private boolean excludeFunctionIsOn = false;

Feature/exclude #303

Feature/exclude #303

Conversation

PiteroS678 commented Aug 3, 2018 • edited Loading

Description

Motivation and Context

Screenshots (if appropriate):

Types of changes

Checklist:

Choose a reason for hiding this comment

malaskowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malaskowski commented Aug 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkaik left a comment

Choose a reason for hiding this comment

PiteroS678 commented Aug 3, 2018 •

edited

Loading