[LNT] Fix use of field index in daily report

[john.brawn]

Summary

Links to graph plots in the daily report make use of the field index but haven't been updated to get it via get_field_index, causing these links to be broken. Fix this by using get_field_index.

[kristof.beyls]

Hi John,

Thanks for fixing this!
Ideally, there should also be a check added to the LNT unit tests (e.g. somewhere near https://github.com/llvm-mirror/lnt/blob/master/tests/server/ui/V4Pages.py#L433).
At the moment, the "check_table_content" function there only checks text in tables, not anything in HTML tags, IIRC. (see function get_table_body_content for where everything inside HTML tags is filtered out, https://github.com/llvm-mirror/lnt/blob/master/tests/server/ui/V4Pages.py#L178).
Maybe the best way to test this is to add a helper function in tests/server/ui/V4Pages.py to extract the full content from a specific field from the table (i.e. including all HTML tags) and check against exact HTML content?

What do you think - would that be feasible and useful to implement?

[marxin]

Thank you for the fix.

[john.brawn]

Maybe the best way to test this is to add a helper function in tests/server/ui/V4Pages.py to extract the full content from a specific field from the table (i.e. including all HTML tags) and check against exact HTML content?

What do you think - would that be feasible and useful to implement?

After trying some things I think it will be easier to extract just the destination of the links and check that those are correct, especially as it's just the destination of the links that was wrong.

[kristof.beyls]

In D47581#1119035, @john.brawn wrote:

Maybe the best way to test this is to add a helper function in tests/server/ui/V4Pages.py to extract the full content from a specific field from the table (i.e. including all HTML tags) and check against exact HTML content?

What do you think - would that be feasible and useful to implement?

After trying some things I think it will be easier to extract just the destination of the links and check that those are correct, especially as it's just the destination of the links that was wrong.

That sounds like a reasonable approach to me too.

[john.brawn]

Add test.

[kristof.beyls]

LGTM.

lnt/trunk/lnt/server/ui/templates/reporting/daily_report.html

<tr>
     <td style="{{ styles.td }}" colspan="{{report.num_prior_days_to_include + 1}}">&nbsp;</td>
   </tr>
 {%-   for machine,day_results in visible_results -%}
 {%-   set key_run = report.get_key_run(machine, 0) %}
   <tr class="searchable">
     <td style="{{ styles.td }}">&nbsp;</td>
     <td class="machine-name" style="{{ styles.td }}"><a href="{{
       ts_url}}/graph?plot.0={{machine.id}}.{{test.id}}.{{
-        field.index}}&amp;highlight_run={{key_run.id}}">{{
+        report.ts.get_field_index(field)}}&amp;highlight_run={{key_run.id}}">{{
           machine.name}}</a></td>
 {%-    set first_result_shown = false -%}
 {%-    for day_result in day_results|reverse -%}
 {%-      if day_result is none %}
     <td style="{{ styles.td }}">-</td>
 {%-      else -%}
 {%-        if first_result_shown -%}
     {{ get_cell_value(day_result) }}

lnt/trunk/tests/server/ui/V4Pages.py

 
 
 def get_table_body_content(table):
     return [[convert_html_to_text(cell).strip()
              for cell in row.findall("./td")]
              for row in table.findall("./tbody/tr")]
 
 
+def get_table_links(table):
+    return [[link.get("href")
+             for cell in row.findall("./td")
+             for link in cell.findall("a")]
+             for row in table.findall("./tbody/tr")]
+
+
 def check_row_is_in_table(table, expected_row_content):
     body_content = get_table_body_content(table)
     assert expected_row_content in body_content, \
         "Expected row content %s not found in %s" % \
         (expected_row_content, body_content)
 
 
 def check_table_content(table, expected_content):
     body_content = get_table_body_content(table)
     assert expected_content == body_content, \
         "Expected table content %s, found %s" % \
         (expected_content, body_content)
 
 
+def check_table_links(table, expected_content):
+    body_content = get_table_links(table)
+    assert expected_content == body_content, \
+        "Expected table links %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)
def main():
     # more than a day apart gets reported
     result_table_20120504 = get_results_table(
         client, '/v4/nts/daily_report/2012/5/04', "Execution Time")
     check_table_content(result_table_20120504,
                         [["test1", ""],
                          ["", "machine2", "1.0000", "-", "900.00%", ""],
                          ["test2", ""],
                          ["", "machine2", "FAIL", "-", "PASS", ""]])
+    check_table_links(result_table_20120504,
+                      [[],
+                       ["/db_default/v4/nts/graph?plot.0=2.4.3&highlight_run=6"],
+                       [],
+                       ["/db_default/v4/nts/graph?plot.0=2.5.3&highlight_run=6"]])
 
     check_body_nr_tests_table(
         client, '/v4/nts/daily_report/2012/5/04',
         [['machine2', '2', '0', '1']])
 
     # Check that a failing result does not show up in the spark line
     # as a dot with value 0.
     result_table_20120513 = get_results_table(
         client, '/v4/nts/daily_report/2012/5/13?num_days=3', "Execution Time")
     check_table_content(result_table_20120513,
                         [["test6", ""],
                          ["", "machine2", "1.0000", "FAIL", "PASS", ""],
                          ["test_hash1", ""],
                          ["", "machine2", "1.0000", '-', '20.00%', ""],
                          ["test_hash2", ""],
                          ["", "machine2", "1.0000", '-', '20.00%', ""],
                          ["test_mhash_on_run", ""],
                          ["", "machine2", "1.0000", '-', '20.00%', ""], ])
+    check_table_links(result_table_20120513,
+                      [[],
+                       ['/db_default/v4/nts/graph?plot.0=2.6.3&highlight_run=9'],
+                       [],
+                       ['/db_default/v4/nts/graph?plot.0=2.7.3&highlight_run=9'],
+                       [],
+                       ['/db_default/v4/nts/graph?plot.0=2.8.3&highlight_run=9'],
+                       [],
+                       ['/db_default/v4/nts/graph?plot.0=2.9.3&highlight_run=9']])
 
     sparkline_test6_xml = \
         get_sparkline(result_table_20120513, "test6", "machine2")
     nr_sample_points = len(extract_sample_points(sparkline_test6_xml))
     assert 2 == nr_sample_points, \
         "Expected 2 sample points, found %d" % nr_sample_points
 
     # Check that a different background color is used in the sparkline