firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove hidden dependency on ABI. (#148987)

Browse Source 
This is part 17 of a broken down version of the #140101 refactor (though this particular dependency didn't actually exist back then).

This only makes one dependency explicit. Further PRs will do the same for other dependencies, until these APIs have no hidden dependencies.

I also snuck in a minor stylistic change, using interpolation instead of explicit `toString`, to make the code more idiomatic. (I'm actually surprised that that didn't trigger a lint, I thought we had a lint to catch that. I must be thinking of something else though.)

This PR makes no effort to keep the order of parameters reasonable. There is an existing TODO to do a refactor sweep later that does nothing but reorder arguments/parameters/fields to be consistent.
This commit is contained in:
Ian Hickson 2024-05-30 12:12:22 -07:00 committed by GitHub
parent 1aba3a66f5
commit 5933e99c6c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 40 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
packages/flutter_goldens/lib/flutter_goldens.dart
View File

@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'dart:async' show FutureOr;
import 'dart:ffi' show Abi;
import 'dart:io' as io show HttpClient, OSError, SocketException;
import 'package:file/file.dart';
@ -68,6 +69,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
fs: fs,
process: process,
httpClient: httpClient,
abi: Abi.current(),
);
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromLocalFileComparator(
@ -78,6 +80,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
fs: fs,
process: process,
httpClient: httpClient,
abi: Abi.current(),
);
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromLocalFileComparator(
@ -91,6 +94,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
fs: fs,
process: process,
httpClient: httpClient,
abi: Abi.current(),
);
} else {
goldenFileComparator = await FlutterLocalFileComparator.fromLocalFileComparator(
@ -100,6 +104,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
fs: fs,
process: process,
httpClient: httpClient,
abi: Abi.current(),
);
}
await testMain();
@ -293,6 +298,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
required FileSystem fs,
required ProcessManager process,
required io.HttpClient httpClient,
required Abi abi,
}) async {
final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
@ -309,6 +315,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
fs: fs,
process: process,
httpClient: httpClient,
abi: abi,
);
await goldens.auth();
return FlutterPostSubmitFileComparator(
@ -386,6 +393,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
required FileSystem fs,
required ProcessManager process,
required io.HttpClient httpClient,
required Abi abi,
}) async {
final Directory baseDirectory = testBasedir ?? FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
@ -405,6 +413,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
fs: fs,
process: process,
httpClient: httpClient,
abi: abi,
);
await goldens.auth();
@ -487,6 +496,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
required FileSystem fs,
required ProcessManager process,
required io.HttpClient httpClient,
required Abi abi,
}) {
final Uri basedir = localFileComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(
@ -496,6 +506,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
fs: fs,
process: process,
httpClient: httpClient,
abi: abi,
);
return FlutterSkippingFileComparator(
basedir,
@ -584,6 +595,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
required FileSystem fs,
required ProcessManager process,
required io.HttpClient httpClient,
required Abi abi,
}) async {
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
@ -602,6 +614,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
fs: fs,
process: process,
httpClient: httpClient,
abi: abi,
);
try {
// Check if we can reach Gold.

10
packages/flutter_goldens/lib/skia_client.dart
View File

@ -52,10 +52,10 @@ class SkiaGoldClient {
required this.fs,
required this.process,
required this.platform,
Abi? abi,
required this.abi,
required this.httpClient,
required this.log,
}) : abi = abi ?? Abi.current();
});
/// The file system to use for storing the local clone of the repository.
///
@ -78,8 +78,6 @@ class SkiaGoldClient {
final io.HttpClient httpClient;
/// The ABI of the current host platform.
///
/// If not overridden for testing, defaults to [Abi.current];
final Abi abi;
/// The local [Directory] within the [comparisonRoot] for the current test
@ -498,7 +496,7 @@ class SkiaGoldClient {
final String? webRenderer = _webRendererValue;
final Map<String, dynamic> keys = <String, dynamic>{
'Platform' : platform.operatingSystem,
'Abi': abi.toString(),
'Abi': '$abi',
'CI' : 'luci',
if (_isImpeller)
'impeller': 'swiftshader',
@ -581,7 +579,7 @@ class SkiaGoldClient {
final Map<String, Object?> parameters = <String, Object?>{
if (_isBrowserTest)
'Browser' : _browserKey,
'Abi': abi.toString(),
'Abi': '$abi',
'CI' : 'luci',
'Platform' : platform.operatingSystem,
if (webRenderer != null)

23
packages/flutter_goldens/test/flutter_goldens_test.dart
View File

@ -31,6 +31,11 @@ const List<int> _kTestPngBytes = <int>[
78, 68, 174, 66, 96, 130,
];
// We intentionally use an ABI that is unlikely to be the one anyone editing this code
// would assume, so that if there any any assumptions made about the ABI, they will
// be more likely to fail the tests.
const Abi testAbi = Abi.fuchsiaRiscv64;
void main() {
group('SkiaGoldClient', () {
test('web HTML test', () async {
@ -55,6 +60,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -105,6 +111,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -147,6 +154,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
@ -175,6 +183,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
@ -207,6 +216,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -238,6 +248,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -288,6 +299,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -346,6 +358,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -408,6 +421,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
const RunInvocation goldctlInvocation = RunInvocation(
@ -453,6 +467,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
@ -587,6 +602,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
const RunInvocation goldctlInvocation = RunInvocation(
@ -636,6 +652,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
const RunInvocation goldctlInvocation = RunInvocation(
@ -681,6 +698,7 @@ void main() {
process: process,
platform: platform,
httpClient: fakeHttpClient,
abi: testAbi,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
);
final Uri imageUrl = Uri.parse(
@ -860,6 +878,7 @@ void main() {
fs: fs,
process: FakeProcessManager(),
httpClient: FakeHttpClient(),
abi: testAbi,
);
expect(fakeSkiaClient.initCalls, 0);
});
@ -948,6 +967,7 @@ void main() {
fs: fs,
process: FakeProcessManager(),
httpClient: FakeHttpClient(),
abi: testAbi,
);
expect(fakeSkiaClient.tryInitCalls, 0);
});
@ -1053,6 +1073,7 @@ void main() {
fs: fs,
process: FakeProcessManager(),
httpClient: FakeHttpClient(),
abi: testAbi,
);
expect(comparator1.runtimeType, FlutterSkippingFileComparator);
@ -1066,6 +1087,7 @@ void main() {
fs: fs,
process: FakeProcessManager(),
httpClient: FakeHttpClient(),
abi: testAbi,
);
expect(comparator2.runtimeType, FlutterSkippingFileComparator);
@ -1079,6 +1101,7 @@ void main() {
fs: fs,
process: FakeProcessManager(),
httpClient: FakeHttpClient(),
abi: testAbi,
);
expect(comparator3.runtimeType, FlutterSkippingFileComparator);