toaster: Improve display of errors and Toaster exceptions
cover toaster: Improve display of errors and Toaster exceptions
Hi,
Fixes recording of error messages so that CRITICAL errors are properly logged and presented to the user.
Modifies how Toaster exceptions are displayed, so they are treated the same way as other classes of ERROR.
The following changes since commit a8e82d93bfa8feb722912d03ee26bbad7e6fe9f7:
toaster: Modify "New build" button behaviour for cli builds project (2015-10-15 14:54:27 +0300)
are available in the git repository at:
git://git.yoctoproject.org/poky-contrib ed/toaster/submit/elliot/toaster/error_display-8320
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/toaster/submit/elliot/toaster/error_display-8320
Elliot Smith (4):
toaster: Record critical errors
toaster: Remove Toaster exceptions section of build dashboard
toaster: Guard against builds with no targets
toaster: Add tests for error message display on the build dashboard
lib/bb/ui/buildinfohelper.py | 5 +- lib/bb/ui/toasterui.py | 2 + lib/toaster/orm/models.py | 24 +++--- .../toastergui/templates/builddashboard.html | 47 ++--------- lib/toaster/toastergui/tests.py | 91 +++++++++++++++++++++- 5 files changed, 116 insertions(+), 53 deletions(-)
--
Regards,
Ed
PATCH 1/4 toaster: Record critical errors
From: Elliot Smith <[email protected]>
Critical errors (where a build failed for reasons of misconfiguration, such as a machine being specified which is not in a project's layers) were being ignored (only log records up to ERROR level were being logged to Toaster's db). This meant that the build would fail but would not correctly report why.
Add support for CRITICAL error levels to the LogMessage model, include errors at this level in the errors property for a build, and show errors at this level in the build dashboard.
[YOCTO #8320]
--- lib/bb/ui/buildinfohelper.py | 5 ++++- lib/bb/ui/toasterui.py | 2 ++ lib/toaster/orm/models.py | 20 +++++++++++++------- lib/toaster/toastergui/templates/builddashboard.html | 11 ++++++----- 4 files changed, 25 insertions(+), 13 deletions(-)
lib/bb/ui/buildinfohelper.py +4 -1
@@ -1394,7 +1394,9 @@ class BuildInfoHelper(object): log_information = {} log_information['build'] = self.internal_state['build'] - if event.levelno == formatter.ERROR: + if event.levelno == formatter.CRITICAL: + log_information['level'] = LogMessage.CRITICAL + elif event.levelno == formatter.ERROR: log_information['level'] = LogMessage.ERROR elif event.levelno == formatter.WARNING: log_information['level'] = LogMessage.WARNING
@@ -1407,6 +1409,7 @@ class BuildInfoHelper(object): log_information['pathname'] = event.pathname log_information['lineno'] = event.lineno logger.info("Logging error 2: %s", log_information) + self.orm_wrapper.create_logmessage(log_information) def close(self, errorcode):
lib/bb/ui/toasterui.py +2 -0
@@ -145,10 +145,12 @@ def main(server, eventHandler, params ): event.levelno = formatter.ERROR buildinfohelper.store_log_event(event) + if event.levelno >= formatter.ERROR: errors = errors + 1 elif event.levelno == formatter.WARNING: warnings = warnings + 1 + # For "normal" logging conditions, don't show note logs from tasks # but do show them if the user has changed the default log level to # include verbose/debug messages
lib/toaster/orm/models.py +13 -7
@@ -353,7 +353,9 @@ class Build(models.Model): @property def errors(self): - return (self.logmessage_set.filter(level=LogMessage.ERROR)|self.logmessage_set.filter(level=LogMessage.EXCEPTION)) + return (self.logmessage_set.filter(level=LogMessage.ERROR) | + self.logmessage_set.filter(level=LogMessage.EXCEPTION) | + self.logmessage_set.filter(level=LogMessage.CRITICAL)) @property def warnings(self):
@@ -1285,16 +1287,20 @@ class LogMessage(models.Model): INFO = 0 WARNING = 1 ERROR = 2 - - LOG_LEVEL = ( (INFO, "info"), - (WARNING, "warn"), - (ERROR, "error"), - (EXCEPTION, "toaster exception")) + CRITICAL = 3 + + LOG_LEVEL = ( + (INFO, "info"), + (WARNING, "warn"), + (ERROR, "error"), + (CRITICAL, "critical"), + (EXCEPTION, "toaster exception") + ) build = models.ForeignKey(Build) task = models.ForeignKey(Task, blank = True, null=True) level = models.IntegerField(choices=LOG_LEVEL, default=INFO) - message=models.CharField(max_length=240) + message = models.CharField(max_length=240) pathname = models.FilePathField(max_length=255, blank=True) lineno = models.IntegerField(null=True)
lib/toaster/toastergui/templates/builddashboard.html +6 -5
@@ -69,11 +69,12 @@ <div class="accordion-body collapse in" id="collapse-errors"> <div class="accordion-inner"> <div class="span10"> - {% for error in logmessages %}{% if error.level == 2 %} - <div class="alert alert-error"> - <pre>{{error.message}}</pre> - </div> - {% endif %} + {% for error in logmessages %} + {% if error.level == 2 or error.level == 3 %} + <div class="alert alert-error"> + <pre>{{error.message}}</pre> + </div> + {% endif %} {% endfor %} </div> </div>
Signature · 1 line
2.1.4
PATCH 2/4 toaster: Remove Toaster exceptions section of build dashboard
In reply to Ed BartoshFrom: Elliot Smith <[email protected]>
Fatal build errors were displayed as exceptions, and highlighted with less severity than they deserved.
Roll back to treating Toaster exceptions as errors by removing the toaster_exceptions member on Build objects and displaying EXCEPTION events in the errors section on the dashboard.
[YOCTO #8320]
--- lib/toaster/orm/models.py | 4 -- .../toastergui/templates/builddashboard.html | 45 ++-------------------- 2 files changed, 4 insertions(+), 45 deletions(-)
lib/toaster/orm/models.py +0 -4
@@ -348,10 +348,6 @@ class Build(models.Model): return Build.BUILD_OUTCOME[int(self.outcome)][1] @property - def toaster_exceptions(self): - return self.logmessage_set.filter(level=LogMessage.EXCEPTION) - - @property def errors(self): return (self.logmessage_set.filter(level=LogMessage.ERROR) | self.logmessage_set.filter(level=LogMessage.EXCEPTION) |
lib/toaster/toastergui/templates/builddashboard.html +4 -41
63 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 3/4 toaster: Guard against builds with no targets
In reply to Ed BartoshFrom: Elliot Smith <[email protected]>
Although this probably can't happen in practice (builds without targets), this constraint is not currently enforced in the data model.
In the unlikely event that a build has no target (e.g. in test cases), this causes a template rendering error.
Rather than rework the data model, add a guard to the template to prevent it from rendering the target name if there are no targets associated with the build.
--- lib/toaster/toastergui/templates/builddashboard.html | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
lib/toaster/toastergui/templates/builddashboard.html +7 -2
@@ -3,7 +3,12 @@ {% load projecttags %} {% block parentbreadcrumb %} -{{build.get_sorted_target_list.0.target}} {%if build.target_set.all.count > 1%}(+ {{build.target_set.all.count|add:"-1"}}){%endif%} {{build.machine}} ({{build.completed_on|date:"d/m/y H:i"}}) +{% if build.get_sorted_target_list.count > 0 %} + {{build.get_sorted_target_list.0.target}} + +{% endif %} + +{%if build.target_set.all.count > 1%}(+ {{build.target_set.all.count|add:"-1"}}){%endif%} {{build.machine}} ({{build.completed_on|date:"d/m/y H:i"}}) {% endblock %} {% block buildinfomain %}
@@ -62,7 +67,7 @@ <div class="accordion-inner"> <div class="span10"> {% for error in build.errors %} - <div class="alert alert-error"> + <div class="alert alert-error" data-error="{{ error.id }}"> <pre>{{error.message}}</pre> </div> {% endfor %}
Signature · 1 line
2.1.4
PATCH 4/4 toaster: Add tests for error message display on the build dashboard
In reply to Ed BartoshFrom: Elliot Smith <[email protected]>
Add tests to ensure that log messages with CRITICAL level and with EXCEPTION level are shown in the main errors display on the build dashboard.
[YOCTO #8320]
--- lib/toaster/toastergui/tests.py | 91 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-)
lib/toaster/toastergui/tests.py +89 -2
@@ -26,15 +26,15 @@ from django.test.client import RequestFactory from django.core.urlresolvers import reverse from django.utils import timezone -from orm.models import Project, Release, BitbakeVersion, Package +from orm.models import Project, Release, BitbakeVersion, Package, LogMessage from orm.models import ReleaseLayerSourcePriority, LayerSource, Layer, Build from orm.models import Layer_Version, Recipe, Machine, ProjectLayer, Target from orm.models import CustomImageRecipe, ProjectVariable from orm.models import Branch from toastergui.tables import SoftwareRecipesTable -from bs4 import BeautifulSoup import json +from bs4 import BeautifulSoup import re PROJECT_NAME = "test project"
@@ -832,3 +832,90 @@ class ProjectPageTests(TestCase): response = self.client.get(url, follow=True) self.assertEqual(response.status_code, 200) + +class BuildDashboardTests(TestCase): + """ Tests for the build dashboard /build/X """ + + def setUp(self): + bbv = BitbakeVersion.objects.create(name="bbv1", giturl="/tmp/", + branch="master", dirpath="") + release = Release.objects.create(name="release1", + bitbake_version=bbv) + project = Project.objects.create_project(name=PROJECT_NAME, + release=release) + + now = timezone.now() + + self.build1 = Build.objects.create(project=project, + started_on=now, + completed_on=now) + + # exception + msg1 = 'an exception was thrown' + self.exception_message = LogMessage.objects.create( + build=self.build1, + level=LogMessage.EXCEPTION, + message=msg1 + ) + + # critical + msg2 = 'a critical error occurred' + self.critical_message = LogMessage.objects.create( + build=self.build1, + level=LogMessage.CRITICAL, + message=msg2 + ) + + def _get_build_dashboard_errors(self): + """ + Get a list of HTML fragments representing the errors on the + build dashboard + """ + url = reverse('builddashboard', args=(self.build1.id,)) + response = self.client.get(url) + soup = BeautifulSoup(response.content) + return soup.select('#errors div.alert-error') + + def _check_for_log_message(self, log_message): + """ + Check whether the LogMessage instance <log_message> is + represented as an HTML error in the build dashboard page + """ + errors = self._get_build_dashboard_errors() + self.assertEqual(len(errors), 2) + + expected_text = log_message.message + expected_id = str(log_message.id) + + found = False + for error in errors: + error_text = error.find('pre').text + text_matches = (error_text == expected_text) + + error_id = error['data-error'] + id_matches = (error_id == expected_id) + + if text_matches and id_matches: + found = True + break + + template_vars = (expected_text, error_text, + expected_id, error_id) + assertion_error_msg = 'exception not found as error: ' \ + 'expected text "%s" and got "%s"; ' \ + 'expected ID %s and got %s' % template_vars + self.assertTrue(found, assertion_error_msg) + + def test_exceptions_show_as_errors(self): + """ + LogMessages with level EXCEPTION should display in the errors + section of the page + """ + self._check_for_log_message(self.exception_message) + + def test_criticals_show_as_errors(self): + """ + LogMessages with level CRITICAL should display in the errors + section of the page + """ + self._check_for_log_message(self.critical_message)
Signature · 1 line
2.1.4
Re: [PATCH 4/4] toaster: Add tests for error message display on the build dashboard
Quoted 24 lines · Ed Bartosh
From: Elliot Smith <[email protected]>
Add tests to ensure that log messages with CRITICAL level and with EXCEPTION level are shown in the main errors display on the build dashboard.
[YOCTO #8320]
Signed-off-by Elliot Smith <[email protected]>Signed-off-by Ed Bartosh <[email protected]>--- lib/toaster/toastergui/tests.py | 91 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-)lib/toaster/toastergui/tests.py +1
-1@@ -26,15 +26,15 @@ from django.test.client import RequestFactory from django.core.urlresolvers import reverse from django.utils import timezone -from orm.models import Project, Release, BitbakeVersion, Package +from orm.models import Project, Release, BitbakeVersion, Package, LogMessage
lib/toaster/toastergui/tests.py from django.test.client import RequestFactory · line 38
Quoted 10 lines
lib/toaster/toastergui/tests.py +1
-1from orm.models import ReleaseLayerSourcePriority, LayerSource, Layer, Build from orm.models import Layer_Version, Recipe, Machine, ProjectLayer, Target from orm.models import CustomImageRecipe, ProjectVariable from orm.models import Branch from toastergui.tables import SoftwareRecipesTable -from bs4 import BeautifulSoup import json +from bs4 import BeautifulSoup import re
Sadly this failed to apply to master, please send a version which does...
Cheers,
Richard