Skip to content

Commit

Permalink
[web] Remove usage of flutter/goldens (flutter#30116)
Browse files Browse the repository at this point in the history
  • Loading branch information
mdebbar authored Jan 24, 2022
1 parent 016f458 commit 83cfdcc
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 295 deletions.
6 changes: 0 additions & 6 deletions lib/web_ui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,6 @@ If you have questions, contact the Flutter Web team on Flutter Discord on the
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.

Expand Down
6 changes: 0 additions & 6 deletions lib/web_ui/dev/environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,6 @@ class Environment {
'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,
Expand Down
2 changes: 0 additions & 2 deletions lib/web_ui/dev/goldens_lock.yaml

This file was deleted.

19 changes: 1 addition & 18 deletions lib/web_ui/dev/steps/compile_tests_step.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'dart:io' as io;

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';
Expand All @@ -20,17 +19,12 @@ import '../utils.dart';
///
/// * 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
Expand All @@ -47,9 +41,6 @@ class CompileTestsStep implements PipelineStep {
@override
Future<void> run() async {
await environment.webUiBuildDir.create();
if (!skipGoldensRepoFetch) {
await fetchGoldensRepo();
}
await copyCanvasKitFiles();
await buildHostPage();
await copyTestFonts();
Expand Down Expand Up @@ -357,11 +348,3 @@ Future<void> buildHostPage() async {
// 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();
}
26 changes: 1 addition & 25 deletions lib/web_ui/dev/steps/run_tests_step.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ class RunTestsStep implements PipelineStep {
// 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>[];

Expand All @@ -97,39 +94,18 @@ class RunTestsStep implements PipelineStep {
// 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(
Expand Down
19 changes: 0 additions & 19 deletions lib/web_ui/dev/test_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,22 +357,6 @@ class BrowserPlatform extends PlatformPlugin {
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,
Expand All @@ -391,9 +375,6 @@ class BrowserPlatform extends PlatformPlugin {
maxDiffRateFailure,
skiaClient,
isCanvaskitTest: isCanvaskitTest,
goldensDirectory: goldensDirectory,
filenameSuffix: _screenshotManager!.filenameSuffix,
write: write,
);
}

Expand Down
15 changes: 1 addition & 14 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@ class TestCommand extends Command<bool> with ArgUtils<bool> {
'.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',
Expand Down Expand Up @@ -126,9 +119,6 @@ class TestCommand extends Command<bool> with ArgUtils<bool> {
/// ".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?;

Expand All @@ -140,10 +130,7 @@ class TestCommand extends Command<bool> with ArgUtils<bool> {

final Pipeline testPipeline = Pipeline(steps: <PipelineStep>[
if (isWatchMode) ClearTerminalScreenStep(),
CompileTestsStep(
skipGoldensRepoFetch: skipGoldensRepoFetch,
testFiles: testFiles,
),
CompileTestsStep(testFiles: testFiles),
RunTestsStep(
browserName: browserName,
testFiles: testFiles,
Expand Down
Binary file not shown.
Binary file removed lib/web_ui/test/golden_files/smoke_test.png
Binary file not shown.
21 changes: 0 additions & 21 deletions lib/web_ui/test/golden_tests/golden_failure_smoke_test.dart

This file was deleted.

28 changes: 0 additions & 28 deletions lib/web_ui/test/golden_tests/golden_success_smoke_test.dart

This file was deleted.

6 changes: 0 additions & 6 deletions web_sdk/web_test_utils/lib/environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,6 @@ class Environment {
'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,
Expand Down
89 changes: 0 additions & 89 deletions web_sdk/web_test_utils/lib/goldens.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -189,89 +186,3 @@ class ImageDiff {
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');
}
}
}
Loading

0 comments on commit 83cfdcc

Please sign in to comment.