Merge lp:~cr3/checkbox/warning_prompt into lp:checkbox

[Marc Tardif]

Merged warning_prompt plugin for whitelist validation warnings by cr3.

Added warning_prompt plugin to prevent the reactor from stopping on some warnings when run non-interactively.

[Sylvain Pineau (sylvain-pineau)]

There's a lot of pep8 fixes but i'm happy with the skeleton of show_warning() in base checkbox/user_interface.py.

Thanks

review: Approve

[Marc Tardif (cr3)]

Thanks, you can now work on having a nice user interface for Qt specifically at your leisure by overriding the show_warning method in the qt interface class.

=== modified file 'checkbox/user_interface.py'
--- checkbox/user_interface.py	2012-07-23 20:50:14 +0000
+++ checkbox/user_interface.py	2012-10-17 19:06:24 +0000
@@ -29,10 +29,20 @@
 
 from checkbox.contrib.REThread import REThread
 
-from checkbox.lib.environ import add_variable, get_variable, remove_variable
+from checkbox.lib.environ import (
+    add_variable,
+    get_variable,
+    remove_variable,
+    )
 
-from checkbox.job import (FAIL, PASS, UNINITIATED,
-    UNRESOLVED, UNSUPPORTED, UNTESTED)
+from checkbox.job import (
+    FAIL,
+    PASS,
+    UNINITIATED,
+    UNRESOLVED,
+    UNSUPPORTED,
+    UNTESTED,
+    )
 from checkbox.reactor import StopAllException
 
 
@@ -83,13 +93,22 @@
         logging.info(text)
         return default
 
-    def show_error(self, primary_text,
-                   secondary_text=None, detailed_text=None):
+    def show_error(
+        self, primary_text, secondary_text=None, detailed_text=None):
         text = filter(None, [primary_text, secondary_text, detailed_text])
         text = '\n'.join(text)
         logging.error(text)
         raise StopAllException("Error: %s" % text)
 
+    def show_warning(
+        self, primary_text, secondary_text=None, detailed_text=None):
+        try:
+            self.show_error(primary_text, secondary_text, detailed_text)
+        except StopAllException:
+            # The only difference with show_error for now is that warnings
+            # don't stop the reactor.
+            pass
+
     def show_progress(self, message, function, *args, **kwargs):
         self.show_progress_start(message)
 
