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/lib/features/erd/erd_view.dart b/lib/features/erd/erd_view.dart index 00fd4946..5feeb843 100644 --- a/lib/features/erd/erd_view.dart +++ b/lib/features/erd/erd_view.dart @@ -292,6 +292,9 @@ class _ErdViewState extends material.State _marked.clear(); _setLayout(ErdLayoutEngine.withSaved(_compute(_visibleOf(schema)), saved)); _loading = false; + _cardCache = {}; + _mountedTables = + schema.tables.length > _progressiveFrom ? _mountChunk : 1 << 30; }); if (schema.truncated) { showAppToast( @@ -1397,6 +1400,129 @@ class _ErdViewState extends material.State ); } + /// Cards built so far for the open diagram: a big one mounts a chunk per + /// frame (see [_mountedTables]) instead of in one long frame. + static const _progressiveFrom = 60; + static const _mountChunk = 24; + int _mountedTables = 1 << 30; + bool _mountPending = false; + + /// Cards by table, with what they were built from. A rebuild of the diagram + /// (a pick, a drag, a toolbar change) reuses every card whose inputs did not + /// change, so Flutter skips its subtree instead of rebuilding 200 of them. + var _cardCache = {}; + + List _cards(ErdSchema schema, ErdLayout layout) { + final tables = schema.tables; + final shown = min(tables.length, _mountedTables); + final menuKey = Object.hash( + Object.hashAll(_marked), + Object.hashAll([ + for (final g in _groups) + Object.hash(g.id, g.name, Object.hashAll(g.tables)), + ]), + ); + // The palette is built anew on each read; its slot colours are what the + // cards and the menu use. + final p = context.semanticPalette; + final palette = (p.type1, p.type2, p.type3, p.type4, p.type5); + final next = {}; + final cards = []; + for (var i = 0; i < shown; i++) { + final t = tables[i]; + final inputs = _CardInputs( + columns: t.columns, + comment: t.comment, + position: layout.positions[t.name]!, + width: layout.widthFor(t.name), + headerColor: _headerColor(t.name), + collapsed: _collapsed.contains(t.name), + marked: _marked.contains(t.name), + dragging: _dragging == t.name, + junctionOf: _junctions[t.name], + menuKey: menuKey, + palette: palette, + onOpenTable: widget.onOpenTable, + onOpenInSql: widget.onOpenInSql, + onShowRelations: widget.onShowRelations, + ); + final cached = _cardCache[t.name]; + final card = cached != null && cached.$1 == inputs + ? cached.$2 + : _card(t, layout); + next[t.name] = (inputs, card); + cards.add(card); + } + _cardCache = next; + if (shown < tables.length) _mountMore(); + return cards; + } + + /// Mounts the next chunk of cards after this frame. + void _mountMore() { + if (_mountPending) return; + _mountPending = true; + material.WidgetsBinding.instance.addPostFrameCallback((_) { + _mountPending = false; + if (!mounted) return; + setState(() => _mountedTables += _mountChunk); + }); + } + + material.Widget _card(ErdTable t, ErdLayout layout) { + return material.Positioned( + left: layout.positions[t.name]!.dx, + top: layout.positions[t.name]!.dy, + // Each card listens to its own focus, so a hover + // rebuilds only the cards it changes. + child: material.ValueListenableBuilder( + valueListenable: _cardState(t.name), + builder: (context, focus, _) => material.Opacity( + // Unrelated cards fade while a table is picked. + opacity: focus.faded ? 0.35 : 1, + child: ContextMenu( + items: ErdTableMenu.items( + table: t.name, + onOpenData: widget.onOpenTable, + onOpenInSql: widget.onOpenInSql, + onShowRelations: widget.onShowRelations, + collapsed: _collapsed.contains(t.name), + onToggleCollapsed: () => + _toggleCollapsed(t.name), + onHide: () => _hide(t.name), + groupItems: _groupMenu(t.name), + slotColor: _slotColor, + onHeaderColor: (slot) => + _setHeaderColor(t.name, slot), + ), + child: ErdTableCard( + table: t, + width: layout.widthFor(t.name), + headerColor: _headerColor(t.name), + highlighted: focus.highlighted, + focusColumn: focus.column, + junctionOf: _junctions[t.name], + dragging: _dragging == t.name, + onOpen: widget.onOpenTable == null + ? null + : () => widget.onOpenTable!(t.name), + marked: _marked.contains(t.name), + onSelect: () => _tapCard(t.name), + onHover: (inside) => _hovered.value = inside + ? t.name + : (_hovered.value == t.name + ? null + : _hovered.value), + onDragStart: () => _dragStart(t.name), + onDragMove: (d) => _dragMove(t.name, d), + onDragEnd: _dragEnd, + ), + ), + ), + ), + ); + } + @override material.Widget build(material.BuildContext context) { final wb = context.workbench; @@ -1481,58 +1607,7 @@ class _ErdViewState extends material.State ), ), ), - for (final t in schema.tables) - material.Positioned( - left: layout.positions[t.name]!.dx, - top: layout.positions[t.name]!.dy, - // Each card listens to its own focus, so a hover - // rebuilds only the cards it changes. - child: material.ValueListenableBuilder( - valueListenable: _cardState(t.name), - builder: (context, focus, _) => material.Opacity( - // Unrelated cards fade while a table is picked. - opacity: focus.faded ? 0.35 : 1, - child: ContextMenu( - items: ErdTableMenu.items( - table: t.name, - onOpenData: widget.onOpenTable, - onOpenInSql: widget.onOpenInSql, - onShowRelations: widget.onShowRelations, - collapsed: _collapsed.contains(t.name), - onToggleCollapsed: () => - _toggleCollapsed(t.name), - onHide: () => _hide(t.name), - groupItems: _groupMenu(t.name), - slotColor: _slotColor, - onHeaderColor: (slot) => - _setHeaderColor(t.name, slot), - ), - child: ErdTableCard( - table: t, - width: layout.widthFor(t.name), - headerColor: _headerColor(t.name), - highlighted: focus.highlighted, - focusColumn: focus.column, - junctionOf: _junctions[t.name], - dragging: _dragging == t.name, - onOpen: widget.onOpenTable == null - ? null - : () => widget.onOpenTable!(t.name), - marked: _marked.contains(t.name), - onSelect: () => _tapCard(t.name), - onHover: (inside) => _hovered.value = inside - ? t.name - : (_hovered.value == t.name - ? null - : _hovered.value), - onDragStart: () => _dragStart(t.name), - onDragMove: (d) => _dragMove(t.name, d), - onDragEnd: _dragEnd, - ), - ), - ), - ), - ), + ..._cards(schema, layout), // Sticky notes above the cards (#1283). for (final n in _notes) _noteWidget(n), ], @@ -1703,3 +1778,67 @@ class _ErdViewState extends material.State ); } } + +/// What a table card is built from outside its own focus: when all of it is +/// equal to the last build's, the card is reused as it is. +@immutable +class _CardInputs { + const _CardInputs({ + required this.columns, + required this.comment, + required this.position, + required this.width, + required this.headerColor, + required this.collapsed, + required this.marked, + required this.dragging, + required this.junctionOf, + required this.menuKey, + required this.palette, + required this.onOpenTable, + required this.onOpenInSql, + required this.onShowRelations, + }); + + final List columns; + final String? comment; + final material.Offset position; + final double width; + final material.Color headerColor; + final bool collapsed; + final bool marked; + final bool dragging; + final String? junctionOf; + + /// Hash of what the card's menu lists: the marked tables and the groups. + final int menuKey; + final Object palette; + final Object? onOpenTable; + final Object? onOpenInSql; + final Object? onShowRelations; + + @override + bool operator ==(Object other) { + if (other is! _CardInputs) return false; + if (columns.length != other.columns.length) return false; + for (var i = 0; i < columns.length; i++) { + if (!identical(columns[i], other.columns[i])) return false; + } + return comment == other.comment && + position == other.position && + width == other.width && + headerColor == other.headerColor && + collapsed == other.collapsed && + marked == other.marked && + dragging == other.dragging && + junctionOf == other.junctionOf && + menuKey == other.menuKey && + palette == other.palette && + onOpenTable == other.onOpenTable && + onOpenInSql == other.onOpenInSql && + onShowRelations == other.onShowRelations; + } + + @override + int get hashCode => Object.hash(columns.length, position, width, menuKey); +} 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") diff --git a/test/features/erd/erd_test.dart b/test/features/erd/erd_test.dart index 8963656d..ed4496be 100644 --- a/test/features/erd/erd_test.dart +++ b/test/features/erd/erd_test.dart @@ -13,6 +13,7 @@ import 'package:querya_desktop/core/erd/erd_layout.dart'; import 'package:querya_desktop/core/erd/erd_model.dart'; import 'package:querya_desktop/core/erd/erd_router.dart'; import 'package:querya_desktop/features/erd/erd_view.dart'; +import 'package:querya_desktop/features/erd/widgets/erd_search_panel.dart'; import 'package:querya_desktop/features/erd/erd_source.dart'; import 'package:querya_desktop/features/workspace/sql_execution_delegate.dart'; @@ -1004,6 +1005,67 @@ void main() { expect(erdCardBuilds, lessThanOrEqualTo(3)); }); + testWidgets('a diagram of many tables mounts its cards over several frames', + (t) async { + await t.binding.setSurfaceSize(const material.Size(1200, 800)); + final big = FakeSqlExecutionDelegate(onExecute: (sql) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + return SqlExecutionResult(rows: [ + for (var i = 0; i < 100; i++) ['t$i', 'id', 'INTEGER', '1'], + ]); + } + return const SqlExecutionResult(); + }); + await t.pumpWidget(queryaThemeTestShell( + child: ErdView( + source: SqlErdSource(delegate: big, dialect: SqlDialect.sqlite)), + )); + int cards() => find + .byWidgetPredicate((w) => + w.key is material.ValueKey && + (w.key! as material.ValueKey).value.startsWith('erd_table_t')) + .evaluate() + .length; + await t.pump(); + await t.pump(); + expect(cards(), inInclusiveRange(1, 99), + reason: 'the first frame does not carry every card'); + for (var i = 0; i < 10; i++) { + await t.pump(); + } + expect(cards(), 100); + }); + + testWidgets('opening the search rebuilds the diagram but not its cards', + (t) async { + await t.binding.setSurfaceSize(const material.Size(1200, 800)); + final wide = FakeSqlExecutionDelegate(onExecute: (sql) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + return SqlExecutionResult(rows: [ + for (var i = 0; i < 20; i++) ['t$i', 'id', 'INTEGER', '1'], + ]); + } + return const SqlExecutionResult(); + }); + await t.pumpWidget(queryaThemeTestShell( + child: ErdView( + source: SqlErdSource(delegate: wide, dialect: SqlDialect.sqlite)), + )); + await t.pump(); + await t.pump(); + await t.pump(); + + // Ctrl+F sets state on the diagram; no card looks different afterwards. + erdCardBuilds = 0; + await t.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await t.sendKeyEvent(LogicalKeyboardKey.keyF); + await t.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + await t.pump(); + expect(find.byType(ErdSearchPanel), findsOneWidget); + expect(erdCardBuilds, 0, + reason: 'the diagram rebuilt, the 20 cards were reused'); + }); + testWidgets('Copy Mermaid puts the diagram on the clipboard', (t) async { await t.binding.setSurfaceSize(const material.Size(1200, 800)); String? copied;