[LNT] Missing "Produced by" field on Run causes UI to crash

[leandron]

Summary

When a given run contains a "producer" value (usually an URL pointing to build systems), the run details page can crash when displaying this run in cases when the previous or baseline runs do not contain a valid value for the "producer" field.

It is possible to see the following error message:

...
File "/sandbox/lnt/server/ui/filters.py", line 47, in filter_producerAsHTML
  if not producer:
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'producer'

This change prevents the UI to crash when "producer" is not found. It also adds a test change to capture this situation.

[kristof.beyls]

tests/server/ui/V4Pages.py

291–292

Ideally, the test would also check that the outputted producer information is as expected, not just that the HTML return code is as expected.
You can find examples of how that's done in this file; just look for how check_nr_machines_reported is used & defined. check_table_content is another example.

Do you think it would be possible to improve the test case like this without too much effort?

[cmatthews]

LGTM, besides Kristof's point about checking the output in the test.

[leandron]

• Improve testing output to check the generated labels
• Small change to escape a bare & that crashes xml.etree.ElementTree
• sync with current SVN rev.

[leandron]

Adding context.

[kristof.beyls]

Mostly looks good, just 2 minor remarks.

tests/server/ui/V4Pages.py

133

I was confused by the name "check_table_row_content", not sure what this function was doing based on the name.
After reading the implementation, I guess "check_row_is_in_table" is a better name?

147

Isn't table_header always "Produced by" for check_producer_label?
If so, wouldn't it be better to make the signature of check_producer_label(client, url, label)?

[leandron]

I'll submit a new version including these fixes.

tests/server/ui/V4Pages.py

133

Yes. I'll rename.

147

Makes sense. I'll change.

[leandron]

Applying code review suggestions.

[kristof.beyls]

LGTM!

lnt/server/ui/templates/reporting/runs.html

<table style="{{ styles['table'] }}" class="{{ classes['table'] }}">
   {% for title, r in (('Current', run), ('Previous', compare_to), ('Baseline', baseline)) %}
     {% if r %}
     <tr id="run-table-{{ title }}">
       <td style="{{ styles['td'] }}"><a href="{{ ts_url }}/{{ r.id }}">{{ title }}</a></td>
       <td style="{{ styles['td'] }}"><a href="{{ ts_url }}/order/{{ r.order.id }}">{{ r.order.llvm_project_revision }}</a></td>
       <td style="{{ styles['td'] }}"><span class="utctime">{{ r.start_time.isoformat() }}</span></td>
       <td style="{{ styles['td'] }}">{{ r.end_time - r.start_time }}</td>
       {% if show_producers %}
-        <td style="{{ styles['td'] }}">{{ r.parameters.producer|producerAsHTML }}</td>
+        <td style="{{ styles['td'] }}">
+          {{ r.parameters.producer|producerAsHTML|safe if 'producer' in r.parameters else "" }}
+        </td>
       {% endif %}
     </tr>
     {% else %}
     <tr id="run-table-{{ title }}">
       <td style="{{ styles['td'] }}" colspan="5">No {{ title }} Run</td>
     </tr>
     {% endif %}
   {% endfor %}

tests/server/ui/Inputs/V4Pages_extra_records.sql

  VALUES(NULL,NULL,'152294'); -- ID 6
 INSERT INTO "NT_Order" ("NextOrder", "PreviousOrder", "llvm_project_revision")
  VALUES(NULL,NULL,'152295'); -- ID 7
 INSERT INTO "NT_Order" ("NextOrder", "PreviousOrder", "llvm_project_revision")
  VALUES(NULL,NULL,'152296'); -- ID 8
 INSERT INTO "NT_Run" ("MachineID", "OrderID", "ImportedFrom", "StartTime",
                       "EndTime", "SimpleRunID", "Parameters")
  VALUES(2,6,'run7.json','2012-05-10 16:28:23.000000',
-        '2012-05-10 16:28:58.000000',NULL,'[]'); -- ID 7
+        '2012-05-10 16:28:58.000000',NULL,
+        '[["producer", "http://my.build.server/buildResult"]]'); -- ID 7
 INSERT INTO "NT_Run" ("MachineID", "OrderID", "ImportedFrom", "StartTime",
                       "EndTime", "SimpleRunID", "Parameters")
  VALUES(2,7,'run8.json','2012-05-11 16:28:23.000000',
         '2012-05-11 16:28:58.000000',NULL,'[]'); -- ID 8
 INSERT INTO "NT_Run" ("MachineID", "OrderID", "ImportedFrom", "StartTime",
                       "EndTime", "SimpleRunID", "Parameters")
  VALUES(2,8,'run9.json','2012-05-12 16:28:23.000000',
         '2012-05-12 16:28:58.000000',NULL,'[]'); -- ID 9

tests/server/ui/V4Pages.py

 
 
 def check_table_content(table, expected_content):
     body_content = [[convert_html_to_text(cell).strip()
                      for cell in row.findall("./td")]
                     for row in table.findall("./tbody/tr")]
     assert expected_content == body_content, \
         "Expected table content %s, found %s" % \
         (expected_content, body_content)
 
 
 def check_body_result_table(client, url, fieldname, expected_content):
     table = get_results_table(client, url, fieldname)
     check_table_content(table, expected_content)
 
 
 def check_body_nr_tests_table(client, url, expected_content):
     table_header = "Number of Tests Seen"
     table = get_table_by_header(client, url, table_header)
     check_table_content(table, expected_content)
 
 
 def get_sparkline(table, testname, machinename):
     body_content = [[cell
                      for cell in row.findall("./td")]
                     for row in table.findall("./tbody/tr")]
     txt_body_content = [[convert_html_to_text(cell).strip()
                          for cell in row.findall("./td")]
                         for row in table.findall("./tbody/tr")]
     cur_test_name = ""
     for rownr, row_content in enumerate(txt_body_content):
check_code(client, '/v4/nts/9999/report',
                expected_code=HTTP_NOT_FOUND)
     check_code(client, '/v4/nts/9999/text_report',
                expected_code=HTTP_NOT_FOUND)
 
     # Get a graph page. This has been changed to redirect.
     check_redirect(client, '/v4/nts/1/graph?test.3=2',
                    'v4/nts/graph\?plot\.0=1\.3\.2&highlight_run=1$')
 
+    # Get a run that contains producer information
+    check_code(client, '/v4/nts/7')
+
     # Get the new graph page.
     check_code(client, '/v4/nts/graph?plot.0=1.3.2')
     # Don't crash when requesting non-existing data
     check_code(client, '/v4/nts/graph?plot.9999=1.3.2')
     check_code(client, '/v4/nts/graph?plot.0=9999.3.2',
                expected_code=HTTP_NOT_FOUND)
     check_code(client, '/v4/nts/graph?plot.0=1.9999.2',
                expected_code=HTTP_NOT_FOUND)