Fixed atest issues on Windows and added kws to split/join cli args.

- Added new 'Command Line To List' and 'List To Command Line' keywords to the Process library to ease splitting/joining cli args. Test and code review as well as documentation still missing. #2160 - Replaced simple 'shlex.split' usage in atests with the aforementioned Command Line To List keyword in Robot data and with enhanced shlex usage in Python libraries. - Added output files to generic Robot/Rebot execution keywords to avoid output streams getting full and execution hang on Windows. - Added 5 minute timeouts to Robot/Rebot execution keywords to avoid hanging in general. Above fixes related to the overall atest execution cleanup (#2091).
robotframework · Sep 28, 2015 · 334886c · 334886c
1 parent 14d0827
commit 334886c
Show file tree

Hide file tree

Showing 10 changed files with 177 additions and 29 deletions.
diff --git a/atest/resources/TestHelper.py b/atest/resources/TestHelper.py
@@ -1,5 +1,4 @@
 import os
-import shlex
 from stat import S_IREAD, S_IWRITE
 
 from robot.api import logger
@@ -23,9 +22,6 @@ def get_output_name(self, *datasources):
     def _get_name(self, path):
         return os.path.splitext(os.path.basename(path))[0]
 
-    def split_shell(self, args):
-        return [a.decode('utf8') for a in shlex.split(args.encode('utf8'))]
-
     def file_should_have_correct_line_separators(self, output, sep=os.linesep):
         if os.path.isfile(output):
             with open(output, 'rb') as infile:

diff --git a/atest/resources/atest_resource.robot b/atest/resources/atest_resource.robot
@@ -11,11 +11,15 @@ Library         read_interpreter.py
 Variables       atest_variables.py
 
 *** Variables ***
-${OUTDIR}       Set in Set Variables
-${OUTFILE}      -- ;; --
-${SYSLOG FILE}  -- ;; --
-${STDERR FILE}  -- ;; --
-${STDOUT FILE}  -- ;; --
+# FIXME: Variables below are currently only used as defaults and overridden in
+# "Set Variables". Should rather use same values always but should also decide
+# what are good values.
+${OUTDIR}       %{TEMPDIR}
+${OUTFILE}      %{TEMPDIR}${/}output.xml
+${SYSLOG FILE}  %{TEMPDIR}${/}syslog.txt
+${STDERR FILE}  %{TEMPDIR}${/}stdout.txt
+${STDOUT FILE}  %{TEMPDIR}${/}stderr.txt
+
 ${SUITE}        Set in Run Helper
 ${ERRORS}       -- ;; --
 ${LIBPATH1}     ${CURDIR}${/}..${/}testresources${/}testlibs
@@ -26,15 +30,17 @@ ${TESTNAME}     ${EMPTY}    # Used when not running test
 *** Keywords ***
 Run Robot Directly
     [Arguments]  ${opts and args}
-    ${opts and args} =   Split shell   ${opts and args}
-    ${result} =  Run Process  @{INTERPRETER.runner}  --outputdir  %{TEMPDIR}  @{opts and args}  stderr=STDOUT
+    ${opts and args} =   Command line to list   ${opts and args}
+    ${result} =  Run Process  @{INTERPRETER.runner}  --outputdir  %{TEMPDIR}  @{opts and args}
+    ...    stdout=${STDOUT FILE}    stderr=STDOUT    timeout=5min    on_timeout=terminate
     Log  ${result.stdout}
     [Return]  ${result}
 
 Run Rebot Directly
     [Arguments]  ${opts and args}
-    ${opts and args} =   Split shell   ${opts and args}
-    ${result} =  Run Process  @{INTERPRETER.rebot}  --outputdir  %{TEMPDIR}  @{opts and args}  stderr=STDOUT
+    ${opts and args} =   Command line to list    ${opts and args}
+    ${result} =  Run Process  @{INTERPRETER.rebot}  --outputdir  %{TEMPDIR}  @{opts and args}
+    ...    stdout=${STDOUT FILE}    stderr=STDOUT    timeout=5min    on_timeout=terminate
     Log  ${result.stdout}
     [Return]  ${result}
 
@@ -52,7 +58,7 @@ Run Tests Without Processing Output
 Run Tests Helper
     [Arguments]  ${user options}  @{data list}
     @{data list} =  Set Variables And Get Datasources  @{data list}
-    @{user options} =   Split shell  ${user options}
+    @{user options} =   Command line to list  ${user options}
     ${result} =  Run Helper  ${INTERPRETER.runner}
     ...    --ConsoleMarkers    OFF    # AUTO (default) doesn't work with IronPython
     ...    @{user options}