@@ -138,8 +157,8 @@
 
         Display an error dialog if everything fails."""
 
-        # If we are called through sudo, determine the real user id and run the
-        # browser with it to get the user's web browser settings.
+        # If we are called through sudo, determine the real user id and
+        # run the browser with it to get the user's web browser settings.
         try:
             uid = int(get_variable("SUDO_UID"))
             gid = int(get_variable("SUDO_GID"))
@@ -149,67 +168,78 @@
             gid = None
             sudo_prefix = []
 
-        # figure out appropriate web browser
-        try:
-            # if ksmserver is running, try kfmclient
-            try:
-                if os.getenv("DISPLAY") and \
-                        subprocess.call(["pgrep", "-x", "-u", str(uid), "ksmserver"],
-                                stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0:
-                    subprocess.Popen(sudo_prefix + ["kfmclient", "openURL", url])
-                    return
-            except OSError:
-                pass
-
-            # if gnome-session is running, try gnome-open; special-case firefox
-            # (and more generally, mozilla browsers) and epiphany to open a new window
-            # with respectively -new-window and --new-window; special-case chromium-browser
-            # to allow file:// URLs as needed by the checkbox report.
-            try:
-                if os.getenv("DISPLAY") and \
-                        subprocess.call(["pgrep", "-x", "-u", str(uid), "gnome-panel|gconfd-2"],
-                                stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0:
-                    from gi.repository import Gio
-
-                    preferred_xml_app = Gio.app_info_get_default_for_type("application/xml",False)
-                    if preferred_xml_app:
-                        preferred_browser = preferred_xml_app.get_executable()
-                        browser = re.match("((firefox|seamonkey|flock)[^\s]*)", preferred_browser)
-                        if browser:
-                            subprocess.Popen(sudo_prefix + [browser.group(0), "-new-window", url])
-                            return
-
-                        browser = re.match("(epiphany[^\s]*)", preferred_browser)
-                        if browser:
-                            subprocess.Popen(sudo_prefix + [browser.group(0), "--new-window", url])
-                            return
-                        
-                        browser = re.match("(chromium-browser[^\s]*)", preferred_browser)
-                        if browser:
-                            subprocess.Popen(sudo_prefix + [browser.group(0), "--allow-file-access-from-files", url])
-                            return
-
-                        subprocess.Popen(sudo_prefix + [preferred_browser % url], shell=True)
-                        return
-
-                    subprocess.Popen(sudo_prefix + ["gnome-open", url])
-                    return
-            except OSError:
-                pass
-
-            # fall back to webbrowser
-            if uid and gid:
-                os.setgroups([gid])
-                os.setgid(gid)
-                os.setuid(uid)
-                remove_variable("SUDO_USER")
-                add_variable("HOME", pwd.getpwuid(uid).pw_dir)
-
-            webbrowser.open(url, new=True, autoraise=True)
-            return
-
-        except Exception as e:
-            pass
+        # if ksmserver is running, try kfmclient
+        try:
+            if (os.getenv("DISPLAY") and
+                    subprocess.call(
+                        ["pgrep", "-x", "-u", str(uid), "ksmserver"],
+                        stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0):
+                subprocess.Popen(sudo_prefix + ["kfmclient", "openURL", url])
+                return
+        except OSError:
+            pass
+
+        # if gnome-session is running, try gnome-open; special-case
+        # firefox (and more generally, mozilla browsers) and epiphany
+        # to open a new window with respectively -new-window and
+        # --new-window; special-case chromium-browser to allow file://
+        # URLs as needed by the checkbox report.
+        try:
+            if (os.getenv("DISPLAY") and
+                subprocess.call(
+                    ["pgrep", "-x", "-u", str(uid), "gnome-panel|gconfd-2"],
+                    stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0):
+                from gi.repository import Gio
+
+                preferred_xml_app = Gio.app_info_get_default_for_type(
+                    "application/xml", False)
+                if preferred_xml_app:
+                    preferred_browser = preferred_xml_app.get_executable()
+                    browser = re.match(
+                        "((firefox|seamonkey|flock)[^\s]*)",
+                        preferred_browser)
+                    if browser:
+                        subprocess.Popen(
+                            sudo_prefix +
+                            [browser.group(0), "-new-window", url])
+                        return
+
+                    browser = re.match("(epiphany[^\s]*)", preferred_browser)
+                    if browser:
+                        subprocess.Popen(
+                            sudo_prefix +
+                            [browser.group(0), "--new-window", url])
+                        return
+
+                    browser = re.match(
+                        "(chromium-browser[^\s]*)", preferred_browser)
+                    if browser:
+                        subprocess.Popen(
+                            sudo_prefix +
+                            [browser.group(0),
+                                "--allow-file-access-from-files", url])
+                        return
+
+                    subprocess.Popen(
+                        sudo_prefix +
+                        [preferred_browser % url], shell=True)
+                    return
+
+                subprocess.Popen(sudo_prefix + ["gnome-open", url])
+                return
+        except OSError:
+            pass
+
+        # fall back to webbrowser
+        if uid and gid:
+            os.setgroups([gid])
+            os.setgid(gid)
+            os.setuid(uid)
+            remove_variable("SUDO_USER")
+            add_variable("HOME", pwd.getpwuid(uid).pw_dir)
+
+        webbrowser.open(url, new=True, autoraise=True)
+        return
 
     def show_report(self, text, results):
         """
@@ -225,5 +255,3 @@
         about each job.
         """
         pass
-        
-
 
=== modified file 'plugins/jobs_info.py'
--- plugins/jobs_info.py	2012-06-26 14:07:30 +0000
+++ plugins/jobs_info.py	2012-10-17 19:06:24 +0000
@@ -16,18 +16,29 @@
 # You should have received a copy of the GNU General Public License
 # along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
 #
-import os, sys, re
+import os
+import re
+import sys
+
 import difflib
 import gettext
 import logging
 
 from collections import defaultdict
+from gettext import gettext as _
 
 from checkbox.lib.resolver import Resolver
 
 from checkbox.arguments import coerce_arguments
-from checkbox.properties import Float, Int, List, Map, Path, String
 from checkbox.plugin import Plugin
+from checkbox.properties import (
+    Float,
+    Int,
+    List,
+    Map,
+    Path,
+    String,
+    )
 
 
 job_schema = Map({
@@ -76,14 +87,17 @@
     def register(self, manager):
         super(JobsInfo, self).register(manager)
 
-        self.whitelist_patterns = self.get_patterns(self.whitelist, self.whitelist_file)
-        self.blacklist_patterns = self.get_patterns(self.blacklist, self.blacklist_file)
+        self.whitelist_patterns = self.get_patterns(
+            self.whitelist, self.whitelist_file)
+        self.blacklist_patterns = self.get_patterns(
+            self.blacklist, self.blacklist_file)
         self.selected_jobs = defaultdict(list)
 
         self._manager.reactor.call_on("prompt-begin", self.prompt_begin)
         self._manager.reactor.call_on("gather", self.gather)
         if logging.getLogger().getEffectiveLevel() <= logging.DEBUG:
-            self._manager.reactor.call_on("prompt-gather", self.post_gather, 90)
+            self._manager.reactor.call_on(
+                "prompt-gather", self.post_gather, 90)
         self._manager.reactor.call_on("report-job", self.report_job, -100)
 
     def prompt_begin(self, interface):
@@ -92,7 +106,8 @@
         to display errors
         """
         self.interface = interface
