JavaScript parser improvement

atest/acceptance/keywords/async_javascript.robot

@@ -6,9 +6,19 @@ Resource          ../resource.robot
 
 *** Test Cases ***
 Should Not Timeout If Callback Invoked Immediately
-    ${result} =    Execute Async Javascript    arguments[arguments.length - 1](123);
+    ${result} =    Execute Async Javascript
+    ...    JAVASCRIPT
+    ...    arguments[arguments.length - 1](123);
     Should Be Equal    ${result}    ${123}
 
+Execute Async Javascript With ARGUMENTS and JAVASCRIPT Marker
+    Execute Async Javascript
+    ...  ARGUMENTS
+    ...  123
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    Alert Should Be Present    123    timeout=10 s
+
 Should Be Able To Return Javascript Primitives From Async Scripts Neither None Nor Undefined
     ${result} =    Execute Async Javascript    arguments[arguments.length - 1](123);
     Should Be Equal    ${result}    ${123}

atest/acceptance/keywords/javascript.robot

@@ -20,18 +20,63 @@ Mouse Down On Link
     Mouse Up    link_mousedown
 
 Execute Javascript
-    [Documentation]    LOG 2 Executing JavaScript:
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
     ...    window.add_content('button_target', 'Inserted directly')
+    ...    Without any arguments.
     Execute Javascript    window.add_content('button_target', 'Inserted directly')
     Page Should Contain    Inserted directly
 
+Execute Javascript With ARGUMENTS and JAVASCRIPT Marker
+    Execute Javascript
+    ...  ARGUMENTS
+    ...  123
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    Alert Should Be Present    123    timeout=10 s
+
+Execute Javascript With JAVASCRIPT and ARGUMENTS Marker
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
+    ...    alert(arguments[0]);
+    ...    By using argument:
+    ...    '123'
+    Execute Javascript
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    ...  ARGUMENTS
+    ...  123
+    Alert Should Be Present    123    timeout=10 s
+
+Execute Javascript With ARGUMENTS Marker Only
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
+    ...    alert(arguments[0]);
+    ...    By using arguments:
+    ...    '123' and '0987'
+    Execute Javascript
+    ...  alert(arguments[0]);
+    ...  ARGUMENTS
+    ...  123
+    ...  0987
+    Alert Should Be Present    123    timeout=10 s
+
 Execute Javascript from File
-    [Documentation]    LOG 2:1 REGEXP: Reading JavaScript from file .*
+    [Documentation]
+    ...    LOG 2:1 REGEXP: Reading JavaScript from file .*executed_by_execute_javascript.*
     ...    LOG 2:2 Executing JavaScript:
     ...    window.add_content('button_target', 'Inserted via file')
+    ...    Without any arguments.
     Execute Javascript    ${CURDIR}/executed_by_execute_javascript.js
     Page Should Contain    Inserted via file
 
+Execute Javascript from File With ARGUMENTS Marker
+    Execute Javascript
+    ...    ${CURDIR}/javascript_alert.js
+    ...    ARGUMENTS
+    ...    123
+    Alert Should Be Present    123    timeout=10 s
+
 Open Context Menu
     [Tags]    Known Issue Safari
     Go To Page "javascript/context_menu.html"

atest/acceptance/keywords/javascript_alert.js

@@ -0,0 +1 @@
+alert(arguments[0]);

requirements-dev.txt

@@ -3,7 +3,7 @@
 
 mockito >= 1.0.0
 robotstatuschecker
-approvaltests >= 0.2.3
+approvaltests >= 0.2.4
 
 # Include normal dependencies from requirements.txt. Makes it possible to use
 # requirements-dev.txt as a single requirement file in PyCharm and other IDEs.

src/SeleniumLibrary/keywords/javascript.py

@@ -15,21 +15,28 @@
 # limitations under the License.
 
 import os
+from collections import namedtuple
+
+from robot.utils import plural_or_not, seq2str
 
 from SeleniumLibrary.base import LibraryComponent, keyword
 
 
 class JavaScriptKeywords(LibraryComponent):
 
+    js_marker = 'JAVASCRIPT'
+    arg_marker = 'ARGUMENTS'
+
     @keyword
     def execute_javascript(self, *code):
-        """Executes the given JavaScript code.
+        """Executes the given JavaScript code with possible arguments.
 
-        ``code`` may contain multiple lines of code and may be divided into
-        multiple cells in the test data. In that case, the parts are
-        concatenated together without adding spaces.
+        ``code`` may be divided into multiple cells in the test data and
+        ``code`` may contain multiple lines of code and arguments. In that case,
+        the JavaScript code parts are concatenated together without adding
+        spaces and optional arguments are separated from ``code``.
 