@@ -64,15 +70,15 @@ Run Tests Helper
 Run Rebot
     [Arguments]  ${options}  @{data list}
     @{data list} =  Set Variables And Get Datasources  @{data list}
-    @{options} =   Split shell  ${options}
+    @{options} =   Command line to list  ${options}
     ${result} =  Run Helper  ${INTERPRETER.rebot}  @{options}  @{data list}
     Process Output  ${OUTFILE}
     [Return]  ${result.rc}
 
 Run Rebot Without Processing Output
     [Arguments]  ${options}  @{data list}
     @{data list} =  Set Variables And Get Datasources  @{data list}
-    @{options} =   Split shell  ${options}
+    @{options} =   Command line to list  ${options}
     ${result} =  Run Helper  ${INTERPRETER.rebot}  @{options}  @{data list}
     [Return]  ${result.rc}
 
@@ -87,6 +93,7 @@ Run Helper
     ...  --log  NONE
     ...  @{arguments}
     ${result} =  Run Process  @{cmd}   stdout=${STDOUTFILE}  stderr=${STDERRFILE}
+    ...    timeout=5min    on_timeout=terminate
     Log  <a href="file://${OUTDIR}">${OUTDIR}</a>  HTML
     Log  <a href="file://${OUTFILE}">${OUTFILE}</a>  HTML
     Log  <a href="file://${STDOUTFILE}">${STDOUTFILE}</a>  HTML

diff --git a/atest/robot/cli/rebot/suite_name_doc_and_metadata.robot b/atest/robot/cli/rebot/suite_name_doc_and_metadata.robot
@@ -13,21 +13,21 @@ Default Name, Doc & Metadata
 
 Overriding Name, Doc & Metadata And Escaping
     [Documentation]    Overriding name, doc and metadata. Also testing escaping values. Tests are run together to have less Rebot runs with same input i.e. to have faster execution.
-    ${bs} =    Set Variable If    __import__('os').name == 'nt'    \\    \\\\
     ${options} =    Catenate
     ...    -N this_is_overridden_next
     ...    --name my_COOL_NameEXEX
-    ...    --doc Even_${bs}cooooler${bs}_docEXQU
+    ...    --doc Even_\\coooolerBS_docEXQU
     ...    --metadata something:New
     ...    --metadata two_parts:three_parts_here
-    ...    -M path:c:${bs}temp${bs}new.txt
+    ...    -M path:c:\\tempBSnew.txt
     ...    -M esc:STQUDOAMHAEXEX
     ...    --escape star:ST
     ...    -E quest:QU
     ...    -E dollar:DO
     ...    -E amp:AM
     ...    -E hash:HA
     ...    -E exclam:EX
+    ...    --escape bslash:BS
     Run Rebot    ${options}    ${MYINPUT}
     Check Names    ${SUITE}    my COOL Name!!
     Check Names    ${SUITE.tests[0]}    First One    my COOL Name!!.

diff --git a/atest/robot/cli/runner/suite_name_doc_and_metadata.robot b/atest/robot/cli/runner/suite_name_doc_and_metadata.robot
@@ -11,22 +11,22 @@ Default Name, Doc & Metadata
     Should Be Equal    ${SUITE.metadata['Something']}    My Value
 
 Overriding Name, Doc & Metadata And Escaping
-    ${bs} =    Set Variable If    __import__('os').name == 'nt'    \\    \\\\
     ${options} =    Catenate
     ...    -l log.html
     ...    -N this_is_overridden_next
     ...    --name my_COOL_Name.EXEX.
-    ...    --doc Even_${bs}cooooler${bs}_docEXQU
+    ...    --doc Even_\\coooolerBS_docEXQU
     ...    --metadata something:new
     ...    --metadata Two_Parts:three_part_VALUE
-    ...    -M path:c:${bs}temp${bs}new.txt
+    ...    -M path:c:\\tempBSnew.txt
     ...    -M esc:STQUDOAMHAEXEX
     ...    --escape star:ST
     ...    -E quest:QU
     ...    -E dollar:DO
     ...    -E amp:AM
     ...    -E hash:HA
     ...    -E exclam:EX
+    ...    --escape bslash:BS
     Run Tests    ${options}    ${TESTFILE}
     Check Names    ${SUITE}    my COOL Name.!!.
     Check Names    ${SUITE.tests[0]}    First One    my COOL Name.!!..

diff --git a/atest/robot/libdoc/LibDocLib.py b/atest/robot/libdoc/LibDocLib.py
@@ -1,32 +1,39 @@
 import json
 import os
 import pprint
+import shlex
 import tempfile
 from os.path import join, dirname, abspath
