Merge pull request #25027 from palimondo/single-table++

Author: swift-ci

benchmark/scripts/compare_perf_tests.py

@@ -547,15 +547,6 @@ def __init__(self, comparator, changes_only,
         self.changes_only = changes_only
         self.single_table = single_table
 
-    MARKDOWN_DETAIL = """
-<details {3}>
-  <summary>{0} ({1})</summary>
-  {2}
-</details>
-"""
-    GIT_DETAIL = """
-{0} ({1}): {2}"""
-
     PERFORMANCE_TEST_RESULT_HEADER = ('TEST', 'MIN', 'MAX', 'MEAN', 'MAX_RSS')
     RESULT_COMPARISON_HEADER = ('TEST', 'OLD', 'NEW', 'DELTA', 'RATIO')
 
@@ -589,16 +580,26 @@ def values(result):
     def markdown(self):
         """Report results of benchmark comparisons in Markdown format."""
         return self._formatted_text(
-            ROW='{0} | {1} | {2} | {3} | {4} \n',
-            HEADER_SEPARATOR='---',
-            DETAIL=self.MARKDOWN_DETAIL)
+            label_formatter=lambda s: ('**' + s + '**'),
+            COLUMN_SEPARATOR=' | ',
+            DELIMITER_ROW=([':---'] + ['---:'] * 4),
+            SEPARATOR='&nbsp; | | | | \n',
+            SECTION="""
+<details {3}>
+  <summary>{0} ({1})</summary>
+  {2}
+</details>
+""")
 
     def git(self):
         """Report results of benchmark comparisons in 'git' format."""
         return self._formatted_text(
-            ROW='{0}   {1}   {2}   {3}   {4} \n',
-            HEADER_SEPARATOR='   ',
-            DETAIL=self.GIT_DETAIL)
+            label_formatter=lambda s: s.upper(),
+            COLUMN_SEPARATOR='   ',
+            DELIMITER_ROW=None,
+            SEPARATOR='\n',
+            SECTION="""
+{0} ({1}): \n{2}""")
 
     def _column_widths(self):
         changed = self.comparator.decreased + self.comparator.increased
@@ -614,53 +615,49 @@ def _column_widths(self):
         ]
 
         def max_widths(maximum, widths):
-            return tuple(map(max, zip(maximum, widths)))
+            return map(max, zip(maximum, widths))
 
-        return reduce(max_widths, widths, tuple([0] * 5))
+        return reduce(max_widths, widths, [0] * 5)
 
-    def _formatted_text(self, ROW, HEADER_SEPARATOR, DETAIL):
+    def _formatted_text(self, label_formatter, COLUMN_SEPARATOR,
+                        DELIMITER_ROW, SEPARATOR, SECTION):
         widths = self._column_widths()
         self.header_printed = False
 
         def justify_columns(contents):
-            return tuple([c.ljust(w) for w, c in zip(widths, contents)])
+            return [c.ljust(w) for w, c in zip(widths, contents)]
 
         def row(contents):
-            return ROW.format(*justify_columns(contents))
-
-        def header(header):
-            return '\n' + row(header) + row(tuple([HEADER_SEPARATOR] * 5))
-
-        def format_columns(r, strong):
-            return (r if not strong else
-                    r[:-1] + ('**{0}**'.format(r[-1]), ))
+            return ('' if not contents else
+                    COLUMN_SEPARATOR.join(justify_columns(contents)) + '\n')
+
+        def header(title, column_labels):
+            labels = (column_labels if not self.single_table else
+                      map(label_formatter, (title, ) + column_labels[1:]))
+            h = (('' if not self.header_printed else SEPARATOR) +
+                 row(labels) +
+                 (row(DELIMITER_ROW) if not self.header_printed else ''))
+            if self.single_table and not self.header_printed:
+                self.header_printed = True
+            return h
+
+        def format_columns(r, is_strong):
+            return (r if not is_strong else
+                    r[:-1] + ('**' + r[-1] + '**', ))
 
         def table(title, results, is_strong=False, is_open=False):
