From e6e547691bc1d8e888f2fe5e3a18d169f48439ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Tue, 19 Sep 2017 23:31:54 +0300 Subject: [PATCH 1/7] utests for current locator parsing logic --- test/unit/locators/test_elementfinder.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/unit/locators/test_elementfinder.py b/test/unit/locators/test_elementfinder.py index 43754bfd9..e0c61a29d 100644 --- a/test/unit/locators/test_elementfinder.py +++ b/test/unit/locators/test_elementfinder.py @@ -5,6 +5,28 @@ from SeleniumLibrary.locators.elementfinder import ElementFinder +class ParseLocatorTests(unittest.TestCase): + + def test_implicit_xpath(self): + self._verify_parse_locator('//foo', 'xpath', '//foo') + self._verify_parse_locator('(//foo)', 'xpath', '(//foo)') + + def test_no_separator(self): + self._verify_parse_locator('foo', 'default', 'foo') + + def test_equal_sign_as_separator(self): + self._verify_parse_locator('class=foo', 'class', 'foo') + self._verify_parse_locator('id=foo=bar', 'id', 'foo=bar') + + def test_ignore_whitespace(self): + self._verify_parse_locator('class = foo', 'class', 'foo') + self._verify_parse_locator(' id = foo = bar ', 'id', 'foo = bar ') + + def _verify_parse_locator(self, locator, prefix, criteria): + parse_locator = ElementFinder(None)._parse_locator + self.assertEqual(parse_locator(locator), (prefix, criteria)) + + class ElementFinderTests(unittest.TestCase): def setUp(self): From 35830b26ef49144d97d100c9b5ce1ed6fc0954e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Tue, 19 Sep 2017 23:59:58 +0300 Subject: [PATCH 2/7] Support `:` as locator separator in addtion to `=`. See #908. Still need to decided how to handle locators which just happen to have `:` in them like `foo:bar`. With the current implementation they fail with a ValueError becaue `foo` is not a valid locator prefix. Could also change the logic to handle this so that `:` and `=` are separators *only* if the part before is a valid prefix. Docs are missing too. --- src/SeleniumLibrary/locators/elementfinder.py | 15 ++++++++++++--- test/acceptance/locators/xpath.robot | 1 + test/unit/locators/test_elementfinder.py | 11 +++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/SeleniumLibrary/locators/elementfinder.py b/src/SeleniumLibrary/locators/elementfinder.py index 1b37c8cb7..6092a8f6a 100644 --- a/src/SeleniumLibrary/locators/elementfinder.py +++ b/src/SeleniumLibrary/locators/elementfinder.py @@ -257,10 +257,19 @@ def _get_tag_and_constraints(self, tag): def _parse_locator(self, locator): if locator.startswith(('//', '(//')): return 'xpath', locator - if '=' not in locator: + index = self._get_locator_separator_index(locator) + if index == -1: return 'default', locator - prefix, criteria = locator.split('=', 1) - return prefix.strip(), criteria.lstrip() + prefix = locator[:index].strip() + criteria = locator[index+1:].lstrip() + return prefix, criteria + + def _get_locator_separator_index(self, locator): + if '=' not in locator: + return locator.find(':') + if ':' not in locator: + return locator.find('=') + return min(locator.find('='), locator.find(':')) def _element_matches(self, element, tag, constraints): if not element.tag_name.lower() == tag: diff --git a/test/acceptance/locators/xpath.robot b/test/acceptance/locators/xpath.robot index 728501616..e8bcc3466 100644 --- a/test/acceptance/locators/xpath.robot +++ b/test/acceptance/locators/xpath.robot @@ -6,6 +6,7 @@ Resource ../resource.robot *** Test Cases *** xpath with prefix should work Page Should Contain Element xpath=//div[@id="div_id"]/a + Page Should Contain Element xpath://div[@id="div_id"]/a xpath with // and without prefix should work Page Should Contain Element //div[@id="div_id"]/a diff --git a/test/unit/locators/test_elementfinder.py b/test/unit/locators/test_elementfinder.py index e0c61a29d..e9565ca87 100644 --- a/test/unit/locators/test_elementfinder.py +++ b/test/unit/locators/test_elementfinder.py @@ -10,6 +10,7 @@ class ParseLocatorTests(unittest.TestCase): def test_implicit_xpath(self): self._verify_parse_locator('//foo', 'xpath', '//foo') self._verify_parse_locator('(//foo)', 'xpath', '(//foo)') + self._verify_parse_locator('//id=bar', 'xpath', '//id=bar') def test_no_separator(self): self._verify_parse_locator('foo', 'default', 'foo') @@ -18,9 +19,19 @@ def test_equal_sign_as_separator(self): self._verify_parse_locator('class=foo', 'class', 'foo') self._verify_parse_locator('id=foo=bar', 'id', 'foo=bar') + def test_colon_as_separator(self): + self._verify_parse_locator('class:foo', 'class', 'foo') + self._verify_parse_locator('id:foo:bar', 'id', 'foo:bar') + + def test_use_first_separator_when_both_are_used(self): + self._verify_parse_locator('id:foo=bar', 'id', 'foo=bar') + self._verify_parse_locator('id=foo:bar', 'id', 'foo:bar') + def test_ignore_whitespace(self): self._verify_parse_locator('class = foo', 'class', 'foo') + self._verify_parse_locator('class : foo', 'class', 'foo') self._verify_parse_locator(' id = foo = bar ', 'id', 'foo = bar ') + self._verify_parse_locator(' id : foo : bar ', 'id', 'foo : bar ') def _verify_parse_locator(self, locator, prefix, criteria): parse_locator = ElementFinder(None)._parse_locator From 49a4c840b6b29c9cc6393f21ce99ed6e598e0faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Wed, 20 Sep 2017 22:21:11 +0300 Subject: [PATCH 3/7] Remove dead and borken code --- src/SeleniumLibrary/locators/elementfinder.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/SeleniumLibrary/locators/elementfinder.py b/src/SeleniumLibrary/locators/elementfinder.py index 6092a8f6a..e9eab81f6 100644 --- a/src/SeleniumLibrary/locators/elementfinder.py +++ b/src/SeleniumLibrary/locators/elementfinder.py @@ -124,9 +124,6 @@ def unregister(self, strategy_name): else: del self._strategies[strategy_name] - def has_strategy(self, strategy_name): - return strategy_name in self.strategies - def _find_by_identifier(self, criteria, tag, constraints): elements = self._normalize_result( self.browser.find_elements_by_id(criteria)) From cb419ca19f9e602c531a766375019c320fc502cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Wed, 20 Sep 2017 22:41:57 +0300 Subject: [PATCH 4/7] Remove duplicate unit tests. Didn't notice earlier that _parse_locator was already tested. Decided to remove old tests because the ones I added are a little simpler due to the use of `_verify_parse_locator` helper. Added few more tests to cover everything the old tests covered. --- test/unit/locators/test_elementfinder.py | 7 +- test/unit/locators/test_locator_parser.py | 79 ----------------------- 2 files changed, 6 insertions(+), 80 deletions(-) delete mode 100644 test/unit/locators/test_locator_parser.py diff --git a/test/unit/locators/test_elementfinder.py b/test/unit/locators/test_elementfinder.py index e9565ca87..cb2e321a7 100644 --- a/test/unit/locators/test_elementfinder.py +++ b/test/unit/locators/test_elementfinder.py @@ -14,6 +14,7 @@ def test_implicit_xpath(self): def test_no_separator(self): self._verify_parse_locator('foo', 'default', 'foo') + self._verify_parse_locator('', 'default', '') def test_equal_sign_as_separator(self): self._verify_parse_locator('class=foo', 'class', 'foo') @@ -27,7 +28,11 @@ def test_use_first_separator_when_both_are_used(self): self._verify_parse_locator('id:foo=bar', 'id', 'foo=bar') self._verify_parse_locator('id=foo:bar', 'id', 'foo:bar') - def test_ignore_whitespace(self): + def test_preserve_trailing_whitespace(self): + self._verify_parse_locator('//foo/bar ', 'xpath', '//foo/bar ') + self._verify_parse_locator('class=foo ', 'class', 'foo ') + + def test_remove_whitespace_around_prefix_and_separator(self): self._verify_parse_locator('class = foo', 'class', 'foo') self._verify_parse_locator('class : foo', 'class', 'foo') self._verify_parse_locator(' id = foo = bar ', 'id', 'foo = bar ') diff --git a/test/unit/locators/test_locator_parser.py b/test/unit/locators/test_locator_parser.py deleted file mode 100644 index 32795e7ba..000000000 --- a/test/unit/locators/test_locator_parser.py +++ /dev/null @@ -1,79 +0,0 @@ -import unittest - -from mockito import mock - -from SeleniumLibrary.locators.elementfinder import ElementFinder - - -class LocatorParserTests(unittest.TestCase): - - @classmethod - def setUpClass(cls): - ctx = mock() - cls.parse = ElementFinder(ctx)._parse_locator - - def test_parse_xpath(self): - prefix, criteria = self.parse('//foo/bar') - self.assertEqual(prefix, 'xpath') - self.assertEqual(criteria, '//foo/bar') - - prefix, criteria = self.parse('(//foo/bar)[2]') - self.assertEqual(prefix, 'xpath') - self.assertEqual(criteria, '(//foo/bar)[2]') - - prefix, criteria = self.parse('xpath=//foo[@attr=value]') - self.assertEqual(prefix, 'xpath') - self.assertEqual(criteria, '//foo[@attr=value]') - - def test_parse_dom(self): - prefix, criteria = self.parse( - 'dom=document.images[56]') - self.assertEqual(prefix, 'dom') - self.assertEqual(criteria, 'document.images[56]') - - def test_parse_link(self): - prefix, criteria = self.parse('link=My Link') - self.assertEqual(prefix, 'link') - self.assertEqual(criteria, 'My Link') - - prefix, criteria = self.parse('partial link=y Lin') - self.assertEqual(prefix, 'partial link') - self.assertEqual(criteria, 'y Lin') - - def test_parse_id(self): - prefix, criteria = self.parse('id=my_element') - self.assertEqual(prefix, 'id') - self.assertEqual(criteria, 'my_element') - - prefix, criteria = self.parse('my_element') - self.assertEqual(prefix, 'default') - self.assertEqual(criteria, 'my_element') - - def test_parse_leave_trailing_spaces(self): - prefix, criteria = self.parse('id= test1 ') - self.assertEqual(prefix, 'id') - self.assertEqual(criteria, 'test1 ') - - prefix, criteria = self.parse('//foo/bar ') - self.assertEqual(prefix, 'xpath') - self.assertEqual(criteria, '//foo/bar ') - - def test_parse_identifier(self): - prefix, criteria = self.parse('identifier=my_element') - self.assertEqual(prefix, 'identifier') - self.assertEqual(criteria, 'my_element') - - def test_parse_default(self): - prefix, criteria = self.parse('default=page?a=b') - self.assertEqual(prefix, 'default') - self.assertEqual(criteria, 'page?a=b') - - def test_parse_invalid(self): - prefix, criteria = self.parse('foo=bar') - self.assertEqual(prefix, 'foo') - self.assertEqual(criteria, 'bar') - - def test_parse_empty(self): - prefix, criteria = self.parse('') - self.assertEqual(prefix, 'default') - self.assertEqual(criteria, '') From 13007a6048c1a5131d762845748eaeb1b15b6f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Wed, 20 Sep 2017 23:02:09 +0300 Subject: [PATCH 5/7] Use locator prefix only if it is valid. Earlier `foo=bar` caused ValueError if `foo` wasn't a valid locator. Now this is same as `default=foo=bar`. Main motivation to the change is avoiding bwic problems now when we are adding support for locators using`foo:bar` syntax (#908). --- src/SeleniumLibrary/locators/elementfinder.py | 15 ++++++--------- test/unit/locators/test_elementfinder.py | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/SeleniumLibrary/locators/elementfinder.py b/src/SeleniumLibrary/locators/elementfinder.py index e9eab81f6..99bb5297c 100644 --- a/src/SeleniumLibrary/locators/elementfinder.py +++ b/src/SeleniumLibrary/locators/elementfinder.py @@ -61,10 +61,7 @@ def find(self, locator, tag=None, first_only=True, required=True): if isinstance(locator, WebElement): return locator prefix, criteria = self._parse_locator(locator) - if prefix not in self._strategies: - raise ValueError("Element locator with prefix '%s' " - "is not supported." % prefix) - strategy = self._strategies.get(prefix) + strategy = self._strategies[prefix] tag, constraints = self._get_tag_and_constraints(tag) elements = strategy(criteria, tag, constraints) if required and not elements: @@ -255,11 +252,11 @@ def _parse_locator(self, locator): if locator.startswith(('//', '(//')): return 'xpath', locator index = self._get_locator_separator_index(locator) - if index == -1: - return 'default', locator - prefix = locator[:index].strip() - criteria = locator[index+1:].lstrip() - return prefix, criteria + if index != -1: + prefix = locator[:index].strip() + if prefix in self._strategies: + return prefix, locator[index+1:].lstrip() + return 'default', locator def _get_locator_separator_index(self, locator): if '=' not in locator: diff --git a/test/unit/locators/test_elementfinder.py b/test/unit/locators/test_elementfinder.py index cb2e321a7..98f896f9a 100644 --- a/test/unit/locators/test_elementfinder.py +++ b/test/unit/locators/test_elementfinder.py @@ -7,6 +7,9 @@ class ParseLocatorTests(unittest.TestCase): + def setUp(self): + self.finder = ElementFinder(None) + def test_implicit_xpath(self): self._verify_parse_locator('//foo', 'xpath', '//foo') self._verify_parse_locator('(//foo)', 'xpath', '(//foo)') @@ -38,8 +41,19 @@ def test_remove_whitespace_around_prefix_and_separator(self): self._verify_parse_locator(' id = foo = bar ', 'id', 'foo = bar ') self._verify_parse_locator(' id : foo : bar ', 'id', 'foo : bar ') + def test_separator_without_matching_prefix_is_ignored(self): + self._verify_parse_locator('no=match', 'default', 'no=match') + self._verify_parse_locator('no:match', 'default', 'no:match') + + def test_registered_strategy_can_be_used_as_prefix(self): + self._verify_parse_locator('registered=no', 'default', 'registered=no') + self._verify_parse_locator('registered:no', 'default', 'registered:no') + self.finder.register('registered', lambda *args: None, persist=True) + self._verify_parse_locator('registered=yes!!', 'registered', 'yes!!') + self._verify_parse_locator('registered:yes!!', 'registered', 'yes!!') + def _verify_parse_locator(self, locator, prefix, criteria): - parse_locator = ElementFinder(None)._parse_locator + parse_locator = self.finder._parse_locator self.assertEqual(parse_locator(locator), (prefix, criteria)) From 668cea175040be3511c7b6ff688e80f2fcebe8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Wed, 20 Sep 2017 23:34:47 +0300 Subject: [PATCH 6/7] More generic test file name --- test/acceptance/locators/{xpath.robot => locator_parsing.robot} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/acceptance/locators/{xpath.robot => locator_parsing.robot} (100%) diff --git a/test/acceptance/locators/xpath.robot b/test/acceptance/locators/locator_parsing.robot similarity index 100% rename from test/acceptance/locators/xpath.robot rename to test/acceptance/locators/locator_parsing.robot From eff2b580dbacbeba9f2c7c7915a92482ad0633cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Kl=C3=A4rck?= Date: Wed, 20 Sep 2017 23:45:46 +0300 Subject: [PATCH 7/7] atests for locator parsing #908 Also tabs -> spaces in edited html file --- .../acceptance/locators/locator_parsing.robot | 22 +++++++++++++++++++ test/resources/html/links.html | 12 +++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/test/acceptance/locators/locator_parsing.robot b/test/acceptance/locators/locator_parsing.robot index e8bcc3466..0d7acab85 100644 --- a/test/acceptance/locators/locator_parsing.robot +++ b/test/acceptance/locators/locator_parsing.robot @@ -13,3 +13,25 @@ xpath with // and without prefix should work xpath with (// and without prefix should work Page Should Contain Element (//div[@id="div_id"]/a)[1] + +Locator without prefix + Page Should Contain Element div_id + +Locator with prefix + Page Should Contain Element id:div_id + Page Should Contain Element id=div_id + Page Should Contain Element id:foo:bar + Page Should Contain Element id=foo:bar + Page Should Contain Element id:bar=foo + Page Should Contain Element id=bar=foo + +Locator with separator but without matching prefix is not special + Page Should Contain Element foo:bar + Page Should Contain Element bar=foo + +Locator with separator and with matchign prefix cannot be used as-is + Page Should Contain Element id:id:problematic + Page Should Contain Element id=id:problematic + Run Keyword And Expect Error + ... Page should have contained element 'id:problematic' but did not + ... Page Should Contain Element id:problematic diff --git a/test/resources/html/links.html b/test/resources/html/links.html index 183c3bb5c..bf9c0eafc 100644 --- a/test/resources/html/links.html +++ b/test/resources/html/links.html @@ -17,13 +17,13 @@ Link with asterisk * and another *
Link with Unicode äöüÄÖÜß -