diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 3c71f017..a14dc034 100644 --- a/.github/workflows/benchmark.yml +++ b/.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 diff --git a/benchmark/app_perf_bench.dart b/benchmark/app_perf_bench.dart index 840de99f..484e1baa 100644 --- a/benchmark/app_perf_bench.dart +++ b/benchmark/app_perf_bench.dart @@ -81,13 +81,17 @@ Future 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 _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 _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 && 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')!; diff --git a/scripts/ci/__pycache__/app_bench_report.cpython-314.pyc b/scripts/ci/__pycache__/app_bench_report.cpython-314.pyc new file mode 100644 index 00000000..bbf27aa4 Binary files /dev/null and b/scripts/ci/__pycache__/app_bench_report.cpython-314.pyc differ diff --git a/scripts/ci/app_bench_report.py b/scripts/ci/app_bench_report.py index 2debe55f..2f3b5486 100644 --- a/scripts/ci/app_bench_report.py +++ b/scripts/ci/app_bench_report.py @@ -6,8 +6,9 @@ Reads the `BENCH 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=`, the number of scenarios over a limit or missing. -Informational: always exits 0. +line is `OVER=`, the number of scenarios over a limit or missing. A run that +aborted prints `BENCH FAILED: ` (`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) diff --git a/scripts/ci/test_app_bench_report.py b/scripts/ci/test_app_bench_report.py index 5485b84a..93fb4b3a 100644 --- a/scripts/ci/test_app_bench_report.py +++ b/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")