Changeset 159690 in webkit

Timestamp:

Nov 22, 2013 8:26:27 AM (10 years ago)

Author:

commit-queue@webkit.org

Message:

EWS creates 0 byte sized log files
​https://bugs.webkit.org/show_bug.cgi?id=107606

Patch by László Langó <​lango@inf.u-szeged.hu> on 2013-11-22
Reviewed by Ryosuke Niwa.

There was a modification in r138264, that tried to make less log,
because some of the messeges were duplicated. After this the EWS
created the log file (with the same name as the bugID) but doesn't
write anything into it, even if there were errors during the build.
From now only creates the log file only if there is some error.

• Scripts/webkitpy/tool/bot/queueengine.py:

(QueueEngine.run): If the build and tests pass, there is no ScriptError raised,
there is nothing to log. Open the log file only if a ScriptError was raised to
avoid to make empty log files for bugs.
(QueueEngine._open_work_log): Does not need to tee STDOUT to log file anymore,
because of changes in r138264. Teeing is used for locally testing purposes and
this feature is not used anymore.
(QueueEngine._ensure_work_log_closed): Close the logfile. We don't use output
teeing anymore. It is a necessary change because of QueueEngine._open_work_log
change.

• Scripts/webkitpy/tool/commands/earlywarningsystem.py:

(AbstractEarlyWarningSystem.review_patch): Raise again the captured ScriptError
to be able to handle it in QueueEngine.run. Without this change, the existing
exception handler never run (the process_work_item method never raise ScriptError)
We can get the error message only from the ScriptError at this point.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/tool/bot/queueengine.py (modified) (3 diffs)
• Scripts/webkitpy/tool/commands/earlywarningsystem.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2013-11-22  László Langó  <lango@inf.u-szeged.hu>
+
+        EWS creates 0 byte sized log files
+        https://bugs.webkit.org/show_bug.cgi?id=107606
+
+        Reviewed by Ryosuke Niwa.
+
+        There was a modification in r138264, that tried to make less log, 
+        because some of the messeges were duplicated. After this the EWS 
+        created the log file (with the same name as the bugID) but doesn't 
+        write anything into it, even if there were errors during the build. 
+        From now only creates the log file only if there is some error.
+
+        * Scripts/webkitpy/tool/bot/queueengine.py:
+        (QueueEngine.run): If the build and tests pass, there is no ScriptError raised,
+        there is nothing to log. Open the log file only if a ScriptError was raised to
+        avoid to make empty log files for bugs.
+        (QueueEngine._open_work_log): Does not need to tee STDOUT to log file anymore,
+        because of changes in r138264. Teeing is used for locally testing purposes and
+        this feature is not used anymore.
+        (QueueEngine._ensure_work_log_closed): Close the logfile. We don't use output 
+        teeing anymore. It is a necessary change because of QueueEngine._open_work_log
+        change.
+        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+        (AbstractEarlyWarningSystem.review_patch): Raise again the captured ScriptError
+        to be able to handle it in QueueEngine.run. Without this change, the existing
+        exception handler never run (the process_work_item method never raise ScriptError)
+        We can get the error message only from the ScriptError at this point.
+
 2013-11-21  Peter Molnar  <pmolnar.u-szeged@partner.samsung.com>
 

trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py

@@ -98 +98 @@
                     continue
 
-                # FIXME: Work logs should not depend on bug_id specificaly.
-                #        This looks fixed, no?
-                self._open_work_log(work_item)
                 try:
                     if not self._delegate.process_work_item(work_item):
@@ -106 +103 @@
                         continue
                 except ScriptError, e:
+                    self._open_work_log(work_item)
+                    self._work_log.write(e.message_with_output())
                     # Use a special exit code to indicate that the error was already
                     # handled in the child process and we should just keep looping.
@@ -140 +139 @@
         if not work_item_log_path:
             return
-        self._work_log = self._output_tee.add_log(work_item_log_path)
+        self._work_log = self._output_tee._open_log_file(work_item_log_path)
 
     def _ensure_work_log_closed(self):
         # If we still have a bug log open, close it.
         if self._work_log:
-            self._output_tee.remove_log(self._work_log)
+            self._work_log.close()
             self._work_log = None
 

trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py

@@ -93 +93 @@
                 self._upload_results_archive_for_patch(patch, results_archive)
             self._did_fail(patch)
-            # FIXME: We're supposed to be able to raise e again here and have
-            # one of our base classes mark the patch as fail, but there seems
-            # to be an issue with the exit_code.
-            return False
+            raise e
 
     # EarlyWarningSystemDelegate methods