-from shlex import split
 from subprocess import call, STDOUT
 
 from robot.api import logger
 from robot.utils import decode_output
 
 ROBOT_SRC = join(dirname(abspath(__file__)), '..', '..', '..', 'src')
 
+
 class LibDocLib(object):
 
     def __init__(self, *command):
         self._cmd = list(command)
 
     def run_libdoc(self, args):
-        cmd = self._cmd + [a.decode('utf8') for a in split(args.encode('utf8'))]
+        cmd = self._cmd + self._split_args(args)
         cmd[-1] = cmd[-1].replace('/', os.sep)
         logger.info(' '.join(cmd))
         stdout = tempfile.TemporaryFile()
-        call(cmd, cwd=ROBOT_SRC, stdout=stdout, stderr=STDOUT, shell=os.sep=='\\')
+        call(cmd, cwd=ROBOT_SRC, stdout=stdout, stderr=STDOUT)
         stdout.seek(0)
         output = stdout.read().replace('\r\n', '\n')
         logger.info(output)
         return decode_output(output)
 
+    def _split_args(self, args):
+        lexer = shlex.shlex(args.encode('UTF-8'), posix=True)
+        lexer.escape = ''
+        lexer.whitespace_split = True
+        return [token.decode('UTF-8') for token in lexer]
+
     def get_libdoc_model_from_html(self, path):
         with open(path) as html_file:
             model_string = self._find_model(html_file)

diff --git a/atest/robot/libdoc/python_library.robot b/atest/robot/libdoc/python_library.robot
@@ -60,8 +60,8 @@ Keyword Documentation
 
 KwArgs and VarArgs
     Run Libdoc And Parse Output    Process
-    Keyword Name Should Be         6    Run Process
-    Keyword Arguments Should Be    6    command    *arguments    **configuration
+    Keyword Name Should Be         8    Run Process
+    Keyword Arguments Should Be    8    command    *arguments    **configuration
 
 Documentation set in __init__
     Run Libdoc And Parse Output    ${TESTDATADIR}/DocSetInInit.py

diff --git a/atest/robot/standard_libraries/process/commandline.robot b/atest/robot/standard_libraries/process/commandline.robot
@@ -0,0 +1,25 @@
+*** Settings ***
+Suite Setup      Run Tests    ${EMPTY}    standard_libraries/process/commandline.robot
+Resource         atest_resource.robot
+
+*** Test Cases ***
+Command line to list basics
+    Check Test Case    ${TESTNAME}
+
+Command line to list with internal quotes
+    Check Test Case    ${TESTNAME}
+
+Command line to list with unbalanced quotes
+    Check Test Case    ${TESTNAME}
+
+Command line to list with escaping
+    Check Test Case    ${TESTNAME}
+
+List to commandline basics
+    Check Test Case    ${TESTNAME}
+
+List to commandline with internal quotes
+    Check Test Case    ${TESTNAME}
+
+List to commandline with escaping
+    Check Test Case    ${TESTNAME}
diff --git a/atest/robot/testdoc/testdoc.robot b/atest/robot/testdoc/testdoc.robot
@@ -36,7 +36,7 @@ Invalid usage
 *** Keyword ***
 Run TestDoc
     [Arguments]    ${args}    ${expected rc}=0
-    ${args} =    Split shell   ${args}
+    @{args} =    Command line to list    ${args}
     ${result}=   Run Process  @{INTERPRETER.testdoc}  @{args}  ${OUTFILE}
     Should Be Equal As Numbers   ${result.rc}    ${expected rc}
     [Return]    ${result.stdout}