-            rows = [
-                row(format_columns(ReportFormatter.values(r), is_strong))
-                for r in results
-            ]
-            if not rows:
+            if not results:
                 return ''
-
-            if self.single_table:
-                t = ''
-                if not self.header_printed:
-                    t += header(ReportFormatter.header_for(results[0]))
-                    self.header_printed = True
-                t += row(('**' + title + '**', '', '', '', ''))
-                t += ''.join(rows)
-                return t
-
-            return DETAIL.format(
-                *[
-                    title, len(results),
-                    (header(ReportFormatter.header_for(results[0])) +
-                     ''.join(rows)),
-                    ('open' if is_open else '')
-                ])
-
-        return ''.join([
+            rows = [row(format_columns(ReportFormatter.values(r), is_strong))
+                    for r in results]
+            table = (header(title if self.single_table else '',
+                            ReportFormatter.header_for(results[0])) +
+                     ''.join(rows))
+            return (table if self.single_table else
+                    SECTION.format(
+                        title, len(results), table, 'open' if is_open else ''))
+
+        return '\n' + ''.join([
             table('Regression', self.comparator.decreased, True, True),
             table('Improvement', self.comparator.increased, True),
             ('' if self.changes_only else

benchmark/scripts/run_smoke_bench

@@ -88,7 +88,7 @@ def main():
         help='In addition to stdout, write the results into a markdown file')
     argparser.add_argument(
         '-threshold', type=float,
-        help='The performance threshold in % which triggers a re-run',
+        help='The performance threshold in %% which triggers a re-run',
         default=5)
     argparser.add_argument(
         '-num-samples', type=int,

benchmark/scripts/test_compare_perf_tests.py

@@ -759,7 +759,7 @@ def test_column_headers(self):
         )
         self.assert_markdown_contains([
             'TEST                  | OLD    | NEW    | DELTA   | RATIO',
-            '---                   | ---    | ---    | ---     | ---    ',
+            ':---                  | ---:   | ---:   | ---:    | ---:   ',
             'TEST                  | MIN    | MAX    | MEAN    | MAX_RSS'])
         self.assert_git_contains([
             'TEST                    OLD      NEW      DELTA     RATIO',
@@ -855,6 +855,36 @@ def test_report_only_changes(self):
         self.assertNotIn('No Changes', html)
         self.assertNotIn('AngryPhonebook', html)
 
+    def test_single_table_report(self):
+        """Single table report has inline headers and no elaborate sections."""
+        self.tc.removed = []  # test handling empty section
+        rf = ReportFormatter(self.tc, changes_only=True, single_table=True)
+        markdown = rf.markdown()
+        self.assertNotIn('<details', markdown)  # no sections
+        self.assertNotIn('\n\n', markdown)  # table must not be broken
+        self.assertNotIn('Removed', markdown)
+        self.assert_report_contains([
+            '\n**Regression** ',
+            '| **OLD**', '| **NEW**', '| **DELTA**', '| **RATIO**',
+            '\n**Added** ',
+            '| **MIN**', '| **MAX**', '| **MEAN**', '| **MAX_RSS**'
+        ], markdown)
+        # Single delimiter row:
+        self.assertIn('\n:---', markdown)  # first column is left aligned
+        self.assertEqual(markdown.count('| ---:'), 4)  # other, right aligned
+        # Separator before every inline header (new section):
+        self.assertEqual(markdown.count('&nbsp; | | | | '), 2)
+
+        git = rf.git()
+        self.assertNotIn('): \n', git)  # no sections
+        self.assertNotIn('REMOVED', git)
+        self.assert_report_contains([
+            '\nREGRESSION ', ' OLD ', ' NEW ', ' DELTA ', ' RATIO ',
+            '\n\nADDED ', ' MIN ', ' MAX ', ' MEAN ', ' MAX_RSS '
+        ], git)
+        # Separator before every inline header (new section):
+        self.assertEqual(git.count('\n\n'), 2)
+
 
 class Test_parse_args(unittest.TestCase):
     required = ['--old-file', 'old.log', '--new-file', 'new.log']