Revision: 3dd82371e6f9
Branch:   default
Author:   Pekka Klärck
Date:     Wed Jan 29 07:34:35 2014 UTC
Log:      Process: Safer implementation to always close custom streams.

Also test enhancements.

Update issue 1640
Changed the implementation so that now custom streams are always closed after process has completed. This guarantees file handles are not leaked if stdout/stderr are not read. Unifying calling process.wait() was also a win.
http://code.google.com/p/robotframework/source/detail?r=3dd82371e6f9

Modified:
 /atest/robot/standard_libraries/process/wait_for_process.txt
 /atest/testdata/standard_libraries/process/resource.txt
 /src/robot/libraries/Process.py

=======================================
--- /atest/robot/standard_libraries/process/wait_for_process.txt Tue Jan 28 09:20:00 2014 UTC +++ /atest/robot/standard_libraries/process/wait_for_process.txt Wed Jan 29 07:34:35 2014 UTC
@@ -21,9 +21,11 @@
Check Log Message ${tc.kws[2].msgs[0]} Waiting for process to complete. Check Log Message ${tc.kws[2].msgs[1]} Process did not complete in 1 second. Check Log Message ${tc.kws[2].msgs[2]} Gracefully terminating process.
+    Check Log Message    ${tc.kws[2].msgs[3]}    Process completed.

 Wait For Process Kill On Timeout
     ${tc} =    Check Test Case    ${TESTNAME}
Check Log Message ${tc.kws[2].msgs[0]} Waiting for process to complete. Check Log Message ${tc.kws[2].msgs[1]} Process did not complete in 1 second. Check Log Message ${tc.kws[2].msgs[2]} Forcefully killing process.
+    Check Log Message    ${tc.kws[2].msgs[3]}    Process completed.
=======================================
--- /atest/testdata/standard_libraries/process/resource.txt Tue Jan 28 22:16:40 2014 UTC +++ /atest/testdata/standard_libraries/process/resource.txt Wed Jan 29 07:34:35 2014 UTC
@@ -26,7 +26,8 @@
     ...    ${stdout_path}=    ${stderr_path}=
     Should Be Equal    ${result.stdout}    ${stdout}    stdout:
     Should Be Equal    ${result.stderr}    ${stderr}    stderr
- Result should match ${result} * * ${rc} ${stdout_path} ${stderr_path}
+    Result should match    ${result}    ${stdout}    ${stderr}    ${rc}
+    ...    ${stdout_path}    ${stderr_path}

 Result should match
     [Arguments]    ${result}    ${stdout}=    ${stderr}=    ${rc}=0
@@ -34,12 +35,19 @@
     Should Match    ${result.stdout}    ${stdout}    stdout
     Should Match    ${result.stderr}    ${stderr}    stderr
     Should Be Equal As Integers    ${result.rc}    ${rc}    rc
-    ${stdout_path} =    Run Keyword If    "${stdout_path}"
-    ...    Normalize Path    ${stdout_path}
-    ${stderr_path} =    Run Keyword If    "${stderr_path}"
-    ...    Normalize Path    ${stderr_path}
+ ${stdout_path} = Custom stream should contain ${stdout_path} ${result.stdout} + ${stderr_path} = Custom stream should contain ${stderr_path} ${result.stderr} Should Be Equal ${result.stdout_path} ${stdout_path} stdout_path Should Be Equal ${result.stderr_path} ${stderr_path} stderr_path
+
+Custom stream should contain
+    [Arguments]    ${path}    ${expected}
+    Return From Keyword If    not "${path}"    ${NONE}
+    ${path} =    Normalize Path    ${path}
+ ${encoding} = Evaluate robot.utils.encoding.OUTPUT_ENCODING robot
+    ${content} =    Get File    ${path}    encoding=${encoding}
+    Should Be Equal    ${content.rstrip()}    ${expected}
+    [Return]    ${path}

 Start Python Process
[Arguments] ${command} ${alias}=${NONE} ${stdout}=${NONE} ${stderr}=${NONE}
=======================================
--- /src/robot/libraries/Process.py     Tue Jan 28 23:10:45 2014 UTC
+++ /src/robot/libraries/Process.py     Wed Jan 29 07:34:35 2014 UTC
@@ -426,7 +426,6 @@
         `timeout` and `on_timeout` are new in Robot Framework 2.8.2.
         """
         process = self._processes[handle]
-        result = self._results[process]
         logger.info('Waiting for process to complete.')
         if timeout:
             timeout = timestr_to_secs(timeout)
@@ -434,9 +433,7 @@
                 logger.info('Process did not complete in %s.'
                             % secs_to_timestr(timeout))
return self._manage_process_timeout(handle, on_timeout.lower())
-        result.rc = process.wait() or 0
-        logger.info('Process completed.')
-        return result
+        return self._wait(process)

     def _manage_process_timeout(self, handle, on_timeout):
         if on_timeout == 'terminate':
@@ -447,6 +444,13 @@
             logger.info('Leaving process intact.')
             return None

+    def _wait(self, process):
+        result = self._results[process]
+        result.rc = process.wait() or 0
+        result.close_custom_streams()
+        logger.info('Process completed.')
+        return result
+
     def terminate_process(self, handle=None, kill=False):
         """Stops the process gracefully or forcefully.

@@ -484,7 +488,6 @@
returning the result object are new features in Robot Framework 2.8.2.
         """
         process = self._processes[handle]
-        result = self._results[process]
         if not hasattr(process, 'terminate'):
             raise RuntimeError('Terminating processes is not supported '
                                'by this Python version.')
@@ -495,8 +498,7 @@
             if not self._process_is_stopped(process, self.KILL_TIMEOUT):
                 raise
             logger.debug('Ignored OSError because process was stopped.')
-        result.rc = process.wait() or 0
-        return result
+        return self._wait(process)

     def _kill(self, process):
         logger.info('Forcefully killing process.')
@@ -687,13 +689,13 @@

     def __init__(self, process, stdout, stderr, rc=None):
         self._process = process
-        self._stdout_stream = stdout
-        self._stderr_stream = stderr
         self.stdout_path = self._get_path(stdout)
         self.stderr_path = self._get_path(stderr)
         self.rc = rc
         self._stdout = None
         self._stderr = None
+        self._custom_streams = [stream for stream in (stdout, stderr)
+                                if self._is_custom_stream(stream)]

     def _get_path(self, stream):
         return stream.name if self._is_custom_stream(stream) else None
@@ -704,7 +706,6 @@
     @property
     def stdout(self):
         if self._stdout is None:
-            self._close_stream(self._stdout_stream)
             self._stdout = self._read_stream(self.stdout_path,
                                              self._process.stdout)
         return self._stdout
@@ -712,15 +713,15 @@
     @property
     def stderr(self):
         if self._stderr is None:
-            self._close_stream(self._stderr_stream)
             self._stderr = self._read_stream(self.stderr_path,
                                              self._process.stderr)
         return self._stderr

-    def _close_stream(self, stream):
-        if self._is_custom_stream(stream) and not stream.closed:
-            stream.flush()
-            stream.close()
+    def close_custom_streams(self):
+        for stream in self._custom_streams:
+            if not stream.closed:
+                stream.flush()
+                stream.close()

     def _read_stream(self, stream_path, stream):
         if stream_path:

--

--- You received this message because you are subscribed to the Google Groups "robotframework-commit" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to robotframework-commit+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to