From 6e11b68d5934dfa936f927a7ba3e27a9e16c5031 Mon Sep 17 00:00:00 2001 From: Ed Manlove Date: Tue, 9 Jan 2024 12:03:57 -0500 Subject: [PATCH 1/5] Initial work on #1877 with some atests --- atest/acceptance/multiple_browsers_options.robot | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atest/acceptance/multiple_browsers_options.robot b/atest/acceptance/multiple_browsers_options.robot index af41306f0..31f86c99a 100644 --- a/atest/acceptance/multiple_browsers_options.robot +++ b/atest/acceptance/multiple_browsers_options.robot @@ -51,3 +51,7 @@ Chrome Browser With Selenium Options Argument With Semicolon ... LOG 1:14 DEBUG GLOB: *["has;semicolon"* Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument("has;semicolon") + +Chrome Browser with Selenium Options Argument Ending With Semicolon + Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} + ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument("--disable-dev-shm-usage") ; From ffbc8639778e941e4b5cc84f0107770e77d05bd7 Mon Sep 17 00:00:00 2001 From: Ed Manlove Date: Sat, 13 Jan 2024 16:14:00 -0500 Subject: [PATCH 2/5] Added additonal test cases around selenium options --- atest/acceptance/multiple_browsers_options.robot | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/atest/acceptance/multiple_browsers_options.robot b/atest/acceptance/multiple_browsers_options.robot index 31f86c99a..14dfde440 100644 --- a/atest/acceptance/multiple_browsers_options.robot +++ b/atest/acceptance/multiple_browsers_options.robot @@ -52,6 +52,19 @@ Chrome Browser With Selenium Options Argument With Semicolon Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument("has;semicolon") -Chrome Browser with Selenium Options Argument Ending With Semicolon +Chrome Browser with Selenium Options Ending With Semicolon Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument("--disable-dev-shm-usage") ; + +Chrome Browser with Selenium Options Ending With A Few Semicolons + Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} + ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument("--disable-dev-shm-usage") ; ; ; + +Chrome Browser with Selenium Options Containing Empty Option + Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} + ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument ( "--disable-dev-shm-usage" ) ; ; add_argument ( "--headless=new" ) + +Chrome Browser with Selenium Options With A Missing Semicolon + Run Keyword And Expect Error ValueError: Unable to parse option: "add_argument ( "--disable-dev-shm-usage" ) add_argument ( "--headless=new" )" + ... Open Browser ${FRONT PAGE} ${BROWSER} remote_url=${REMOTE_URL} + ... desired_capabilities=${DESIRED_CAPABILITIES} options=add_argument ( "--disable-dev-shm-usage" ) add_argument ( "--headless=new" ) From 66aabbd01836f41f3b7f8fda1fbe7dc3c1e79011 Mon Sep 17 00:00:00 2001 From: Ed Manlove Date: Sun, 14 Jan 2024 14:56:55 -0500 Subject: [PATCH 3/5] Allow for trailing semicolon within selenium options Within the selenium options argument of the `Open Browser` keword, if there were trailing semicolons then the keyword would fail. With this change we allow trailing semicolons. In addition the library warns about emtpy options. Fixes #1877 --- .../keywords/webdrivertools/webdrivertools.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py b/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py index 16e3d1f48..1cab9d36a 100644 --- a/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py +++ b/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py @@ -503,6 +503,9 @@ def create(self, browser, options): selenium_options = selenium_options() for option in options: for key in option: + if key == '' and option[key]==[]: + logger.warn('Empty selenium option found and ignored. Suggested you review options passed to `Open Browser` keyword') + continue attr = getattr(selenium_options, key) if callable(attr): attr(*option[key]) @@ -566,9 +569,12 @@ def _split(self, options): split_options = [] start_position = 0 tokens = generate_tokens(StringIO(options).readline) - for toknum, tokval, tokpos, _, _ in tokens: - if toknum == token.OP and tokval == ";": + for toktype, tokval, tokpos, _, _ in tokens: + if toktype == token.OP and tokval == ";": split_options.append(options[start_position : tokpos[1]].strip()) start_position = tokpos[1] + 1 - split_options.append(options[start_position:]) + # Handles trailing semicolon + # !! Note: If multiline options allowed this splitter might fail !! + if toktype == token.NEWLINE and start_position != tokpos[1]: + split_options.append(options[start_position : tokpos[1]].strip()) return split_options From 5be0ff2d4a02413e40c16d011a6b81e6dedbdc1b Mon Sep 17 00:00:00 2001 From: Ed Manlove Date: Sat, 20 Jan 2024 15:38:08 -0500 Subject: [PATCH 4/5] Updated unit test on option parser The new selenium options parser has a slightly different behaviour when it comes to multiple arguments with generious spacing within its option string. In stead of leaving preceeding and trailing spaces it removes those. This change is approved files corrects for this change. --- ...test_selenium_options_parser.test_split_options.approved.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utest/test/keywords/approved_files/test_selenium_options_parser.test_split_options.approved.txt b/utest/test/keywords/approved_files/test_selenium_options_parser.test_split_options.approved.txt index 5f96b0b0e..ba75ac3b6 100644 --- a/utest/test/keywords/approved_files/test_selenium_options_parser.test_split_options.approved.txt +++ b/utest/test/keywords/approved_files/test_selenium_options_parser.test_split_options.approved.txt @@ -5,4 +5,4 @@ Selenium options string splitting 2) ['attribute=True'] 3) ['attribute="semi;colons;middle"', 'other_attribute=True'] 4) ['method("arg1;")', 'method(";arg2;")'] -5) ['method ( " arg1 ")', ' method ( " arg2 " ) '] +5) ['method ( " arg1 ")', 'method ( " arg2 " )'] From 4ad50bb1355f23ecbbccae928f287820079f4c89 Mon Sep 17 00:00:00 2001 From: Ed Manlove Date: Sat, 20 Jan 2024 16:00:34 -0500 Subject: [PATCH 5/5] Removed unnecessary DebugLibrary I let this leak in. It shouldn't be there and it looks like it is causing trouble in GitHub Actions --- atest/acceptance/keywords/elements.robot | 1 - 1 file changed, 1 deletion(-) diff --git a/atest/acceptance/keywords/elements.robot b/atest/acceptance/keywords/elements.robot index 5419baa15..fa053b0ff 100644 --- a/atest/acceptance/keywords/elements.robot +++ b/atest/acceptance/keywords/elements.robot @@ -3,7 +3,6 @@ Documentation Tests elements Test Setup Go To Page "links.html" Resource ../resource.robot Library String -Library DebugLibrary *** Test Cases *** Get Many Elements