ci(bench): erd_open_200 diagnoses itself; an aborted bench run fails the job (#1546)

.github/workflows/benchmark.yml

@@ -137,3 +137,13 @@ jobs:
           python3 scripts/ci/app_bench_report.py "$RUNNER_TEMP/app.log" > "$RUNNER_TEMP/app_report.txt"
           cat "$RUNNER_TEMP/app_report.txt"
           tail -n +2 "$RUNNER_TEMP/app_report.txt" >> "$GITHUB_STEP_SUMMARY"
+
+      # `flutter run` exits 0 whatever the app does; an aborted scenario run
+      # prints `BENCH FAILED`, so the job turns red instead of reporting only the
+      # scenarios that ran. The job stays informational (continue-on-error).
+      - name: Fail when the run aborted
+        run: |
+          if grep -q 'BENCH FAILED' "$RUNNER_TEMP/app.log"; then
+            grep -A3 'BENCH FAILED' "$RUNNER_TEMP/app.log" | head -n 8
+            exit 1
+          fi

benchmark/app_perf_bench.dart

@@ -81,13 +81,17 @@ Future<void> main() async {
   runApp(const QueryaApp());
 
   WidgetsBinding.instance.addPostFrameCallback((_) async {
+    var failed = false;
     try {
       await _runAll();
     } catch (e, st) {
-      stdout.writeln('BENCH error: $e\n$st');
+      // `flutter run` reports 0 whatever the app exits with, so the CI step
+      // looks for this line as well (scripts/ci/app_bench_report.py).
+      stdout.writeln('BENCH FAILED: $e\n$st');
+      failed = true;
     }
     await dataDir.delete(recursive: true).catchError((_) => dataDir);
-    exit(0);
+    exit(failed ? 1 : 0);
   });
 }
 
@@ -257,10 +261,25 @@ Future<void> _runAll() async {
   SqlEditorCommandBridge.instance.invokeNew();
   await _wait(300);
   await _runInActiveTab(ddl.toString());
+  // The script must have run: a failed or half-run script leaves the diagram
+  // of the demo's own tables, which looks like a slow ERD.
+  final ddlSession = ws.activeSession;
+  stdout.writeln('BENCH erd_ddl status=${ddlSession.statusLine} '
+      'error=${ddlSession.error}');
+  if (ddlSession.error != null) {
+    throw StateError('the 200-table script failed: ${ddlSession.error}');
+  }
   await _scenario('erd_open_200', () async {
-    await _tap(_byKey('open_diagram_tab'));
-    await _waitFor(() => _find((e) => e.widget.key == const ValueKey('erd_table_erd_t199')) != null,
-        'diagram of 200 tables');
+    // Not a tap on the toolbar button: with seven query tabs open the tab bar
+    // is full and the button is pushed out of the 1280x720 window, so the tap
+    // landed on a tab and the diagram never opened (#1546).
+    ws.openDiagramTab();
+    await _waitFor(() => ws.activeSession.isDiagram as bool, 'the Diagram tab',
+        onTimeout: () => _dumpErdState(ws));
+    await _waitFor(
+        () => _find((e) => e.widget.key == const ValueKey('erd_table_erd_t199')) != null,
+        'diagram of 200 tables',
+        onTimeout: () => _dumpErdState(ws));
     await _wait(800);
   });
   await _scenario('erd_hover', () => _hoverOver(_find((e) => e.widget is ErdView)!, 3000));
@@ -271,6 +290,22 @@ Future<void> _runAll() async {
   });
 }
 
+/// What the screen shows when the diagram did not come up: the active tab and
+/// how many table cards exist, so a failed run says which of the two it was.
+void _dumpErdState(dynamic ws) {
+  var cards = 0;
+  void visit(Element e) {
+    final k = e.widget.key;
+    if (k is ValueKey<String> && k.value.startsWith('erd_table_')) cards++;
+    e.visitChildren(visit);
+  }
+
+  WidgetsBinding.instance.rootElement!.visitChildren(visit);
+  final session = ws.activeSession;
+  stdout.writeln('BENCH erd_state activeTab="${session.title}" '
+      'isDiagram=${session.isDiagram} cards=$cards');
+}
+
 /// The SQL workspace of the open connection.
 dynamic _workspace() =>
     _findState((s) => s.runtimeType.toString() == 'GenericSqlWorkspaceState')!;

scripts/ci/app_bench_report.py

@@ -6,8 +6,9 @@
 Reads the `BENCH <scenario> frames=.. build p50/p90/max=.. raster ..` lines and
 checks each scenario of LIMITS: build p90 against its limit (ms), and for
 multi_tab_run the number of query panes rebuilt by one run. The first output
-line is `OVER=<n>`, the number of scenarios over a limit or missing.
-Informational: always exits 0.
+line is `OVER=<n>`, the number of scenarios over a limit or missing. A run that
+aborted prints `BENCH FAILED: <reason>` (`flutter run` exits 0 whatever the
+app does), which the report names. Informational: always exits 0.
 """
 import re
 import sys
@@ -32,6 +33,13 @@
     r"build p50/p90/max=([\d.]+)/([\d.]+)/([\d.]+)\s+"
     r"raster p50/p90/max=([\d.]+)/([\d.]+)/([\d.]+).*?stutters=(\d+)")
 _PANES = re.compile(r"BENCH multi_tab_run panes rebuilt=(\d+)")
+_FAILED = re.compile(r"BENCH FAILED: (.*)$", re.M)
+
+
+def failure(text):
+    """The reason the bench aborted, or None."""
+    m = _FAILED.search(text)
+    return m[1].strip() if m else None
 
 
 def parse(text):
@@ -75,6 +83,9 @@ def report(text):
         over += bad
         lines += ["", f"Query panes rebuilt by one run in one of five tabs: "
                       f"**{panes}** (limit {MAX_PANES_REBUILT}){' ⚠️' if bad else ''}."]
+    failed = failure(text)
+    if failed:
+        lines += ["", f"⚠️ The run aborted: `{failed}`. Scenarios after it did not run."]
     lines += ["", "Times in ms, profile build under xvfb with software "
                   "rendering. Informational, never blocks a merge."]
     return over, "\n".join(lines)

scripts/ci/test_app_bench_report.py

@@ -64,6 +64,16 @@ def test_rebuilding_every_pane_is_flagged(self):
         self.assertEqual(over, 1)
         self.assertIn("**5**", body)
 
+    def test_an_aborted_run_is_named(self):
+        text = line("charts_5000") + "\nBENCH FAILED: Bad state: timeout waiting for the Diagram tab\nstack\n"
+        self.assertEqual(abr.failure(text), "Bad state: timeout waiting for the Diagram tab")
+        _, body = abr.report(text)
+        self.assertIn("The run aborted: `Bad state: timeout waiting for the Diagram tab`", body)
+
+    def test_a_clean_run_has_no_failure(self):
+        self.assertIsNone(abr.failure(all_ok()))
+        self.assertNotIn("aborted", abr.report(all_ok())[1])
+
     def test_main_prints_the_count_first(self):
         with tempfile.TemporaryDirectory() as d:
             p = os.path.join(d, "app.log")