From 0e2c7408c4c0a0db4614549ff3410e19575319de Mon Sep 17 00:00:00 2001 From: Jeremias Roessler Date: Wed, 23 Oct 2019 16:36:33 +0200 Subject: [PATCH 1/5] Since these are now handled in lines 68-75, they should only occurr if complex But then they show a misleading error message... --- src/main/java/de/retest/web/selenium/TestHealer.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/main/java/de/retest/web/selenium/TestHealer.java b/src/main/java/de/retest/web/selenium/TestHealer.java index 41e86c1a1..a71e44175 100644 --- a/src/main/java/de/retest/web/selenium/TestHealer.java +++ b/src/main/java/de/retest/web/selenium/TestHealer.java @@ -169,14 +169,6 @@ private WebElement findElementByCssSelector( final ByCssSelector by ) { private String retrieveUsableCssSelector( final ByCssSelector by ) { final String rawSelector = ByWhisperer.retrieveCssSelector( by ); - if ( rawSelector.startsWith( "#" ) ) { - throw new IllegalArgumentException( - "To search for element by ID, use `By.id()` instead of `#id` as CSS selector." ); - } - if ( !rawSelector.startsWith( "." ) ) { - throw new IllegalArgumentException( - "To search for element by tag, use `By.tagName()` instead of `tag` as CSS selector." ); - } // remove leading . final String selector = rawSelector.substring( 1 ); if ( selector.matches( ".*[<>:+\\s\"\\[\\*].*" ) ) { From a98c9ca59ac16db1b55f07c3c918c6ff840c0554 Mon Sep 17 00:00:00 2001 From: Jeremias Roessler Date: Wed, 23 Oct 2019 16:36:42 +0200 Subject: [PATCH 2/5] Improve error message --- src/main/java/de/retest/web/selenium/TestHealer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/retest/web/selenium/TestHealer.java b/src/main/java/de/retest/web/selenium/TestHealer.java index a71e44175..8a49fba61 100644 --- a/src/main/java/de/retest/web/selenium/TestHealer.java +++ b/src/main/java/de/retest/web/selenium/TestHealer.java @@ -172,7 +172,9 @@ private String retrieveUsableCssSelector( final ByCssSelector by ) { // remove leading . final String selector = rawSelector.substring( 1 ); if ( selector.matches( ".*[<>:+\\s\"\\[\\*].*" ) ) { - throw new IllegalArgumentException( "For now, only simple class selector is implemented." ); + throw new IllegalArgumentException( + "For now, only simple CSS selectors are implemented. Please report your chosen selector ('" + + rawSelector + "') at https://github.com/retest/recheck-web/issues." ); } return selector; } From 2c820b9444d28e8791e0045d9daeccc3e86ad089 Mon Sep 17 00:00:00 2001 From: Jeremias Roessler Date: Wed, 23 Oct 2019 22:33:14 +0200 Subject: [PATCH 3/5] Inline method call --- .../de/retest/web/selenium/TestHealer.java | 46 ++++++++----------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/src/main/java/de/retest/web/selenium/TestHealer.java b/src/main/java/de/retest/web/selenium/TestHealer.java index 8a49fba61..9ecbec378 100644 --- a/src/main/java/de/retest/web/selenium/TestHealer.java +++ b/src/main/java/de/retest/web/selenium/TestHealer.java @@ -65,13 +65,6 @@ private WebElement findElement( final By by ) { return findElementByLinkText( (ByLinkText) by ); } if ( by instanceof ByCssSelector ) { - final String rawSelector = ByWhisperer.retrieveCssSelector( (ByCssSelector) by ); - if ( rawSelector.startsWith( "#" ) && !isComplexCssSelector( rawSelector ) ) { - return findElement( By.id( rawSelector.substring( 1 ) ) ); - } - if ( !rawSelector.startsWith( "." ) && !isComplexCssSelector( rawSelector ) ) { - return findElement( By.tagName( rawSelector ) ); - } return findElementByCssSelector( (ByCssSelector) by ); } if ( by instanceof ByXPath ) { @@ -84,10 +77,6 @@ private WebElement findElement( final By by ) { "Healing tests with " + by.getClass().getSimpleName() + " not yet implemented" ); } - private boolean isComplexCssSelector( final String rawSelector ) { - return rawSelector.contains( " " ) || rawSelector.contains( "[" ); - } - private WebElement findElementById( final ById by ) { final String id = retrieveId( by ); final Element actualElement = @@ -152,10 +141,25 @@ private WebElement findElementByLinkText( final ByLinkText by ) { } private WebElement findElementByCssSelector( final ByCssSelector by ) { - final String selector = retrieveUsableCssSelector( by ); - - final Element actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, - lastActualState, CLASS, value -> ((String) value).contains( selector ) ); + final String selector = ByWhisperer.retrieveCssSelector( by ); + if ( selector.matches( ".*[<>:+\\s\"\\[\\*].*" ) ) { + throw new IllegalArgumentException( + "For now, only simple CSS selectors are implemented. Please report your chosen selector ('" + + selector + "') at https://github.com/retest/recheck-web/issues." ); + } + Element actualElement = null; + if ( selector.startsWith( "#" ) ) { + actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, lastActualState, ID, + selector.substring( 1 ) ); + } + if ( selector.matches( "^[a-zA-Z]*" ) ) { + actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, lastActualState, TYPE, + selector ); + } + if ( selector.startsWith( "." ) ) { + actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, lastActualState, CLASS, + value -> ((String) value).contains( selector.substring( 1 ) ) ); + } if ( actualElement == null ) { logger.warn( "{} with CSS selector '{}'.", ELEMENT_NOT_FOUND_MESSAGE, selector ); @@ -167,18 +171,6 @@ private WebElement findElementByCssSelector( final ByCssSelector by ) { } } - private String retrieveUsableCssSelector( final ByCssSelector by ) { - final String rawSelector = ByWhisperer.retrieveCssSelector( by ); - // remove leading . - final String selector = rawSelector.substring( 1 ); - if ( selector.matches( ".*[<>:+\\s\"\\[\\*].*" ) ) { - throw new IllegalArgumentException( - "For now, only simple CSS selectors are implemented. Please report your chosen selector ('" - + rawSelector + "') at https://github.com/retest/recheck-web/issues." ); - } - return selector; - } - private WebElement findElementByXPath( final ByXPath byXPath ) { final String xpathExpression = ByWhisperer.retrieveXPath( byXPath ); if ( xpathExpression.matches( ".*[<>:+\\s\"|'@\\*].*" ) ) { From 0d2d74251f379c968554d944dd64d069b0674c6d Mon Sep 17 00:00:00 2001 From: Jeremias Roessler Date: Thu, 24 Oct 2019 19:57:39 +0200 Subject: [PATCH 4/5] Instead of throwing an exception, we should just log and mention that we cannot heal that --- .../de/retest/web/selenium/TestHealer.java | 24 ++++-- .../retest/web/selenium/TestHealerTest.java | 74 +++++++------------ 2 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/main/java/de/retest/web/selenium/TestHealer.java b/src/main/java/de/retest/web/selenium/TestHealer.java index 9ecbec378..409fe62b3 100644 --- a/src/main/java/de/retest/web/selenium/TestHealer.java +++ b/src/main/java/de/retest/web/selenium/TestHealer.java @@ -142,10 +142,11 @@ private WebElement findElementByLinkText( final ByLinkText by ) { private WebElement findElementByCssSelector( final ByCssSelector by ) { final String selector = ByWhisperer.retrieveCssSelector( by ); - if ( selector.matches( ".*[<>:+\\s\"\\[\\*].*" ) ) { - throw new IllegalArgumentException( - "For now, only simple CSS selectors are implemented. Please report your chosen selector ('" - + selector + "') at https://github.com/retest/recheck-web/issues." ); + if ( isNotYetSupportedCssSelector( selector ) ) { + logger.warn( + "Unbreakable tests are not implemented for all CSS selectors. Please report your chosen selector ('{}') at https://github.com/retest/recheck-web/issues.", + selector ); + return null; } Element actualElement = null; if ( selector.startsWith( "#" ) ) { @@ -171,10 +172,21 @@ private WebElement findElementByCssSelector( final ByCssSelector by ) { } } + protected static boolean isNotYetSupportedCssSelector( final String selector ) { + return selector.matches( ".*[<>:+\\s\"\\[\\*].*" ); + } + + protected static boolean isNotYetSupportedXPathExpression( final String xpathExpression ) { + return xpathExpression.matches( ".*[<>:+\\s\"|'@\\*].*" ); + } + private WebElement findElementByXPath( final ByXPath byXPath ) { final String xpathExpression = ByWhisperer.retrieveXPath( byXPath ); - if ( xpathExpression.matches( ".*[<>:+\\s\"|'@\\*].*" ) ) { - throw new IllegalArgumentException( "For now, only simple class selector is implemented." ); + if ( isNotYetSupportedXPathExpression( xpathExpression ) ) { + logger.warn( + "Unbreakable tests are not implemented for all XPath selectors. Please report your chosen selector ('{}') at https://github.com/retest/recheck-web/issues.", + xpathExpression ); + return null; } final Element actualElement = findMatchingElement( xpathExpression ); diff --git a/src/test/java/de/retest/web/selenium/TestHealerTest.java b/src/test/java/de/retest/web/selenium/TestHealerTest.java index 2f7e3172e..e366546e9 100644 --- a/src/test/java/de/retest/web/selenium/TestHealerTest.java +++ b/src/test/java/de/retest/web/selenium/TestHealerTest.java @@ -3,8 +3,9 @@ import static de.retest.recheck.ui.Path.fromString; import static de.retest.recheck.ui.descriptors.Element.create; import static de.retest.web.selenium.TestHealer.findElement; +import static de.retest.web.selenium.TestHealer.isNotYetSupportedCssSelector; +import static de.retest.web.selenium.TestHealer.isNotYetSupportedXPathExpression; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -87,42 +88,33 @@ public void ByCssSelector_matches_elements_with_given_class() { } @Test - public void not_yet_implemented_ByCssSelector_should_throw_exception() { + public void empty_selectors_should_not_throw_exception() { final RecheckDriver wrapped = mock( RecheckDriver.class ); final RootElement state = mock( RootElement.class ); when( wrapped.getLastExpectedState() ).thenReturn( state ); when( wrapped.getLastActualState() ).thenReturn( state ); - try { - findElement( By.cssSelector( ".open > .dropdown-toggle.btn-primary" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.cssSelector( ".btn-primary[disabled]" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.cssSelector( "fieldset[disabled]" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.cssSelector( ".btn-group-vertical > .btn:not(:first-child):not(:last-child)" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.cssSelector( "[data-toggle=\"buttons\"] > .btn-group > .btn input[type=\"radio\"]" ), - wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.cssSelector( ".input-group[class*=\"col-\"]" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} + assertThat( findElement( By.cssSelector( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.className( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.id( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.linkText( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.name( "" ), wrapped ) ).isNull(); + // assertThat( findElement( By.partialLinkText( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.tagName( "" ), wrapped ) ).isNull(); + assertThat( findElement( By.xpath( "" ), wrapped ) ).isNull(); + } + + @Test + public void not_yet_implemented_ByCssSelector_should_be_logged() { + assertThat( isNotYetSupportedCssSelector( ".open > .dropdown-toggle.btn-primary" ) ).isTrue(); + assertThat( isNotYetSupportedCssSelector( ".btn-primary[disabled]" ) ).isTrue(); + assertThat( isNotYetSupportedCssSelector( "fieldset[disabled]" ) ).isTrue(); + assertThat( isNotYetSupportedCssSelector( ".btn-group-vertical > .btn:not(:first-child):not(:last-child)" ) ) + .isTrue(); + assertThat( + isNotYetSupportedCssSelector( "[data-toggle=\"buttons\"] > .btn-group > .btn input[type=\"radio\"]" ) ) + .isTrue(); + assertThat( isNotYetSupportedCssSelector( ".input-group[class*=\"col-\"]" ) ).isTrue(); } @Test @@ -147,20 +139,8 @@ public void ByXPathExpression_matches_elements_with_given_xpath() { } @Test - public void not_yet_implemented_ByXPathExpression_should_throw_exception() { - final RecheckDriver wrapped = mock( RecheckDriver.class ); - final RootElement state = mock( RootElement.class ); - when( wrapped.getLastExpectedState() ).thenReturn( state ); - when( wrapped.getLastActualState() ).thenReturn( state ); - - try { - findElement( By.xpath( "//div[@id='mw-content-text']/div[2]" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} - - try { - findElement( By.xpath( "//button[contains(.,'Search')]" ), wrapped ); - fail( "Excpected exception" ); - } catch ( final IllegalArgumentException e ) {} + public void not_yet_implemented_ByXPathExpression_should_log_warning() { + assertThat( isNotYetSupportedXPathExpression( "//div[@id='mw-content-text']/div[2]" ) ).isTrue(); + assertThat( isNotYetSupportedXPathExpression( "//button[contains(.,'Search')]" ) ).isTrue(); } } From 9805c4ee765a7d1e76d83281675174537f5182ea Mon Sep 17 00:00:00 2001 From: Jeremias Roessler Date: Thu, 24 Oct 2019 20:01:07 +0200 Subject: [PATCH 5/5] Account for hyphens --- .../de/retest/web/selenium/TestHealer.java | 2 +- .../retest/web/selenium/TestHealerTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/retest/web/selenium/TestHealer.java b/src/main/java/de/retest/web/selenium/TestHealer.java index 409fe62b3..aaaadd075 100644 --- a/src/main/java/de/retest/web/selenium/TestHealer.java +++ b/src/main/java/de/retest/web/selenium/TestHealer.java @@ -153,7 +153,7 @@ private WebElement findElementByCssSelector( final ByCssSelector by ) { actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, lastActualState, ID, selector.substring( 1 ) ); } - if ( selector.matches( "^[a-zA-Z]*" ) ) { + if ( selector.matches( "^[a-z\\-A-Z]*" ) ) { actualElement = de.retest.web.selenium.By.findElementByAttribute( lastExpectedState, lastActualState, TYPE, selector ); } diff --git a/src/test/java/de/retest/web/selenium/TestHealerTest.java b/src/test/java/de/retest/web/selenium/TestHealerTest.java index e366546e9..70bc4f8fa 100644 --- a/src/test/java/de/retest/web/selenium/TestHealerTest.java +++ b/src/test/java/de/retest/web/selenium/TestHealerTest.java @@ -63,6 +63,26 @@ public void ByCssSelector_using_tag_should_redirect() { assertThat( findElement( By.cssSelector( "div" ), wrapped ) ).isEqualTo( resultMarker ); } + @Test + public void ByCssSelector_using_tag_with_hyphen_should_redirect() { + final RecheckDriver wrapped = mock( RecheckDriver.class ); + final AutocheckingWebElement resultMarker = mock( AutocheckingWebElement.class ); + + final RootElement state = mock( RootElement.class ); + when( wrapped.getLastExpectedState() ).thenReturn( state ); + when( wrapped.getLastActualState() ).thenReturn( state ); + + final String xpath = "html[1]/ytd-grid-video-renderer[1]"; + final IdentifyingAttributes identifying = + IdentifyingAttributes.create( fromString( xpath ), "ytd-grid-video-renderer" ); + final Element element = create( "id", state, identifying, new MutableAttributes().immutable() ); + when( state.getContainedElements() ).thenReturn( Collections.singletonList( element ) ); + when( wrapped.findElement( By.xpath( xpath ) ) ).thenReturn( resultMarker ); + + // assertThat( By.cssSelector( "div" ).matches( element ) ).isTrue(); + assertThat( findElement( By.cssSelector( "ytd-grid-video-renderer" ), wrapped ) ).isEqualTo( resultMarker ); + } + @Test public void ByCssSelector_matches_elements_with_given_class() { final RecheckDriver wrapped = mock( RecheckDriver.class );