-        self.unused_patterns = self.whitelist_patterns + self.blacklist_patterns
+        self.unused_patterns = (
+            self.whitelist_patterns + self.blacklist_patterns)
 
     def check_ordered_messages(self, messages):
         """Return whether the list of messages are ordered or not."""
@@ -113,8 +128,8 @@
             try:
                 file = open(filename)
             except IOError as e:
-                error_message=(gettext.gettext("Failed to open file '%s': %s") %
-                        (filename, e.strerror))
+                error_message = (_("Failed to open file '%s': %s")
+                    % (filename, e.strerror))
                 logging.critical(error_message)
                 sys.stderr.write("%s\n" % error_message)
                 sys.exit(os.EX_NOINPUT)
@@ -148,7 +163,9 @@
         def report_message(message):
             if self.whitelist_patterns:
                 name = message["name"]
-                if not [name for p in self.whitelist_patterns if p.match(name)]:
+                names = [name for p in self.whitelist_patterns
+                    if p.match(name)]
+                if not names:
                     return
 
             messages.append(message)
@@ -156,7 +173,8 @@
         # Set domain and message event handler
         old_domain = gettext.textdomain()
         gettext.textdomain(self.domain)
-        event_id = self._manager.reactor.call_on("report-message", report_message, 100)
+        event_id = self._manager.reactor.call_on(
+            "report-message", report_message, 100)
 
         for directory in self.directories:
             self._manager.reactor.fire("message-directory", directory)
@@ -182,7 +200,8 @@
             messages = sorted(messages, key=key_function)
 
         if not self.check_ordered_messages(messages):
-            old_message_names = [message["name"] + "\n" for message in messages]
+            old_message_names = [
+                message["name"] + "\n" for message in messages]
             resolver = Resolver(key_func=lambda m: m["name"])
             for message in messages:
                 resolver.add(
@@ -192,7 +211,8 @@
             # Check if messages are already topologically ordered
             if (self.whitelist_patterns and
                 logging.getLogger().getEffectiveLevel() <= logging.DEBUG):
-                new_message_names = [message["name"] + "\n" for message in messages]
+                new_message_names = [
+                    message["name"] + "\n" for message in messages]
                 detailed_text = "".join(
                     difflib.unified_diff(
                         old_message_names,
@@ -200,7 +220,7 @@
                         "old whitelist",
                         "new whitelist"))
                 self._manager.reactor.fire(
-                        "prompt-error",
+                        "prompt-warning",
                         self.interface,
                         "Whitelist not topologically ordered",
                         "Jobs will be reordered to fix broken dependencies",
@@ -231,7 +251,7 @@
                  'Please make sure that the patterns you used are up-to-date\n'
                  .format('\n'.join(['- {0}'.format(tc)
                                     for tc in orphan_test_cases])))
-            self._manager.reactor.fire('prompt-error', self.interface,
+            self._manager.reactor.fire('prompt-warning', self.interface,
                                        'Orphan test cases detected',
                                        "Some test cases aren't included "
                                        'in any test suite',
@@ -244,7 +264,7 @@
                  "Please make sure that the patterns you used are up-to-date\n"
                  .format('\n'.join(['- {0}'.format(p.pattern[1:-1])
                                         for p in self.unused_patterns])))
-            self._manager.reactor.fire('prompt-error', self.interface,
+            self._manager.reactor.fire('prompt-warning', self.interface,
                                        'Unused patterns',
                                        'Please make sure that the patterns '
                                        'you used are up-to-date',
 
=== added file 'plugins/warning_prompt.py'
--- plugins/warning_prompt.py	1970-01-01 00:00:00 +0000
+++ plugins/warning_prompt.py	2012-10-17 19:06:24 +0000
@@ -0,0 +1,35 @@
+#
+# This file is part of Checkbox.
+#
+# Copyright 2012 Canonical Ltd.
+#
+# Checkbox is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Checkbox is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
+#
+from checkbox.plugin import Plugin
+
+
+class WarningPrompt(Plugin):
+
+    def register(self, manager):
+        super(WarningPrompt, self).register(manager)
+
+        self._manager.reactor.call_on("prompt-warning",
+            self.prompt_warning)
+
+    def prompt_warning(self, interface,
+                     primary_text, secondary_text=None, detailed_text=None):
+        interface.show_warning(primary_text, secondary_text, detailed_text)
+
+
+factory = WarningPrompt