Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

add option for bulk-updating screenshots; update screenshots #12797

Merged
merged 6 commits into from
Oct 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/web_ui/dev/felt.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ CommandRunner runner = CommandRunner<bool>(
'Command-line utility for building and testing Flutter web engine.',
)
..addCommand(LicensesCommand())
..addCommand(TestsCommand())
..addCommand(TestCommand())
..addCommand(BuildCommand());

void main(List<String> args) async {
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/dev/goldens_lock.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
repository: https://github.com/flutter/goldens.git
revision: dd993a32c23c5c542f083134467e7cda09cac975
revision: 17a42169bbf6739421fce8a0a1695eb3405375b6
31 changes: 23 additions & 8 deletions lib/web_ui/dev/test_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ class BrowserPlatform extends PlatformPlugin {
///
/// [root] is the root directory that the server should serve. It defaults to
/// the working directory.
static Future<BrowserPlatform> start({String root}) async {
static Future<BrowserPlatform> start({String root, bool doUpdateScreenshotGoldens: false}) async {
var server = shelf_io.IOServer(await HttpMultiServer.loopback(0));
return BrowserPlatform._(
server,
Configuration.current,
p.fromUri(await Isolate.resolvePackageUri(
Uri.parse('package:test/src/runner/browser/static/favicon.ico'))),
root: root);
server,
Configuration.current,
p.fromUri(await Isolate.resolvePackageUri(
Uri.parse('package:test/src/runner/browser/static/favicon.ico'))),
root: root,
doUpdateScreenshotGoldens: doUpdateScreenshotGoldens,
);
}

/// The test runner configuration.
Expand Down Expand Up @@ -99,8 +101,11 @@ class BrowserPlatform extends PlatformPlugin {
/// Whether [close] has been called.
bool get _closed => _closeMemo.hasRun;

/// Whether to update screenshot golden files.
final bool doUpdateScreenshotGoldens;

BrowserPlatform._(this._server, Configuration config, String faviconPath,
{String root})
{String root, this.doUpdateScreenshotGoldens})
: _config = config,
_root = root == null ? p.current : root,
_http = config.pubServeUrl == null ? null : HttpClient() {
Expand Down Expand Up @@ -147,6 +152,10 @@ class BrowserPlatform extends PlatformPlugin {
}

Future<String> _diffScreenshot(String filename, bool write, [ Map<String, dynamic> region ]) async {
if (doUpdateScreenshotGoldens) {
write = true;
}

String goldensDirectory;
if (filename.startsWith('__local__')) {
filename = filename.substring('__local__/'.length);
Expand Down Expand Up @@ -213,8 +222,14 @@ To automatically create this file call matchGoldenFile('$filename', write: true)

if (write) {
// Don't even bother with the comparison, just write and return
print('Updating screenshot golden: $file');
file.writeAsBytesSync(encodePng(screenshot), flush: true);
return 'Golden file $filename was updated. You can remove "write: true" in the call to matchGoldenFile.';
if (doUpdateScreenshotGoldens) {
// Do not fail tests when bulk-updating screenshot goldens.
return 'OK';
} else {
return 'Golden file $filename was updated. You can remove "write: true" in the call to matchGoldenFile.';
}
}

ImageDiff diff = ImageDiff(golden: decodeNamedImage(file.readAsBytesSync(), filename), other: screenshot);
Expand Down
50 changes: 35 additions & 15 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import 'test_platform.dart';
import 'environment.dart';
import 'utils.dart';

class TestsCommand extends Command<bool> {
TestsCommand() {
class TestCommand extends Command<bool> {
TestCommand() {
argParser
..addMultiOption(
'target',
Expand All @@ -32,6 +32,13 @@ class TestsCommand extends Command<bool> {
help: 'Pauses the browser before running a test, giving you an '
'opportunity to add breakpoints or inspect loaded code before '
'running the code.',
)
..addFlag(
'update-screenshot-goldens',
defaultsTo: false,
help: 'When running screenshot tests writes them to the file system into '
'.dart_tool/goldens. Use this option to bulk-update all screenshots, '
'for example, when a new browser version affects pixels.',
);

addChromeVersionOption(argParser);
Expand All @@ -47,7 +54,7 @@ class TestsCommand extends Command<bool> {
Future<bool> run() async {
Chrome.version = chromeVersion;

_copyAhemFontIntoWebUi();
_copyTestFontsIntoWebUi();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

await _buildHostPage();

final List<FilePath> targets =
Expand All @@ -73,6 +80,10 @@ class TestsCommand extends Command<bool> {
/// See [ChromeInstallerCommand.chromeVersion].
String get chromeVersion => argResults['chrome-version'];

/// When running screenshot tests writes them to the file system into
/// ".dart_tool/goldens".
bool get doUpdateScreenshotGoldens => argResults['update-screenshot-goldens'];

Future<void> _runTargetTests(List<FilePath> targets) async {
await _runTestBatch(targets, concurrency: 1, expectFailure: false);
_checkExitCode();
Expand Down Expand Up @@ -234,7 +245,11 @@ class TestsCommand extends Command<bool> {
...testFiles.map((f) => f.relativeToWebUi).toList(),
];
hack.registerPlatformPlugin(<Runtime>[Runtime.chrome], () {
return BrowserPlatform.start(root: io.Directory.current.path);
return BrowserPlatform.start(
root: io.Directory.current.path,
// It doesn't make sense to update a screenshot for a test that is expected to fail.
doUpdateScreenshotGoldens: !expectFailure && doUpdateScreenshotGoldens,
);
});

// We want to run tests with `web_ui` as a working directory.
Expand All @@ -257,15 +272,20 @@ class TestsCommand extends Command<bool> {
}
}

void _copyAhemFontIntoWebUi() {
final io.File sourceAhemTtf = io.File(path.join(
environment.flutterDirectory.path,
'third_party',
'txt',
'third_party',
'fonts',
'ahem.ttf'));
final String destinationAhemTtfPath =
path.join(environment.webUiRootDir.path, 'lib', 'assets', 'ahem.ttf');
sourceAhemTtf.copySync(destinationAhemTtfPath);
const List<String> _kTestFonts = <String>['ahem.ttf', 'Roboto-Regular.ttf'];

void _copyTestFontsIntoWebUi() {
final String fontsPath = path.join(
environment.flutterDirectory.path,
'third_party',
'txt',
'third_party',
'fonts',
);

for (String fontFile in _kTestFonts) {
final io.File sourceTtf = io.File(path.join(fontsPath, fontFile));
final String destinationTtfPath = path.join(environment.webUiRootDir.path, 'lib', 'assets', fontFile);
sourceTtf.copySync(destinationTtfPath);
}
}
2 changes: 1 addition & 1 deletion lib/web_ui/lib/assets/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ahem.ttf
*.ttf
10 changes: 7 additions & 3 deletions lib/web_ui/lib/src/engine/text/font_collection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

part of engine;

const String _testFontFamily = 'Ahem';
const String _testFontUrl = 'packages/ui/assets/ahem.ttf';
const String _ahemFontFamily = 'Ahem';
const String _ahemFontUrl = 'packages/ui/assets/ahem.ttf';
const String _robotoFontFamily = 'Roboto';
const String _robotoFontUrl = 'packages/ui/assets/Roboto-Regular.ttf';

/// This class is responsible for registering and loading fonts.
///
Expand Down Expand Up @@ -75,7 +77,9 @@ class FontCollection {
void debugRegisterTestFonts() {
_testFontManager = _FontManager();
_testFontManager.registerAsset(
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
_ahemFontFamily, 'url($_ahemFontUrl)', const <String, String>{});
_testFontManager.registerAsset(
_robotoFontFamily, 'url($_robotoFontUrl)', const <String, String>{});
}

/// Returns a [Future] that completes when the registered fonts are loaded
Expand Down
Binary file modified lib/web_ui/test/golden_files/smoke_test.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 6 additions & 1 deletion lib/web_ui/test/golden_tests/golden_success_smoke_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ import 'dart:html' as html;

import 'package:test/test.dart';
import 'package:ui/ui.dart';
import 'package:ui/src/engine.dart';
import 'package:web_engine_tester/golden_tester.dart';

void main() {
void main() async {
debugEmulateFlutterTesterEnvironment = true;
await webOnlyInitializePlatform(assetManager: WebOnlyMockAssetManager());
Copy link
Member

Choose a reason for hiding this comment

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

(I guess there's some styles in the platform init that causes the color of the font to become red)

Copy link
Member

Choose a reason for hiding this comment

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

Other than the above, LGTM!


test('screenshot test reports success', () async {
html.document.body.style.fontFamily = 'Roboto';
html.document.body.innerHtml = 'Hello world!';
await matchGoldenFile('__local__/smoke_test.png', region: Rect.fromLTWH(0, 0, 320, 200));
});
Expand Down