[web] Remove usage of flutter/goldens (#30116)
diff --git a/lib/web_ui/README.md b/lib/web_ui/README.md
index 786482f..af67701 100644
--- a/lib/web_ui/README.md
+++ b/lib/web_ui/README.md
@@ -184,12 +184,6 @@
web. Versions are not automatically updated whenever a new release is available.
Instead, we update this file manually once in a while.
-`goldens_lock.yaml` refers to a revision in the https://github.com/flutter/goldens
-repo. Screenshot tests are compared with the golden files at that revision.
-When making engine changes that affect screenshots, first submit a PR to
-flutter/goldens updating the screenshots. Then update this file pointing to
-the new revision.
-
`canvaskit_lock.yaml` locks the version of CanvasKit for tests and production
use.
diff --git a/lib/web_ui/dev/environment.dart b/lib/web_ui/dev/environment.dart
index 8bba0f5..792ea23 100644
--- a/lib/web_ui/dev/environment.dart
+++ b/lib/web_ui/dev/environment.dart
@@ -149,12 +149,6 @@
'lib',
));
- /// Path to the clone of the flutter/goldens repository.
- io.Directory get webUiGoldensRepositoryDirectory => io.Directory(pathlib.join(
- webUiBuildDir.path,
- 'goldens',
- ));
-
/// Path to the base directory to be used by Skia Gold.
io.Directory get webUiSkiaGoldDirectory => io.Directory(pathlib.join(
webUiDartToolDir.path,
diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml
deleted file mode 100644
index 75c3c34..0000000
--- a/lib/web_ui/dev/goldens_lock.yaml
+++ /dev/null
@@ -1,2 +0,0 @@
-repository: https://github.com/flutter/goldens.git
-revision: 0e62a287b5109053bc3e59dd8fc832f488d904c9
diff --git a/lib/web_ui/dev/steps/compile_tests_step.dart b/lib/web_ui/dev/steps/compile_tests_step.dart
index 86088aa..dcca072 100644
--- a/lib/web_ui/dev/steps/compile_tests_step.dart
+++ b/lib/web_ui/dev/steps/compile_tests_step.dart
@@ -7,7 +7,6 @@
import 'package:path/path.dart' as pathlib;
import 'package:pool/pool.dart';
-import 'package:web_test_utils/goldens.dart';
import '../environment.dart';
import '../exceptions.dart';
@@ -20,17 +19,12 @@
///
/// * canvaskit/ - CanvasKit artifacts
/// * assets/ - test fonts
-/// * goldens/ - the goldens fetched from flutter/goldens
/// * host/ - compiled test host page and static artifacts
/// * test/ - compiled test code
/// * test_images/ - test images copied from Skis sources.
class CompileTestsStep implements PipelineStep {
- CompileTestsStep({
- this.skipGoldensRepoFetch = false,
- this.testFiles,
- });
+ CompileTestsStep({this.testFiles});
- final bool skipGoldensRepoFetch;
final List<FilePath>? testFiles;
@override
@@ -47,9 +41,6 @@
@override
Future<void> run() async {
await environment.webUiBuildDir.create();
- if (!skipGoldensRepoFetch) {
- await fetchGoldensRepo();
- }
await copyCanvasKitFiles();
await buildHostPage();
await copyTestFonts();
@@ -357,11 +348,3 @@
// Record the timestamp to avoid rebuilding unless the file changes.
timestampFile.writeAsStringSync(timestamp);
}
-
-Future<void> fetchGoldensRepo() async {
- print('INFO: Fetching goldens repo');
- final GoldensRepoFetcher goldensRepoFetcher = GoldensRepoFetcher(
- environment.webUiGoldensRepositoryDirectory,
- pathlib.join(environment.webUiDevDir.path, 'goldens_lock.yaml'));
- await goldensRepoFetcher.fetch();
-}
diff --git a/lib/web_ui/dev/steps/run_tests_step.dart b/lib/web_ui/dev/steps/run_tests_step.dart
index d5601c2..c5c5518 100644
--- a/lib/web_ui/dev/steps/run_tests_step.dart
+++ b/lib/web_ui/dev/steps/run_tests_step.dart
@@ -86,9 +86,6 @@
// Separate screenshot tests from unit-tests. Screenshot tests must run
// one at a time. Otherwise, they will end up screenshotting each other.
// This is not an issue for unit-tests.
- final FilePath failureSmokeTestPath = FilePath.fromWebUi(
- 'test/golden_tests/golden_failure_smoke_test.dart',
- );
final List<FilePath> screenshotTestFiles = <FilePath>[];
final List<FilePath> unitTestFiles = <FilePath>[];
@@ -97,39 +94,18 @@
// Not a test file at all. Skip.
continue;
}
- if (testFilePath == failureSmokeTestPath) {
- // A smoke test that fails on purpose. Skip.
- continue;
- }
- // All files under test/golden_tests are considered golden tests.
- final bool isUnderGoldenTestsDirectory =
- pathlib.split(testFilePath.relativeToWebUi).contains('golden_tests');
// Any file whose name ends with "_golden_test.dart" is run as a golden test.
final bool isGoldenTestFile = pathlib
.basename(testFilePath.relativeToWebUi)
.endsWith('_golden_test.dart');
- if (isUnderGoldenTestsDirectory || isGoldenTestFile) {
+ if (isGoldenTestFile) {
screenshotTestFiles.add(testFilePath);
} else {
unitTestFiles.add(testFilePath);
}
}
- // This test returns a non-zero exit code on purpose. Run it separately.
- if (testFiles.contains(failureSmokeTestPath)) {
- await _runTestBatch(
- testFiles: <FilePath>[failureSmokeTestPath],
- browserEnvironment: _browserEnvironment,
- concurrency: 1,
- expectFailure: true,
- isDebug: isDebug,
- doUpdateScreenshotGoldens: doUpdateScreenshotGoldens,
- skiaClient: skiaClient,
- overridePathToCanvasKit: overridePathToCanvasKit,
- );
- }
-
// Run non-screenshot tests with high concurrency.
if (unitTestFiles.isNotEmpty) {
await _runTestBatch(
diff --git a/lib/web_ui/dev/test_platform.dart b/lib/web_ui/dev/test_platform.dart
index cc2dd88..834787c 100644
--- a/lib/web_ui/dev/test_platform.dart
+++ b/lib/web_ui/dev/test_platform.dart
@@ -357,22 +357,6 @@
write = true;
}
- String goldensDirectory;
- if (filename.startsWith('__local__')) {
- filename = filename.substring('__local__/'.length);
- goldensDirectory = p.join(
- env.environment.webUiRootDir.path,
- 'test',
- 'golden_files',
- );
- } else {
- goldensDirectory = p.join(
- env.environment.webUiGoldensRepositoryDirectory.path,
- 'engine',
- 'web',
- );
- }
-
final Rectangle<num> regionAsRectange = Rectangle<num>(
region['x'] as num,
region['y'] as num,
@@ -391,9 +375,6 @@
maxDiffRateFailure,
skiaClient,
isCanvaskitTest: isCanvaskitTest,
- goldensDirectory: goldensDirectory,
- filenameSuffix: _screenshotManager!.filenameSuffix,
- write: write,
);
}
diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart
index 9868b88..fbee2b9 100644
--- a/lib/web_ui/dev/test_runner.dart
+++ b/lib/web_ui/dev/test_runner.dart
@@ -60,13 +60,6 @@
'.dart_tool/goldens. Use this option to bulk-update all screenshots, '
'for example, when a new browser version affects pixels.',
)
- ..addFlag(
- 'skip-goldens-repo-fetch',
- defaultsTo: false,
- help: 'If set reuses the existig flutter/goldens repo clone. Use this '
- 'to avoid overwriting local changes when iterating on golden '
- 'tests. This is off by default.',
- )
..addOption(
'browser',
defaultsTo: 'chrome',
@@ -126,9 +119,6 @@
/// ".dart_tool/goldens".
bool get doUpdateScreenshotGoldens => boolArg('update-screenshot-goldens');
- /// Whether to fetch the goldens repo prior to running tests.
- bool get skipGoldensRepoFetch => boolArg('skip-goldens-repo-fetch');
-
/// Path to a CanvasKit build. Overrides the default CanvasKit.
String? get overridePathToCanvasKit => argResults!['canvaskit-path'] as String?;
@@ -140,10 +130,7 @@
final Pipeline testPipeline = Pipeline(steps: <PipelineStep>[
if (isWatchMode) ClearTerminalScreenStep(),
- CompileTestsStep(
- skipGoldensRepoFetch: skipGoldensRepoFetch,
- testFiles: testFiles,
- ),
+ CompileTestsStep(testFiles: testFiles),
RunTestsStep(
browserName: browserName,
testFiles: testFiles,
diff --git a/lib/web_ui/test/golden_files/smoke_test.iOS_Safari.png b/lib/web_ui/test/golden_files/smoke_test.iOS_Safari.png
deleted file mode 100644
index 7f2995f..0000000
--- a/lib/web_ui/test/golden_files/smoke_test.iOS_Safari.png
+++ /dev/null
Binary files differ
diff --git a/lib/web_ui/test/golden_files/smoke_test.png b/lib/web_ui/test/golden_files/smoke_test.png
deleted file mode 100644
index f1b4a7f..0000000
--- a/lib/web_ui/test/golden_files/smoke_test.png
+++ /dev/null
Binary files differ
diff --git a/lib/web_ui/test/golden_tests/golden_failure_smoke_test.dart b/lib/web_ui/test/golden_tests/golden_failure_smoke_test.dart
deleted file mode 100644
index 85297e5..0000000
--- a/lib/web_ui/test/golden_tests/golden_failure_smoke_test.dart
+++ /dev/null
@@ -1,21 +0,0 @@
-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'dart:html' as html;
-
-import 'package:test/bootstrap/browser.dart';
-import 'package:test/test.dart';
-import 'package:ui/ui.dart';
-import 'package:web_engine_tester/golden_tester.dart';
-
-void main() {
- internalBootstrapBrowserTest(() => testMain);
-}
-
-void testMain() {
- test('screenshot test reports failure', () async {
- html.document.body!.innerHtml = 'Text that does not appear on the screenshot!';
- await matchGoldenFile('__local__/smoke_test.png', region: const Rect.fromLTWH(0, 0, 320, 200));
- });
-}
diff --git a/lib/web_ui/test/golden_tests/golden_success_smoke_test.dart b/lib/web_ui/test/golden_tests/golden_success_smoke_test.dart
deleted file mode 100644
index b6ce8db..0000000
--- a/lib/web_ui/test/golden_tests/golden_success_smoke_test.dart
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'dart:html' as html;
-
-import 'package:test/bootstrap/browser.dart';
-import 'package:test/test.dart';
-import 'package:ui/src/engine.dart';
-import 'package:ui/ui.dart';
-import 'package:web_engine_tester/golden_tester.dart';
-
-void main() {
- internalBootstrapBrowserTest(() => testMain);
-}
-
-Future<void> testMain() 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!';
- // TODO(yjbanov): https://github.com/flutter/flutter/issues/74702 , reduce webkit percentage.
- await matchGoldenFile('__local__/smoke_test.png', region: const Rect.fromLTWH(0, 0, 320, 200),
- maxDiffRatePercent: browserEngine == BrowserEngine.webkit ? 3.0 : 0.28);
- });
-}
diff --git a/web_sdk/web_test_utils/lib/environment.dart b/web_sdk/web_test_utils/lib/environment.dart
index 3b0772c..81466b4 100644
--- a/web_sdk/web_test_utils/lib/environment.dart
+++ b/web_sdk/web_test_utils/lib/environment.dart
@@ -170,12 +170,6 @@
'lib',
));
- /// Path to the clone of the flutter/goldens repository.
- io.Directory get webUiGoldensRepositoryDirectory => io.Directory(pathlib.join(
- webUiDartToolDir.path,
- 'goldens',
- ));
-
/// Path to the base directory to be used by Skia Gold.
io.Directory get webUiSkiaGoldDirectory => io.Directory(pathlib.join(
webUiDartToolDir.path,
diff --git a/web_sdk/web_test_utils/lib/goldens.dart b/web_sdk/web_test_utils/lib/goldens.dart
index 47f9f6c..2f3f199 100644
--- a/web_sdk/web_test_utils/lib/goldens.dart
+++ b/web_sdk/web_test_utils/lib/goldens.dart
@@ -4,9 +4,6 @@
import 'dart:io' as io;
import 'package:image/image.dart';
-import 'package:path/path.dart' as path;
-
-import 'package:yaml/yaml.dart';
/// How to compares pixels within the image.
///
@@ -189,89 +186,3 @@
String getPrintableDiffFilesInfo(double diffRate, double maxRate) =>
'(${(diffRate * 100).toStringAsFixed(4)}% of pixels were different. '
'Maximum allowed rate is: ${(maxRate * 100).toStringAsFixed(4)}%).';
-
-/// Downloads the repository that stores the golden files.
-///
-/// Reads the url of the repo and `commit no` to sync to, from
-/// `goldens_lock.yaml`.
-class GoldensRepoFetcher {
- final io.Directory _webUiGoldensRepositoryDirectory;
- final String _lockFilePath;
-
- /// Constructor that takes directory to download the repository and
- /// file with goldens repo information.
- GoldensRepoFetcher(this._webUiGoldensRepositoryDirectory, this._lockFilePath);
-
- /// Fetches golden files from github.com/flutter/goldens, cloning the
- /// repository if necessary.
- ///
- /// The repository is cloned into web_ui/.dart_tool.
- Future<void> fetch() async {
- final io.File lockFile = io.File(path.join(_lockFilePath));
- final YamlMap lock = loadYaml(lockFile.readAsStringSync()) as YamlMap;
- final String repository = lock['repository'] as String;
- final String revision = lock['revision'] as String;
-
- final String? localRevision = await _getLocalRevision();
- if (localRevision == revision) {
- return;
- }
-
- print('Fetching $repository@$revision');
-
- if (!_webUiGoldensRepositoryDirectory.existsSync()) {
- _webUiGoldensRepositoryDirectory.createSync(recursive: true);
- await _runGit(
- <String>['init'],
- _webUiGoldensRepositoryDirectory.path,
- );
-
- await _runGit(
- <String>['remote', 'add', 'origin', repository],
- _webUiGoldensRepositoryDirectory.path,
- );
- }
-
- await _runGit(
- <String>['fetch', 'origin', 'main'],
- _webUiGoldensRepositoryDirectory.path,
- );
- await _runGit(
- <String>['checkout', revision],
- _webUiGoldensRepositoryDirectory.path,
- );
- }
-
- Future<String?> _getLocalRevision() async {
- final io.File head = io.File(
- path.join(_webUiGoldensRepositoryDirectory.path, '.git', 'HEAD'));
-
- if (!head.existsSync()) {
- return null;
- }
-
- return head.readAsStringSync().trim();
- }
-
- /// Runs `git` with given arguments.
- Future<void> _runGit(
- List<String> arguments,
- String workingDirectory,
- ) async {
- final io.Process process = await io.Process.start(
- 'git',
- arguments,
- workingDirectory: workingDirectory,
- // Running the process in a system shell for Windows. Otherwise
- // the process is not able to get Dart from path.
- runInShell: io.Platform.isWindows,
- mode: io.ProcessStartMode.inheritStdio,
- );
- final int exitCode = await process.exitCode;
- if (exitCode != 0) {
- throw Exception('Git command failed with arguments $arguments on '
- 'workingDirectory: $workingDirectory resulting with exitCode: '
- '$exitCode');
- }
- }
-}
diff --git a/web_sdk/web_test_utils/lib/image_compare.dart b/web_sdk/web_test_utils/lib/image_compare.dart
index 4a3bef9..7f20f79 100644
--- a/web_sdk/web_test_utils/lib/image_compare.dart
+++ b/web_sdk/web_test_utils/lib/image_compare.dart
@@ -33,56 +33,36 @@
double maxDiffRateFailure,
SkiaGoldClient? skiaClient, {
required bool isCanvaskitTest,
- // TODO(mdebbar): Remove these args with goldens repo.
- String goldensDirectory = '',
- String filenameSuffix = '',
- bool write = false,
}) async {
- if (_isLuci && skiaClient != null) {
+ if (skiaClient == null) {
+ return 'OK';
+ }
+
+ final String screenshotPath = _getFullScreenshotPath(filename);
+ final File screenshotFile = File(screenshotPath);
+ await screenshotFile.writeAsBytes(encodePng(screenshot), flush: true);
+
+ if (_isLuci) {
// This is temporary to get started by uploading existing screenshots to
// Skia Gold. The next step would be to actually use Skia Gold for
// comparison.
-
- // TODO(mdebbar): Use Skia Gold for comparison, not only for uploading.
- await _uploadToSkiaGold(skiaClient, screenshot, filename, isCanvaskitTest);
+ final int screenshotSize = screenshot.width * screenshot.height;
+ await _uploadToSkiaGold(skiaClient, screenshotFile, screenshotSize, filename, isCanvaskitTest);
+ return 'OK';
}
- filename = filename.replaceAll('.png', '$filenameSuffix.png');
+ final Image? golden = await _getGolden(filename);
- final Environment environment = Environment();
- if (goldensDirectory.isEmpty) {
- goldensDirectory = p.join(
- environment.webUiGoldensRepositoryDirectory.path,
- 'engine',
- 'web',
- );
- }
- // Bail out fast if golden doesn't exist, and user doesn't want to create it.
- final File file = File(p.join(
- goldensDirectory,
- filename,
- ));
- if (!file.existsSync() && !write) {
- return '''
-Golden file $filename does not exist.
-
-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);
- 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.';
- }
+ if (doUpdateScreenshotGoldens) {
+ return 'OK';
}
- final Image golden = decodeNamedImage(file.readAsBytesSync(), filename)!;
+ if (golden == null) {
+ // This is a new screenshot that doesn't have an existing golden.
+ return 'No golden was found for "$filename". If this is a new screenshot'
+ 'test, please take a look at the generated screenshot:\n\n'
+ '* file://$screenshotPath\n';
+ }
// Compare screenshots.
final ImageDiff diff = ImageDiff(
@@ -94,7 +74,7 @@
if (diff.rate > 0) {
final String testResultsPath = environment.webUiTestResultsDirectory.path;
Directory(testResultsPath).createSync(recursive: true);
- final String basename = p.basenameWithoutExtension(file.path);
+ final String basename = p.basenameWithoutExtension(filename);
final File actualFile =
File(p.join(testResultsPath, '$basename.actual.png'));
@@ -105,7 +85,7 @@
final File expectedFile =
File(p.join(testResultsPath, '$basename.expected.png'));
- file.copySync(expectedFile.path);
+ screenshotFile.copySync(expectedFile.path);
final File reportFile =
File(p.join(testResultsPath, '$basename.report.html'));
@@ -140,10 +120,6 @@
final String localReportPath = '$testResultsPath/$basename.report.html';
message.writeln(localReportPath);
-
- message.writeln(
- 'To update the golden file call matchGoldenFile(\'$filename\', write: '
- 'true).');
message.writeln('Golden file: ${expectedFile.path}');
message.writeln('Actual file: ${actualFile.path}');
@@ -160,48 +136,51 @@
return 'OK';
}
+Future<Image?> _getGolden(String filename) {
+ // TODO(mdebbar): Fetch the golden from Skia Gold.
+ return Future<Image?>.value(null);
+}
+
+String _getFullScreenshotPath(String filename) {
+ return p.join(environment.webUiSkiaGoldDirectory.path, filename);
+}
+
Future<void> _uploadToSkiaGold(
SkiaGoldClient skiaClient,
- Image screenshot,
+ File screenshotFile,
+ int screenshotSize,
String filename,
bool isCanvaskitTest,
) async {
// Can't upload to Gold Skia unless running in LUCI.
assert(_isLuci);
- // Write the screenshot to the file system so it can be consumed by the
- // `goldctl` tool.
- final File goldenFile = File(p.join(environment.webUiSkiaGoldDirectory.path, filename));
- await goldenFile.writeAsBytes(encodePng(screenshot), flush: true);
-
- final int screenshotSize = screenshot.width * screenshot.height;
-
if (_isPreSubmit) {
- return _uploadInPreSubmit(skiaClient, filename, goldenFile, screenshotSize, isCanvaskitTest);
+ return _uploadInPreSubmit(skiaClient, filename, screenshotFile, screenshotSize, isCanvaskitTest);
}
if (_isPostSubmit) {
- return _uploadInPostSubmit(skiaClient, filename, goldenFile, screenshotSize, isCanvaskitTest);
+ return _uploadInPostSubmit(skiaClient, filename, screenshotFile, screenshotSize, isCanvaskitTest);
}
}
Future<void> _uploadInPreSubmit(
SkiaGoldClient skiaClient,
String filename,
- File goldenFile,
+ File screenshotFile,
int screenshotSize,
bool isCanvaskitTest,
) {
assert(_isPreSubmit);
- return skiaClient.tryjobAdd(filename, goldenFile, screenshotSize, isCanvaskitTest);
+ return skiaClient.tryjobAdd(filename, screenshotFile, screenshotSize, isCanvaskitTest);
}
Future<void> _uploadInPostSubmit(
SkiaGoldClient skiaClient,
String filename,
- File goldenFile,
+ File screenshotFile,
int screenshotSize,
bool isCanvaskitTest,
) {
assert(_isPostSubmit);
- return skiaClient.imgtestAdd(filename, goldenFile, screenshotSize, isCanvaskitTest);
+ return skiaClient.imgtestAdd(filename, screenshotFile, screenshotSize, isCanvaskitTest);
}
