Skip to content

[FEATURE] - Remove the need for testGoldenScene #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
timcreatedit wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[FEATURE] - Remove the need for testGoldenScene #72

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd3981f
[FEATURE] - Remove the need for `testGoldenScene` runner
timcreatedit Nov 11, 2025
ff785e7
deprecate old test function
timcreatedit Nov 11, 2025
ee6c88c
reformat with 120 line length
timcreatedit Nov 11, 2025
42ff456
add line length to project settings for vs code users
timcreatedit Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/src/fonts/fonts.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_test_goldens/src/fonts/golden_toolkit_fonts.dart' as golden_toolkit;
import 'package:flutter_test_goldens/src/fonts/golden_toolkit_fonts.dart'
as golden_toolkit;

/// Remember if fonts have already been loaded in this isolate.
bool _fontsLoaded = false;
Copy link
Contributor

@matthew-carroll matthew-carroll Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually needed? What's the current behavior that you're trying to fix with this?

Copy link
Author

@timcreatedit timcreatedit Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me like right now if I have multiple testGoldenScene tests in the same suite, they will all read the font manifest, even though the fonts have already been loaded. The same thing would happen with my change, but even if we don't move font loading somewhere else it seems like wasted work?

Copy link
Contributor

@matthew-carroll matthew-carroll Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just looking for clarity as to whether this actually prevents additional execution, or if we're just repeating something that's already tracked inside the font loading behavior. So without this we're re-reading the manifest on every call even if fonts are already loaded? (I haven't looked at that in a while).


