launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/not-error-output-bug-981085 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #981085 in MAAS: "A node in the ready state has an error output section even though the comissioning process worked fine"
  https://bugs.launchpad.net/maas/+bug/981085

For more details, see:
https://code.launchpad.net/~allenap/maas/not-error-output-bug-981085/+merge/102140
-- 
https://code.launchpad.net/~allenap/maas/not-error-output-bug-981085/+merge/102140
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/not-error-output-bug-981085 into lp:maas.

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-04-16 05:48:10 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-04-16 16:25:24 +0000
@@ -61,12 +61,16 @@
           {{ node.display_status }}
       </span>
     </li>
-    {% if node.error %}
+    {% if error_text %}
     <li class="block first">
       <h4>Error output</h4>
-      <span>
-          {{ node.error }}
-      </span>
+      <span>{{ error_text }}</span>
+    </li>
+    {% endif %}
+    {% if status_text %}
+    <li class="block first">
+      <h4>Console output</h4>
+      <span>{{ status_text }}</span>
     </li>
     {% endif %}
     {% if node.owner %}
@@ -77,4 +81,3 @@
     {% endif %}
   </ul>
 {% endblock %}
-

=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py	2012-04-16 13:20:17 +0000
+++ src/maasserver/tests/test_views.py	2012-04-16 16:25:24 +0000
@@ -690,15 +690,30 @@
 
         self.assertEqual(0, len(doc.cssselect('form#node_actions input')))
 
-    def test_view_node_shows_error_if_set(self):
-        node = factory.make_node(
-            owner=self.logged_in_user, error=factory.getRandomString())
-        node_link = reverse('node-view', args=[node.system_id])
-        response = self.client.get(node_link)
-        doc = fromstring(response.content)
-        content_text = doc.cssselect('#content')[0].text_content()
-        self.assertIn("Error output", content_text)
-        self.assertIn(node.error, content_text)
+    def test_view_node_shows_console_output_if_error_set(self):
+        # When node.error is set but the node's status does not indicate an
+        # error condition, the contents of node.error are displayed as console
+        # output.
+        node = factory.make_node(
+            owner=self.logged_in_user, error=factory.getRandomString(),
+            status=NODE_STATUS.READY)
+        node_link = reverse('node-view', args=[node.system_id])
+        response = self.client.get(node_link)
+        console_output = fromstring(response.content).xpath(
+            '//h4[text()="Console output"]/following-sibling::span/text()')
+        self.assertEqual([node.error], console_output)
+
+    def test_view_node_shows_error_output_if_error_set(self):
+        # When node.error is set and the node's status indicates an error
+        # condition, the contents of node.error are displayed as error output.
+        node = factory.make_node(
+            owner=self.logged_in_user, error=factory.getRandomString(),
+            status=NODE_STATUS.FAILED_TESTS)
+        node_link = reverse('node-view', args=[node.system_id])
+        response = self.client.get(node_link)
+        error_output = fromstring(response.content).xpath(
+            '//h4[text()="Error output"]/following-sibling::span/text()')
+        self.assertEqual([node.error], error_output)
 
     def test_view_node_shows_no_error_if_no_error_set(self):
         node = factory.make_node(owner=self.logged_in_user)

=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py	2012-04-12 13:14:10 +0000
+++ src/maasserver/views.py	2012-04-16 16:25:24 +0000
@@ -147,6 +147,10 @@
             NODE_PERMISSION.ADMIN, node)
         if node.status in (NODE_STATUS.COMMISSIONING, NODE_STATUS.READY):
             messages.info(self.request, NODE_BOOT_INFO)
+        context['error_text'] = (
+            node.error if node.status == NODE_STATUS.FAILED_TESTS else None)
+        context['status_text'] = (
+            node.error if node.status != NODE_STATUS.FAILED_TESTS else None)
         return context
 
     def get_success_url(self):