-        If ``code`` is an absolute path to an existing file, the JavaScript
+        If ``code`` is a path to an existing file, the JavaScript
         to execute will be read from that file. Forward slashes work as
         a path separator on all operating systems.
 
@@ -42,19 +49,30 @@ def execute_javascript(self, *code):
         This keyword returns whatever the executed JavaScript code returns.
         Return values are converted to the appropriate Python types.
 
+        Starting from SeleniumLibrary 3.2 it is possible to provide JavaScript
+        [https://seleniumhq.github.io/selenium/docs/api/py/webdriver_remote/selenium.webdriver.remote.webdriver.html#selenium.webdriver.remote.webdriver.WebDriver.execute_script|
+        arguments] as part of ``code`` argument. The JavaScript code and
+        arguments must be separated with `JAVASCRIPT` and `ARGUMENTS` markers
+        and must used exactly with this format. If the Javascript code is
+        first, then the `JAVASCRIPT` marker is optional. The order of
+        `JAVASCRIPT` and `ARGUMENTS` markers can swapped, but if `ARGUMENTS`
+        is first marker, then `JAVASCRIPT` marker is mandatory. It is only
+        allowed to use `JAVASCRIPT` and `ARGUMENTS` markers only one time in the
+        ``code`` argument.
+
         Examples:
         | `Execute JavaScript` | window.myFunc('arg1', 'arg2') |
         | `Execute JavaScript` | ${CURDIR}/js_to_execute.js    |
-        | ${sum} =             | `Execute JavaScript` | return 1 + 1; |
-        | `Should Be Equal`    | ${sum}               | ${2}          |
+        | `Execute JavaScript` | alert(arguments[0]); | ARGUMENTS | 123 |
+        | `Execute JavaScript` | ARGUMENTS | 123 | JAVASCRIPT | alert(arguments[0]); |
         """
-        js = self._get_javascript_to_execute(code)
-        self.info("Executing JavaScript:\n%s" % js)
-        return self.driver.execute_script(js)
+        js_code, js_args = self._get_javascript_to_execute(code)
+        self._js_logger('Executing JavaScript', js_code, js_args)
+        return self.driver.execute_script(js_code, *js_args)
 
     @keyword
     def execute_async_javascript(self, *code):
-        """Executes asynchronous JavaScript code.
+        """Executes asynchronous JavaScript code with possible arguments.
 
         Similar to `Execute Javascript` except that scripts executed with
         this keyword must explicitly signal they are finished by invoking the
@@ -64,6 +82,11 @@ def execute_async_javascript(self, *code):
         Scripts must complete within the script timeout or this keyword will
         fail. See the `Timeouts` section for more information.
 
+        Starting from SeleniumLibrary 3.2 it is possible to provide JavaScript
+        [https://seleniumhq.github.io/selenium/docs/api/py/webdriver_remote/selenium.webdriver.remote.webdriver.html#selenium.webdriver.remote.webdriver.WebDriver.execute_async_script|
+        arguments] as part of ``code`` argument. See `Execute Javascript` for
+        more details.
+
         Examples:
         | `Execute Async JavaScript` | var callback = arguments[arguments.length - 1]; window.setTimeout(callback, 2000); |
         | `Execute Async JavaScript` | ${CURDIR}/async_js_to_execute.js |
@@ -73,19 +96,72 @@ def execute_async_javascript(self, *code):
         | ...         | window.setTimeout(answer, 2000);                |
         | `Should Be Equal` | ${result} | text |
         """