/// Tools for working with fonts in tests.
abstract class TestFonts {
/// Load all fonts registered with the app and make them available
/// to widget tests.
static Future<void> loadAppFonts() async {
if (_fontsLoaded) {
return;
}
await golden_toolkit.loadAppFonts();
_fontsLoaded = true;
}

static const openSans = "packages/flutter_test_goldens/OpenSans";
Expand Down
139 changes: 88 additions & 51 deletions lib/src/scenes/gallery.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'package:flutter/material.dart' hide Image;
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_test_goldens/src/flutter/flutter_camera.dart';
import 'package:flutter_test_goldens/src/flutter/flutter_test_extensions.dart';
import 'package:flutter_test_goldens/src/fonts/fonts.dart';
import 'package:flutter_test_goldens/src/goldens/golden_collections.dart';
import 'package:flutter_test_goldens/src/goldens/golden_comparisons.dart';
import 'package:flutter_test_goldens/src/goldens/golden_rendering.dart';
Expand Down Expand Up @@ -296,48 +297,63 @@ class Gallery {
Future<void> run(WidgetTester tester) async {
FtgLog.pipeline.info("Rendering or comparing golden - $_sceneDescription");

// Build each golden tree and take `FlutterScreenshot`s.
final camera = FlutterCamera();
await _takeNewScreenshots(tester, camera);

// Convert each `FlutterScreenshot` to a golden `GoldenSceneScreenshot`, which includes
// additional metadata, and multiple image representations.
final screenshots = await _convertFlutterScreenshotsToSceneScreenshots(tester, camera.photos);

if (autoUpdateGoldenFiles) {
// Generate new goldens.
FtgLog.pipeline.finer("Generating new goldens...");
// TODO: Return a success/failure report that we can publish to the test output.
await _updateGoldenScene(
tester,
_fileName,
screenshots,
);
FtgLog.pipeline.finer("Done generating new goldens.");
} else {
// Compare to existing goldens.
FtgLog.pipeline.finer("Comparing existing goldens...");
// TODO: Return a success/failure report that we can publish to the test output.
await _compareGoldens(tester, _fileName, screenshots);
FtgLog.pipeline.finer("Done comparing goldens.");
await TestFonts.loadAppFonts();
Copy link
Contributor

@matthew-carroll matthew-carroll Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to force this on everyone. If people want standard Ahem goldens, they need to be able to get them.


tester.view
Copy link
Contributor

@matthew-carroll matthew-carroll Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't wanna force this either.

I see that you're trying to merge things together, but that's actually the opposite of what a toolkit should do. Higher level conveniences that add default behaviors is fine. But forcing decisions on everyone at the center of the tool will lead to angry devs who get to a certain point of adoption and then realize they can't control something that they need to control.

..devicePixelRatio = 1.0
..platformDispatcher.textScaleFactorTestValue = 1.0;

try {
// Build each golden tree and take `FlutterScreenshot`s.
final camera = FlutterCamera();
await _takeNewScreenshots(tester, camera);

// Convert each `FlutterScreenshot` to a golden `GoldenSceneScreenshot`, which includes
// additional metadata, and multiple image representations.
final screenshots = await _convertFlutterScreenshotsToSceneScreenshots(
tester, camera.photos);

if (autoUpdateGoldenFiles) {
// Generate new goldens.
FtgLog.pipeline.finer("Generating new goldens...");
// TODO: Return a success/failure report that we can publish to the test output.
await _updateGoldenScene(
tester,
_fileName,
screenshots,
);
FtgLog.pipeline.finer("Done generating new goldens.");
} else {
// Compare to existing goldens.
FtgLog.pipeline.finer("Comparing existing goldens...");
// TODO: Return a success/failure report that we can publish to the test output.
await _compareGoldens(tester, _fileName, screenshots);
FtgLog.pipeline.finer("Done comparing goldens.");
}
FtgLog.pipeline.fine("Done with golden generation/comparison");
} finally {
tester.view.reset();
}

FtgLog.pipeline.fine("Done with golden generation/comparison");
}

/// For each scene screenshot request, pumps its widget tree, and then screenshots it with
/// the given [camera].
Future<void> _takeNewScreenshots(WidgetTester tester, FlutterCamera camera) async {
Future<void> _takeNewScreenshots(
WidgetTester tester, FlutterCamera camera) async {
for (final item in _requests.values) {
FtgLog.pipeline.info("Building gallery item: ${item.description}");
final itemScaffold = item.scaffold ?? _itemScaffold ?? GoldenSceneTheme.current.itemScaffold;
final itemScaffold = item.scaffold ??
_itemScaffold ??
GoldenSceneTheme.current.itemScaffold;
final itemConstraints = item.constraints ?? _itemConstraints;

// Simulate the desired platform for this item.
final previousPlatform = debugDefaultTargetPlatformOverride;
debugDefaultTargetPlatformOverride = item.platform ?? previousPlatform;

if (itemConstraints != null && itemConstraints.hasBoundedWidth && itemConstraints.hasBoundedHeight) {
if (itemConstraints != null &&
itemConstraints.hasBoundedWidth &&
itemConstraints.hasBoundedHeight) {
// Some tests may want to control the size of the window. If we're given bounded
// constraints, make the window the biggest allowable size.
final previousSize = tester.view.physicalSize;
Expand All @@ -354,7 +370,8 @@ class Gallery {
KeyedSubtree(
// ^ Always pump with a new GlobalKey to force a fresh tree between screenshots.
key: GlobalKey(),
child: _buildItem(tester, itemScaffold, itemConstraints, Builder(builder: item.builder!)),
child: _buildItem(tester, itemScaffold, itemConstraints,
Builder(builder: item.builder!)),
),
);
} else {
Expand All @@ -363,7 +380,8 @@ class Gallery {
KeyedSubtree(
// ^ Always pump with a new GlobalKey to force a fresh tree between screenshots.
key: GlobalKey(),
child: _buildItem(tester, itemScaffold, itemConstraints, item.child!),
child:
_buildItem(tester, itemScaffold, itemConstraints, item.child!),
),
);
}
Expand All @@ -373,11 +391,13 @@ class Gallery {

// Take a screenshot.
expect(item.boundsFinder, findsOne);
final renderObject = item.boundsFinder.evaluate().first.findRenderObject();
final renderObject =
item.boundsFinder.evaluate().first.findRenderObject();
expect(
renderObject,
isNotNull,
reason: "Failed to find a render object for gallery item '${item.description}'",
reason:
"Failed to find a render object for gallery item '${item.description}'",
);

await tester.runAsync(() async {
Expand All @@ -404,15 +424,17 @@ class Gallery {
);
}

Future<Map<String, GoldenSceneScreenshot>> _convertFlutterScreenshotsToSceneScreenshots(
Future<Map<String, GoldenSceneScreenshot>>
_convertFlutterScreenshotsToSceneScreenshots(
WidgetTester tester,
List<FlutterScreenshot> flutterScreenshots,
) async {
final candidateScreenshots = <String, GoldenSceneScreenshot>{};
await tester.runAsync(() async {
for (final flutterScreenshot in flutterScreenshots) {
// Decode Flutter screenshot to raw PNG data.
final byteData = (await flutterScreenshot.pixels.toByteData(format: ui.ImageByteFormat.png))!;
final byteData = (await flutterScreenshot.pixels
.toByteData(format: ui.ImageByteFormat.png))!;

// Create a golden representation of the screenshot and store it.
final candidate = GoldenSceneScreenshot(
Expand All @@ -438,7 +460,8 @@ class Gallery {
Map<String, GoldenSceneScreenshot> newGoldens,
) async {
// Layout candidate screenshots in the gallery so we can lookup their final offsets and sizes.
var sceneMetadata = await _layoutGalleryWithNewGoldens(tester, _layout, newGoldens);
var sceneMetadata =
await _layoutGalleryWithNewGoldens(tester, _layout, newGoldens);

FtgLog.pipeline.finer("Running momentary delay for render flakiness");
await tester.runAsync(() async {
Expand All @@ -449,7 +472,8 @@ class Gallery {
});
await tester.pumpAndSettle();

FtgLog.pipeline.finer("Doing golden generation - window size: ${tester.view.physicalSize}");
FtgLog.pipeline.finer(
"Doing golden generation - window size: ${tester.view.physicalSize}");
expect(
find.byType(GoldenSceneBounds),
findsOne,
Expand All @@ -474,7 +498,8 @@ Image.memory(
)''',
);
}
await expectLater(find.byType(GoldenSceneBounds), matchesGoldenFile(_goldenFilePath()));
await expectLater(
find.byType(GoldenSceneBounds), matchesGoldenFile(_goldenFilePath()));

FtgLog.pipeline.finer("Writing updated golden scene to file");
final goldenFile = File(_goldenFilePath());
Expand All @@ -495,7 +520,8 @@ Image.memory(
final content = SceneLayoutContent(
description: _sceneDescription,
goldens: Map<GoldenSceneScreenshot, GlobalKey>.fromEntries(
goldenScreenshots.entries.map((entry) => MapEntry(entry.value, GlobalKey())),
goldenScreenshots.entries
.map((entry) => MapEntry(entry.value, GlobalKey())),
),
);

Expand Down Expand Up @@ -529,8 +555,9 @@ Image.memory(
GoldenImageMetadata(
id: golden.id,
metadata: golden.metadata,
topLeft:
(content.goldens[golden]!.currentContext!.findRenderObject() as RenderBox).localToGlobal(Offset.zero),
topLeft: (content.goldens[golden]!.currentContext!
.findRenderObject() as RenderBox)
.localToGlobal(Offset.zero),
size: content.goldens[golden]!.currentContext!.size!,
),
],
Expand All @@ -549,11 +576,13 @@ Image.memory(
Map<String, GoldenSceneScreenshot> candidateCollection,
) async {
// Extract scene metadata and golden images from image file.
FtgLog.pipeline.fine("Extracting golden collection from scene file (goldens).");
FtgLog.pipeline
.fine("Extracting golden collection from scene file (goldens).");
final goldenFile = File(_goldenFilePath());
if (!goldenFile.existsSync()) {
// TODO: report error in structured way.
throw Exception("Can't compare goldens. Golden file doesn't exist: ${goldenFile.path}");
throw Exception(
"Can't compare goldens. Golden file doesn't exist: ${goldenFile.path}");
}
final goldenCollection = extractGoldenCollectionFromSceneFile(goldenFile);

Expand All @@ -562,7 +591,8 @@ Image.memory(
final pngText = scenePngBytes.readTextMetadata();
final sceneJsonText = pngText["flutter_test_goldens"];
if (sceneJsonText == null) {
throw Exception("Golden image is missing scene metadata: ${goldenFile.path}");
throw Exception(
"Golden image is missing scene metadata: ${goldenFile.path}");
}
final sceneJson = JsonDecoder().convert(sceneJsonText);
final metadata = GoldenSceneMetadata.fromJson(sceneJson);
Expand All @@ -572,7 +602,8 @@ Image.memory(
final mismatches = compareGoldenCollections(
goldenCollection,
ScreenshotCollection(candidateCollection),
tolerances: _requests.map((id, request) => MapEntry(id, request.tolerancePx)),
tolerances:
_requests.map((id, request) => MapEntry(id, request.tolerancePx)),
);

final items = <GoldenReport>[];
Expand All @@ -581,7 +612,8 @@ Image.memory(

FtgLog.pipeline.fine("Mismatches ($existingGoldenFileName):");
for (final mismatch in mismatches.mismatches.values) {
FtgLog.pipeline.fine(" - ${mismatch.golden?.id ?? mismatch.screenshot?.id}: $mismatch");
FtgLog.pipeline.fine(
" - ${mismatch.golden?.id ?? mismatch.screenshot?.id}: $mismatch");
switch (mismatch) {
case MissingCandidateMismatch():
// A golden candidate is missing.
Expand All @@ -604,7 +636,8 @@ Image.memory(
}

// Find the golden metadata for this candidate.
final goldenMetadata = metadata.images.where((image) => image.id == candidateId).first;
final goldenMetadata =
metadata.images.where((image) => image.id == candidateId).first;

final mismatch = mismatches.mismatches[candidateId];
if (mismatch == null) {
Expand All @@ -628,7 +661,8 @@ Image.memory(
return;
}

FtgLog.pipeline.info("Found ${mismatches.mismatches.length} golden mismatches in scene.");
FtgLog.pipeline.info(
"Found ${mismatches.mismatches.length} golden mismatches in scene.");
final report = GoldenSceneReport(
metadata: metadata,
items: items,
Expand All @@ -647,7 +681,8 @@ Image.memory(
const JsonEncoder().convert(metadata.toJson()),
);

final file = File("$_goldenFailureDirectoryPath/failure_$existingGoldenFileName.png");
final file = File(
"$_goldenFailureDirectoryPath/failure_$existingGoldenFileName.png");
file.writeAsBytesSync(pngData);
});

Expand All @@ -659,9 +694,11 @@ Image.memory(
}

// TODO: Dedup following with Timeline
String get _testFileDirectory => (goldenFileComparator as LocalFileComparator).basedir.path;
String get _testFileDirectory =>
(goldenFileComparator as LocalFileComparator).basedir.path;

String get _goldenDirectory => "$_testFileDirectory${_directory.path}$separator";
String get _goldenDirectory =>
"$_testFileDirectory${_directory.path}$separator";

/// Calculates and returns a complete file path to the golden file specified by
/// this gallery, which consists of the current test file directory + an optional
Expand Down
Loading