Core components content fragment handling fixes #391

kaczymuczy · 2019-10-14T10:51:39Z

Description

Fixes for BB enabling the handling of Content Fragment core component
Required for: wttech/bobcat-aem-tests#13
Part of #373

Motivation and Context

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:

I have read the CONTRIBUTING document.
My code follows the code styleguide of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.

…gment component

Fix DefaultSelect to enable BB the configuration of the Content Fragment component

…fixes

Add ContentFragmentPathBrowser dialog field

...m/cognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowserImpl.java

mkrzyzanowski · 2019-10-14T13:19:28Z

...m/cognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowserImpl.java

+    // the application. Sometimes a warning dialog appears and it needs to be handled.
+    if (bobcatWait
+        .tweak(new TimingsBuilder().explicitTimeout(1).build())
+        .ignoring(Stream.of(NoSuchElementException.class).collect(Collectors.toList()))


Why a stream?
You can use: ignoring(NoSuchElementException.class)

I was testing this solution and thought that I might add more Exceptions later. But I didn't :D

mkrzyzanowski · 2019-10-14T13:54:06Z

...m/cognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowserImpl.java

+
+    // Branching logic in tests is not OK, it's true. But this part handles an unpredictable part of
+    // the application. Sometimes a warning dialog appears and it needs to be handled.
+    if (bobcatWait


Try to extract the logic into a self-describing method instead of commenting.

About the logic itself: are you trying to wait for the warning message -> confirm it -> verify the message disappeared?
Few comments:

you are ignoring the results of the last isConditionMet - you burn some CPU time here, nothing more.

why the if clause in the first place? Does the message not appear every time? How about a following flow: wait for message to appear -> click it -> wait for it to disappear?

OK, will move to separate method.
The warning message doesn't always appear and I don't know yet what causes it to appear.
I know I'm ignoring the results of isConditionMet - it's a pretty sleep, nothing more, but it helps me avoid handling Selenium Exceptions

mkrzyzanowski · 2019-10-14T14:00:46Z

...main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultNumberInput.java

+/**
+ * Default implementation of {@link NumberInput}
+ */
+@PageObject(css = Locators.NUMBERINPUT_CSS)


If the locator is not used anywhere else, put it here directly.

mkrzyzanowski · 2019-10-14T14:05:54Z

...main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultPathBrowser.java

-
-    bobcatWait.until(elementToBeClickable(firstResult)).click();
+    bobcatWait.until(elementToBeClickable(firstResult));
+    input.sendKeys(Keys.ENTER);


Why the change from the previous implementation?

The previous implementation didn't support external links, like to github.com - BB would click the first result suggestem by AEM, so a page within the AEM instance instead of github.com

mkrzyzanowski · 2019-10-14T14:06:39Z

.../src/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultSelect.java


 /**
 * Default implementation of {@link Select}
 */
-@PageObject(xpath = "//coral-select/..")
+@PageObject(css = Locators.SELECT_CSS)


Same as above - if this is the only place where the selector is used, leave it here.

mkrzyzanowski · 2019-10-14T14:10:24Z

.../src/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultSelect.java

  private List<WebElement> label;

+  @Inject
+  private BobcatWait wait;


mkrzyzanowski · 2019-10-14T14:10:47Z

...c/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultTextfield.java


 /**
 * Default implementation of {@link Textfield}
 */
-@PageObject(css = Locators.FIELD_WRAPPER_CSS)
+@PageObject(css = Locators.TEXTFIELD_CSS)


Same comment regarding locator

mkrzyzanowski · 2019-10-14T14:13:53Z

...m/cognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowserImpl.java

+ * Default implementation of {@link PathBrowser}
+ */
+@PageObject(css = Locators.AUTOCOMPLETE_CSS)
+public class ContentFragmentPathBrowserImpl implements ContentFragmentPathBrowser {


The DefaultXYZ naming is more aligned with the rest fields

mkrzyzanowski · 2019-10-22T12:52:38Z

...ognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultContentFragmentPathBrowser.java

+ * #%L
+ * Bobcat
+ * %%
+ * Copyright (C) 2016 Cognifide Ltd.


Lies, it's 2019

volodymyr.vashchuk added 5 commits October 10, 2019 13:59

Fix handling of Textfield to enable configuration for the Content Fra…

f65d045

…gment component

Fix an error in the DefaultTextfield

1a594fb

Fix DefaultSelect to enable BB the configuration of the Content Fragment component

Merge branch 'master' into core-components-content-fragment-handling-…

bcc90e2

…fixes

Add NumberInput dialog field

2f0ea6a

Add ContentFragmentPathBrowser dialog field

Fix handling the content fragment warning dialog

a0a8c3d

kaczymuczy mentioned this pull request Oct 14, 2019

Core components content fragment tests wttech/bobcat-aem-tests#13

Merged

mkrzyzanowski suggested changes Oct 14, 2019

View reviewed changes

volodymyr.vashchuk added 2 commits October 15, 2019 11:28

Fix most issues mentioned in CR

0f9e02e

Fix the last of the issues mentioned in the CR

a09cf05

kaczymuczy requested a review from mkrzyzanowski October 15, 2019 10:06

mkrzyzanowski reviewed Oct 22, 2019

View reviewed changes

mkrzyzanowski approved these changes Oct 22, 2019

View reviewed changes

Fix the current year in copyright comments

2d9b0c3

mkrzyzanowski merged commit fdad29b into master Oct 22, 2019

mkrzyzanowski deleted the core-components-content-fragment-handling-fixes branch October 22, 2019 14:21

kaczymuczy restored the core-components-content-fragment-handling-fixes branch October 22, 2019 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core components content fragment handling fixes #391

Core components content fragment handling fixes #391

kaczymuczy commented Oct 14, 2019 •

edited

Loading

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 14, 2019

kaczymuczy Oct 15, 2019

mkrzyzanowski Oct 22, 2019

kaczymuczy Oct 22, 2019

Core components content fragment handling fixes #391

Core components content fragment handling fixes #391

Conversation

kaczymuczy commented Oct 14, 2019 • edited Loading

Description

Motivation and Context

Types of changes

Checklist:

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

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

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

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

kaczymuczy commented Oct 14, 2019 •

edited

Loading