-        js = self._get_javascript_to_execute(code)
-        self.info("Executing Asynchronous JavaScript:\n%s" % js)
-        return self.driver.execute_async_script(js)
-
-    def _get_javascript_to_execute(self, lines):
-        code = ''.join(lines)
-        path = code.replace('/', os.sep)
-        if os.path.isabs(path) and os.path.isfile(path):
-            code = self._read_javascript_from_file(path)
-        return code
+        js_code, js_args = self._get_javascript_to_execute(code)
+        self._js_logger('Executing Asynchronous JavaScript', js_code, js_args)
+        return self.driver.execute_async_script(js_code, *js_args)
+
+    def _js_logger(self, base, code, args):
+        message = '%s:\n%s\n' % (base, code)
+        if args:
+            message = ('%sBy using argument%s:\n%s'
+                       % (message, plural_or_not(args), seq2str(args)))
+        else:
+            message = '%sWithout any arguments.' % message
+        self.info(message)
+
+    def _get_javascript_to_execute(self, code):
+        js_code, js_args = self._separate_code_and_args(code)
+        if not js_code:
+            raise ValueError('JavaScript code was not found from code argument.')
+        js_code = ''.join(js_code)
+        path = js_code.replace('/', os.sep)
+        if os.path.isfile(path):
+            js_code = self._read_javascript_from_file(path)
+        return js_code, js_args
+
+    def _separate_code_and_args(self, code):
+        code = list(code)
+        self._check_marker_error(code)
+        index = self._get_marker_index(code)
+        if self.arg_marker not in code:
+            return code[index.js + 1:], []
+        if self.js_marker not in code:
+            return code[0:index.arg], code[index.arg + 1:]
+        else:
+            if index.js == 0:
+                return code[index.js + 1:index.arg], code[index.arg + 1:]
+            else:
+                return code[index.js + 1:], code[index.arg + 1:index.js]
+
+    def _check_marker_error(self, code):
+        if not code:
+            raise ValueError('There must be at least one argument defined.')
+        message = None
+        template = '%s marker was found two times in the code.'
+        if code.count(self.js_marker) > 1:
+            message = template % self.js_marker
+        if code.count(self.arg_marker) > 1:
+            message = template % self.arg_marker
+        index = self._get_marker_index(code)
+        if index.js > 0 and index.arg != 0:
+            message = template % self.js_marker
+        if message:
+            raise ValueError(message)
+
+    def _get_marker_index(self, code):
+        Index = namedtuple('Index', 'js arg')
+        if self.js_marker in code:
+            js = code.index(self.js_marker)
+        else:
+            js = -1
+        if self.arg_marker in code:
+            arg = code.index(self.arg_marker)
+        else:
+            arg = -1
+        return Index(js=js, arg=arg)
 
     def _read_javascript_from_file(self, path):
         self.info('Reading JavaScript from file <a href="file://%s">%s</a>.'
-                  .format(path.replace(os.sep, '/'), path), html=True)
+                  % (path.replace(os.sep, '/'), path), html=True)
         with open(path) as file:
             return file.read().strip()

utest/test/keywords/approvaltests_config.json

@@ -1,3 +1,3 @@
 {
   "subdirectory": "approved_files"
-}
+}

utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_check_marker_error.approved.txt

@@ -0,0 +1,24 @@
+error
+
+0) There must be at least one argument defined.
+1) ARGUMENTS marker was found two times in the code.
+2) JAVASCRIPT marker was found two times in the code.
+3) ARGUMENTS marker was found two times in the code.
+4) JAVASCRIPT marker was found two times in the code.
+5) ARGUMENTS marker was found two times in the code.
+6) JAVASCRIPT marker was found two times in the code.
+7) There must be at least one argument defined.
+8) None
+9) None
+10) None
+11) None
+12) None
+13) None
+14) None
+15) None
+16) None
+17) None
+18) None
+19) None
+20) None
+21) None

utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript.approved.txt

@@ -1 +1 @@
-code here
+codehere + []

utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript_no_code.approved.txt

@@ -0,0 +1 @@
+JavaScript code was not found from code argument.

utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_indexing.approved.txt

@@ -0,0 +1,17 @@
+index
+
+0) Index(js=-1, arg=-1)
+1) Index(js=-1, arg=-1)
+2) Index(js=-1, arg=-1)
+3) Index(js=0, arg=-1)
+4) Index(js=-1, arg=2)
+5) Index(js=-1, arg=-1)
+6) Index(js=-1, arg=-1)
+7) Index(js=0, arg=3)
+8) Index(js=0, arg=-1)
+9) Index(js=1, arg=0)
+10) Index(js=0, arg=3)
+11) Index(js=3, arg=0)
+12) Index(js=-1, arg=-1)
+13) Index(js=-1, arg=-1)
+14) Index(js=0, arg=-1)

utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_separate_code_and_args.approved.txt

@@ -0,0 +1,17 @@
+code and args
+
+0) There must be at least one argument defined.
+1) (['code1'], [])
+2) (['code1', 'code2'], [])
+3) (['code1', 'code2'], [])
+4) (['code1', 'code2'], ['arg1', 'arg2'])
+5) (['code1', 'code2', 'arguments', 'arg1', 'arg2'], [])
+6) (['javascript', 'code1', 'code2'], [])
+7) (['code1', 'code2'], [])
+8) (['code1', 'code2', 'argUMENTs'], [])
+9) (['code1', 'code2'], [])
+10) (['code1', 'code2'], ['arg1', 'arg2'])
+11) (['code1', 'code2'], ['arg1', 'arg2'])
+12) (['aRGUMENTS', 'arg1', 'arg2', 'jAVASCRIPT', 'code1', 'code2'], [])
+13) (['JAVASCRIPTCODE', 'code1', 'code2'], [])
+14) (['code1', 'code2', 'ARGUMENTS ARG2', 'arg3'], [])