Skip to content

Commit

Permalink
add option for bulk-updating screenshots; update screenshots (flutter…
Browse files Browse the repository at this point in the history
…#12797)

* add option for bulk-updating screenshots; update screenshots
  • Loading branch information
yjbanov authored Oct 4, 2019
1 parent 7d55166 commit bba0065
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 30 deletions.
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();
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());

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

0 comments on commit bba0065

Please sign in to comment.