From 9cb6e3a9f05bf844062b54cf473a7d8f0d5b06b9 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:42 +0900 Subject: [PATCH 1/8] refactor(cli): write the shell template as is The template already starts with its import: Dart drops the newline after the opening quotes, so trimLeft() changed nothing. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/cli/init_command.dart | 2 +- test/cli/misc_commands_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/cli/init_command.dart b/lib/src/cli/init_command.dart index d81e6cc..787d89e 100644 --- a/lib/src/cli/init_command.dart +++ b/lib/src/cli/init_command.dart @@ -79,7 +79,7 @@ class InitCommand(final ShutterContext context) extends Command { } else { File(path) ..createSync(recursive: true) - ..writeAsStringSync(shellTemplate(project.dependencies).trimLeft()); + ..writeAsStringSync(shellTemplate(project.dependencies)); ShutterIO.stdoutSink.writeln('wrote ${project.shown(path)}'); } return 0; diff --git a/test/cli/misc_commands_test.dart b/test/cli/misc_commands_test.dart index cf61712..9597630 100644 --- a/test/cli/misc_commands_test.dart +++ b/test/cli/misc_commands_test.dart @@ -96,7 +96,7 @@ void main() { expect(first.stdout, 'wrote lib/preview/shell.dart\n'); expect( File(p.join(root, 'lib', 'preview', 'shell.dart')).readAsStringSync(), - shellTemplate(const {}).trimLeft(), + shellTemplate(const {}), ); writeFiles(root, {'lib/preview/shell.dart': '// mine'}); final second = await runCli(['init'], fakeContext(root)); From 1cbc3d94dcb28dc2f8d055bc025edddc38344fb3 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:46 +0900 Subject: [PATCH 2/8] refactor(run): hold a run's setup as one RunSetup in RunManifest lib/ only read the fields back through the setup getter; RunDiff already holds its setups this way. The manifest JSON is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/cli/shot_command.dart | 8 +++-- lib/src/run/manifest.dart | 50 ++++++++++++-------------- test/cli/shot_command_test.dart | 5 +-- test/diff/diff_engine_test.dart | 4 +-- test/reporters/shot_reporter_test.dart | 14 +++++--- test/run/manifest_test.dart | 8 +++-- 6 files changed, 47 insertions(+), 42 deletions(-) diff --git a/lib/src/cli/shot_command.dart b/lib/src/cli/shot_command.dart index 6321735..b89d43d 100644 --- a/lib/src/cli/shot_command.dart +++ b/lib/src/cli/shot_command.dart @@ -198,9 +198,11 @@ class ShotCommand(final ShutterContext context) extends Command { run: p.basename(runDir), shots: [...shots]..sort(Shot.bySource), shell: shellRecord, - actions: [for (final action in actions) action.label], - screen: screen, - viewport: viewport, + setup: ( + actions: [for (final action in actions) action.label], + screen: screen, + viewport: viewport, + ), )..write(runDir); reportShots(manifest, runDir, ShutterIO.stdoutSink); return manifest.exitCode; diff --git a/lib/src/run/manifest.dart b/lib/src/run/manifest.dart index ff6980b..c404470 100644 --- a/lib/src/run/manifest.dart +++ b/lib/src/run/manifest.dart @@ -129,15 +129,8 @@ class const RunManifest({ /// The shell file, or null for the default shell. final ShellFile? shell, - /// The actions performed on every preview before the capture, as given - /// (`tap text:Save`). - final List actions = const [], - - /// The whole viewport was captured (`--capture screen`). - final bool screen = false, - - /// The `--viewport` the shots were taken in. - final (double, double)? viewport, + /// What the run did besides rendering; see [RunSetup]. + final RunSetup setup = plainSetup, }) { factory RunManifest.fromJson(Map json) => RunManifest( run: json['run'] as String, @@ -152,12 +145,14 @@ class const RunManifest({ ), _ => null, }, - actions: [...?(json['actions'] as List?)?.cast()], - screen: json['capture'] == 'screen', - viewport: switch (json['viewport']) { - [final num w, final num h] => (w.toDouble(), h.toDouble()), - _ => null, - }, + setup: ( + actions: [...?(json['actions'] as List?)?.cast()], + screen: json['capture'] == 'screen', + viewport: switch (json['viewport']) { + [final num w, final num h] => (w.toDouble(), h.toDouble()), + _ => null, + }, + ), ); /// Reads `/manifest.json`. @@ -166,21 +161,22 @@ class const RunManifest({ as Map, ); - RunSetup get setup => (actions: actions, screen: screen, viewport: viewport); - /// 0 when every shot is ok, 2 when any is an `error`. int get exitCode => shots.any((s) => s.status == ShotStatus.error) ? 2 : 0; - Map toJson() => { - 'run': run, - if (shell case (:final path, :final sha256)?) - 'shell': {'path': path, 'sha256': sha256}, - if (actions.isNotEmpty) 'actions': actions, - if (screen) 'capture': 'screen', - if (viewport case (final w, final h)?) - 'viewport': [jsonNumber(w), jsonNumber(h)], - 'shots': [for (final shot in shots) shot.toJson()], - }; + Map toJson() { + final (:actions, :screen, :viewport) = setup; + return { + 'run': run, + if (shell case (:final path, :final sha256)?) + 'shell': {'path': path, 'sha256': sha256}, + if (actions.isNotEmpty) 'actions': actions, + if (screen) 'capture': 'screen', + if (viewport case (final w, final h)?) + 'viewport': [jsonNumber(w), jsonNumber(h)], + 'shots': [for (final shot in shots) shot.toJson()], + }; + } /// Writes `/manifest.json` as indented JSON. void write(String dir) => File( diff --git a/test/cli/shot_command_test.dart b/test/cli/shot_command_test.dart index b448271..72d3409 100644 --- a/test/cli/shot_command_test.dart +++ b/test/cli/shot_command_test.dart @@ -284,8 +284,9 @@ void main() { expect(report['capture'], 'screen'); expect(report['viewport'], [390, 844]); final manifest = RunManifest.read(report['run'] as String); - expect(manifest.actions, labels); - expect((manifest.screen, manifest.viewport), (true, (390.0, 844.0))); + final setup = manifest.setup; + expect(setup.actions, labels); + expect((setup.screen, setup.viewport), (true, (390.0, 844.0))); for (final (flag, kind) in [ ('--hover', ActionKind.hover), diff --git a/test/diff/diff_engine_test.dart b/test/diff/diff_engine_test.dart index 0253e07..bc8f277 100644 --- a/test/diff/diff_engine_test.dart +++ b/test/diff/diff_engine_test.dart @@ -41,9 +41,7 @@ void main() { run: 'pressed', shots: [], shell: (path: 'lib/preview/shell.dart', sha256: '1f2e'), - actions: ['press text:OK'], - screen: true, - viewport: (390, 844), + setup: (actions: ['press text:OK'], screen: true, viewport: (390, 844)), )..write(dir), ); final diff = diffRuns(run('plain', const [], const {}), pressed); diff --git a/test/reporters/shot_reporter_test.dart b/test/reporters/shot_reporter_test.dart index 8e9105d..0b03b2a 100644 --- a/test/reporters/shot_reporter_test.dart +++ b/test/reporters/shot_reporter_test.dart @@ -83,9 +83,11 @@ shots: const RunManifest( run: 'r', shots: [], - actions: ['tap text:Open', 'press key:save'], - screen: true, - viewport: (390, 844), + setup: ( + actions: ['tap text:Open', 'press key:save'], + screen: true, + viewport: (390, 844), + ), ), ); expect( @@ -99,7 +101,11 @@ shots: ), ); final screenOnly = await render( - const RunManifest(run: 'r', shots: [], screen: true), + const RunManifest( + run: 'r', + shots: [], + setup: (actions: [], screen: true, viewport: null), + ), ); expect(screenOnly, contains('shell: default\ncapture: screen\nsummary:')); }); diff --git a/test/run/manifest_test.dart b/test/run/manifest_test.dart index ee8b7fc..b2f1fe5 100644 --- a/test/run/manifest_test.dart +++ b/test/run/manifest_test.dart @@ -76,9 +76,11 @@ void main() { const RunManifest( run: 'r', shots: [], - actions: ['tap text:Open', 'press key:save'], - screen: true, - viewport: (390, 844.5), + setup: ( + actions: ['tap text:Open', 'press key:save'], + screen: true, + viewport: (390, 844.5), + ), ).write(dir); final back = RunManifest.read(dir); final setup = back.setup; From 7aca5580b2d6674f089d505fa6309fb3f09ddcc4 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:47 +0900 Subject: [PATCH 3/8] refactor(engine): look up google_fonts once per capture _capture looked the package up again for the generated config; the flag now comes from the lookup capture branches on. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/engine/flutter_test_engine.dart | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/src/engine/flutter_test_engine.dart b/lib/src/engine/flutter_test_engine.dart index 55b4a70..000187e 100644 --- a/lib/src/engine/flutter_test_engine.dart +++ b/lib/src/engine/flutter_test_engine.dart @@ -45,7 +45,12 @@ class FlutterTestEngine implements Engine { final googleFonts = request.project.packageRoot('google_fonts'); final cache = FontCache(request.project.googleFontsDir, fetch: fetchFont); final design = DesignSupport.detect(request.project, sdk); - var pass = await _capture(request, design, cache.list()); + var pass = await _capture( + request, + design, + cache.list(), + googleFonts: googleFonts != null, + ); if (googleFonts == null) return pass.shots; final known = {}; final failures = {}; @@ -82,7 +87,7 @@ class FlutterTestEngine implements Engine { await Future.wait(pending.map(fetch)); if (!added) break; - pass = await _capture(request, design, cache.list()); + pass = await _capture(request, design, cache.list(), googleFonts: true); } return [ for (final shot in pass.shots) @@ -101,8 +106,9 @@ class FlutterTestEngine implements Engine { Future<({List shots, Map> missingFonts})> _capture( CaptureRequest request, DesignSupport design, - List fonts, - ) async { + List fonts, { + required bool googleFonts, + }) async { final project = request.project; final shots = [ for (final library in request.libraries) @@ -114,7 +120,7 @@ class FlutterTestEngine implements Engine { request: request, materialFontsDir: sdk.materialFontsDir, design: design, - googleFonts: project.packageRoot('google_fonts') != null, + googleFonts: googleFonts, fonts: fonts, ), ); From d07e5b179cdd0be37652edb216ffcd7f64d6b0ec Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:47 +0900 Subject: [PATCH 4/8] refactor(engine): build an action's optional text once label and source repeated their whole string for --enter; the optional part is built as a suffix, as WidgetShot does for its size. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/engine/interaction.dart | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/src/engine/interaction.dart b/lib/src/engine/interaction.dart index 49bc7e3..952371f 100644 --- a/lib/src/engine/interaction.dart +++ b/lib/src/engine/interaction.dart @@ -47,18 +47,21 @@ class const ShotAction({ }) { /// As given on the command line, and recorded in the manifest: /// `tap text:Save`, `enter key:name=Koji`. - String get label => switch (text) { - final text? => '${kind.name} ${by.name}:$value=$text', - null => '${kind.name} ${by.name}:$value', - }; + String get label { + final text = switch (this.text) { + final text? => '=$text', + null => '', + }; + return '${kind.name} ${by.name}:$value$text'; + } /// The harness's `ShutterAction` for this action, as Dart source. - String get source => switch (text) { - final text? => - '\$shutter.ShutterAction(.${kind.name}, .${by.name}, ' - '${dartString(value)}, text: ${dartString(text)})', - null => - '\$shutter.ShutterAction(.${kind.name}, .${by.name}, ' - '${dartString(value)})', - }; + String get source { + final text = switch (this.text) { + final text? => ', text: ${dartString(text)}', + null => '', + }; + return '\$shutter.ShutterAction(.${kind.name}, .${by.name}, ' + '${dartString(value)}$text)'; + } } From e6852afbabd144c221c63f9b45cfbb8a834b7b84 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:47 +0900 Subject: [PATCH 5/8] refactor(engine): use null-aware map elements in the harness The harness writes the same keys as Shot.toJson, which already uses them; brightness loses its null assertion. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/engine/harness_text.dart | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/src/engine/harness_text.dart b/lib/src/engine/harness_text.dart index 39b2895..3e95df8 100644 --- a/lib/src/engine/harness_text.dart +++ b/lib/src/engine/harness_text.dart @@ -196,8 +196,8 @@ Map _entryFields( ) => { 'id': id, 'name': name, - if (entry.file != null) 'file': entry.file, - if (entry.line != null) 'line': entry.line, + 'file': ?entry.file, + 'line': ?entry.line, }; /// `at` pointing at the declaration; none for an expression. @@ -258,9 +258,8 @@ Future _capture( }; final result = { ..._entryFields(entry, id, name), - if (preview.brightness != null) 'brightness': preview.brightness!.name, - if (preview.textScaleFactor != null) - 'text_scale_factor': preview.textScaleFactor, + 'brightness': ?preview.brightness?.name, + 'text_scale_factor': ?preview.textScaleFactor, }; LocalizationsResolver? resolver; // What undoes a held action: a pressed or hovering pointer, the focus From a1f0b68020e0124d8a50dabb2726f3f522b1e286 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:47 +0900 Subject: [PATCH 6/8] refactor(engine): make the widget helper name private Only generator.dart uses it. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/engine/generator.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/engine/generator.dart b/lib/src/engine/generator.dart index 6d4dbd6..7be6b25 100644 --- a/lib/src/engine/generator.dart +++ b/lib/src/engine/generator.dart @@ -38,7 +38,7 @@ class const GeneratorConfig({ }); /// Name of the helper library shooting [CaptureRequest.widget]. -const widgetHelper = 'widget'; +const _widgetHelper = 'widget'; /// Writes `.dart_tool/shutter/test//`: the harness, the design library /// adapters, one helper library per source library (and one for the @@ -65,9 +65,9 @@ String writeGeneratedTest(GeneratorConfig config) { helpers.add('l$i'); } if (request.widget case final widget?) { - File(p.join(dir, 'sources', '$widgetHelper.dart')) + File(p.join(dir, 'sources', '$_widgetHelper.dart')) .writeAsStringSync(widget.helperSource()); - helpers.add(widgetHelper); + helpers.add(_widgetHelper); } final testPath = p.join(dir, 'shutter_test.dart'); From ffc50e4b586a0fbe65a15630965624178155cb4b Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:33:47 +0900 Subject: [PATCH 7/8] refactor(scan): compute the static id in Candidate.staticId Its only caller in lib/ was the getter, as WidgetShot.staticId computes its own; the id format is documented on the getter. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/scan/candidate.dart | 12 +++++------- test/scan/candidate_test.dart | 11 ++++++++++- test/scan/scanner_test.dart | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/src/scan/candidate.dart b/lib/src/scan/candidate.dart index 0ced00d..5b8a313 100644 --- a/lib/src/scan/candidate.dart +++ b/lib/src/scan/candidate.dart @@ -33,15 +33,13 @@ class Candidate({ /// without being compiled. final String? error, }) { - /// First 16 hex of `sha256("||")`. - String get staticId => shotStaticId(file, symbol, annotationIndex); + /// Static half of a shot id: the first 16 hex of + /// `sha256("||")`. The runtime half (the + /// index of the `Preview` produced by `transform()`) is appended after + /// a `.`. + String get staticId => shortHash('$file|$symbol|$annotationIndex'); } -/// Static half of a shot id. The runtime half (the index of the -/// `Preview` produced by `transform()`) is appended after a `.`. -String shotStaticId(String file, String symbol, int annotationIndex) => - shortHash('$file|$symbol|$annotationIndex'); - /// First 16 hex digits of `sha256(input)`. String shortHash(String input) => sha256.convert(utf8.encode(input)).toString().substring(0, 16); diff --git a/test/scan/candidate_test.dart b/test/scan/candidate_test.dart index b5dec1d..6858be4 100644 --- a/test/scan/candidate_test.dart +++ b/test/scan/candidate_test.dart @@ -4,6 +4,15 @@ import 'package:test/test.dart'; void main() { test('static id is the first 16 hex of sha256(file|symbol|index)', () { // Ids pair shots across runs, so the formula must not drift. - expect(shotStaticId('lib/a.dart', 'a', 0), 'e815cfe43338aced'); + final candidate = Candidate( + file: 'lib/a.dart', + line: 1, + column: 1, + symbol: 'a', + annotationIndex: 0, + annotation: '', + target: '', + ); + expect(candidate.staticId, 'e815cfe43338aced'); }); } diff --git a/test/scan/scanner_test.dart b/test/scan/scanner_test.dart index 84e6d45..493bd8c 100644 --- a/test/scan/scanner_test.dart +++ b/test/scan/scanner_test.dart @@ -57,7 +57,7 @@ class Button extends Widget { ); expect(c.target, r'$i2.a'); expect(c.error, isNull); - expect(c.staticId, shotStaticId('lib/preview/a.dart', 'a', 0)); + expect(c.staticId, shortHash('lib/preview/a.dart|a|0')); }); test('only Flutter\'s Preview and MultiPreview make previews; a ' From 20344af0b07c7378dbccdbafbe4e93bc5c89cab6 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 23 Sep 2026 15:34:08 +0900 Subject: [PATCH 8/8] docs(engine): correct and reflow the engine and harness doc comments The engine writes a per-run directory and deletes it, not one file; two harness doc lines ran past 80 columns. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/engine/flutter_test_engine.dart | 6 +++--- lib/src/engine/harness_text.dart | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/src/engine/flutter_test_engine.dart b/lib/src/engine/flutter_test_engine.dart index 000187e..bdedb75 100644 --- a/lib/src/engine/flutter_test_engine.dart +++ b/lib/src/engine/flutter_test_engine.dart @@ -21,9 +21,9 @@ const _maxFontRounds = 3; /// `flutter test` exits 79 when a suite registers no tests. const _noTestsRan = 79; -/// Engine v1: generates a `flutter_test` file under -/// `.dart_tool/shutter/test/`, runs -/// it with `flutter test`, and deletes it. +/// Engine v1: generates a `flutter_test` suite in a per-run directory, +/// `.dart_tool/shutter/test//`, runs it with `flutter test`, and +/// deletes the directory. class FlutterTestEngine implements Engine { FlutterTestEngine({ required this.sdk, diff --git a/lib/src/engine/harness_text.dart b/lib/src/engine/harness_text.dart index 3e95df8..1346340 100644 --- a/lib/src/engine/harness_text.dart +++ b/lib/src/engine/harness_text.dart @@ -1,9 +1,9 @@ -/// Source of `.dart_tool/shutter/test//shutter_harness.dart`, the runtime -/// half of the `flutter_test` engine. It is plain Flutter test code: it -/// depends on `flutter` and `flutter_test` only, so the target project -/// gains no dependency on shutter. The generated `shutter_test.dart` calls [run] -/// with the scanned entries; each preview becomes one `testWidgets` that -/// writes `/.png` and `/.results/.json`. +/// Source of `.dart_tool/shutter/test//shutter_harness.dart`, the +/// runtime half of the `flutter_test` engine. It is plain Flutter test code: +/// it depends on `flutter` and `flutter_test` only, so the target project +/// gains no dependency on shutter. The generated `shutter_test.dart` calls +/// [run] with the scanned entries; each preview becomes one `testWidgets` +/// that writes `/.png` and `/.results/.json`. /// /// The end-to-end tests under `test/e2e/` compile and run this text /// against `example/`.