diff --git a/atest/testdata/standard_libraries/process/commandline.robot b/atest/testdata/standard_libraries/process/commandline.robot
@@ -0,0 +1,97 @@
+*** Settings ***
+Library          Process
+
+*** Variables ***
+${C0}            ${EMPTY}
+@{L0}            @{EMPTY}
+${C1}            hello
+@{L1}            hello
+${C2}            two args
+@{L2}            two    args
+${C3}            "one arg"
+@{L3}            one arg
+${C4}            'one arg again'
+@{L4}            one arg again
+${C5}            multiple "args passed this time" and 'it ought to work'
+@{L5}            multiple    args passed this time     and    it ought to work
+${C6}            ä äŋd "öther nön-äcïï" ŝtüff 'hërë wë hävë'
+@{L6}            ä    äŋd    öther nön-äcïï     ŝtüff    hërë wë hävë
+${C7}            \u1234 "\u2603 \U0001F4A9"
+@{L7}            \u1234    \u2603 \U0001F4A9
+${C8}            c:\\temp "c:\\program files"
+@{L8}            c:\\temp    c:\\program files
+${C9}            "" ''
+@{L9}            ${EMPTY}    ${EMPTY}
+${BASICS}        10
+
+*** Test Cases ***
+Command line to list basics
+    [Template]    Command line to list should succeed
+    :FOR    ${i}    IN RANGE    ${BASICS}
+    \    ${C${i}}    @{L${i}}
+    "justone"    justone
+
+Command line to list with internal quotes
+    [Template]    Command line to list should succeed
+    "inter'nal quotes"               inter'nal quotes
+    'can be "surrounded"'            can be "surrounded"
+    "with ''other'' quotes" '"""'    with ''other'' quotes    """
+
+Command line to list with unbalanced quotes
+    [Template]    Command line to list should fail
+    "oo
+    "
+    '
+    "foo"bar"
+    foo'bar
+
+Command line to list with escaping
+    [Template]    Command line to list should succeed
+    c:\\temp                          c:temp                       escaping=True
+    c:\\\\temp                        c:\\temp                     escaping=True
+    "c:\\temp"                        c:\\temp                     escaping=True
+    'c:\\temp'                        c:\\temp                     escaping=True
+    C:\\\\Program\\ Files\\\\Blaah    C:\\Program Files\\Blaah     escaping=True
+    "C:\\Program Files\\Blaah"        C:\\Program Files\\Blaah     escaping=True
+    'C:\\Program Files\\Blaah'        C:\\Program Files\\Blaah     escaping=True
+    "internal \\"quotes\\"\\""        internal "quotes""           escaping=True
+    'internal \\"quotes\\"\\"'        internal \\"quotes\\"\\"     escaping=True
+    'internal \\'quotes\\'\\''        internal 'quotes''           escaping=True
+    "internal \\'quotes\\'\\'"        internal \\'quotes\\'\\'     escaping=True
+    \\\\\\"                           \\"                          escaping=True
+    \\\\\\\\\\"                       \\\\"                        escaping=True
+    \\\\\\"\\\\                       \\"\\                        escaping=True
+    "\\\\\\"\\\\"                     \\"\\                        escaping=True
+
+List to commandline basics
+    [Template]    List to command line should succeed
+    :FOR    ${i}    IN RANGE    ${BASICS}
+    \    ${C${i}.replace("'", '"')}    @{L${i}}
+
+List to commandline with internal quotes
+    [Template]    List to command line should succeed
+    "internal \\"double' quotes"       internal "double' quotes
+    "will be \\"'escaped'\\"" ' \\"    will be "'escaped'"    '    "
+
+List to commandline with escaping
+    [Template]    List to command line should succeed
+    c:\\temp                          c:\\temp
+    "C:\\Program Files\\Blaah"        C:\\Program Files\\Blaah
+    \\\\\\"                           \\"
+    \\\\\\\\\\"                       \\\\"
+
+*** Keywords ***
+Command line to list should succeed
+    [Arguments]    ${input}    @{expected}    &{config}
+    ${result} =    Command line to list    ${input}    &{config}
+    Should be equal    ${result}    ${expected}
+
+Command line to list should fail
+    [Arguments]    ${input}    ${error}=No closing quotation
+    Run keyword and expect error    ValueError: Parsing '${input}' failed: ${error}
+    ...    Command line to list    ${input}
+
+List to command line should succeed
+    [Arguments]    ${expected}    @{input}
+    ${result} =    List to command line    ${input}
+    Should be equal    ${result}    ${expected}
diff --git a/src/robot/libraries/Process.py b/src/robot/libraries/Process.py
@@ -16,10 +16,11 @@
 import os
 import subprocess
 import time
+import shlex
 import signal as signal_module
 
 from robot.utils import (ConnectionCache, abspath, encode_to_system,
-                         decode_output, is_truthy, secs_to_timestr,
+                         decode_output, is_falsy, is_truthy, secs_to_timestr,
                          timestr_to_secs, IRONPYTHON, JYTHON)
 from robot.version import get_version
 from robot.api import logger
@@ -725,6 +726,21 @@ def _process_is_stopped(self, process, timeout):
             time.sleep(min(0.1, timeout))
         return stopped()
 
+    def command_line_to_list(self, args, escaping=False):
+        lexer = shlex.shlex(args.encode('UTF-8'), posix=True)
+        if is_falsy(escaping):
+            lexer.escape = ''
+        lexer.escapedquotes = '"\''
+        lexer.commenters = ''
+        lexer.whitespace_split = True
+        try:
+            return [token.decode('UTF-8') for token in lexer]
+        except ValueError as err:
+            raise ValueError("Parsing '%s' failed: %s" % (args, err))
+
+    def list_to_command_line(self, args):
+        return subprocess.list2cmdline(args)
+
 
 class ExecutionResult(object):