Skip to content

Commit 5933e99

Browse files
HixieHixie
authored
Remove hidden dependency on ABI. (#148987)
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.
1 parent 1aba3a6 commit 5933e99

File tree

3 files changed

+40
-6
lines changed

3 files changed

+40
-6
lines changed

‎packages/flutter_goldens/lib/flutter_goldens.dart

+13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import 'dart:async' show FutureOr;
6+
import 'dart:ffi' show Abi;
67
import 'dart:io' as io show HttpClient, OSError, SocketException;
78

89
import 'package:file/file.dart';
@@ -68,6 +69,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
6869
fs: fs,
6970
process: process,
7071
httpClient: httpClient,
72+
abi: Abi.current(),
7173
);
7274
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
7375
goldenFileComparator = await FlutterPreSubmitFileComparator.fromLocalFileComparator(
@@ -78,6 +80,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
7880
fs: fs,
7981
process: process,
8082
httpClient: httpClient,
83+
abi: Abi.current(),
8184
);
8285
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
8386
goldenFileComparator = FlutterSkippingFileComparator.fromLocalFileComparator(
@@ -91,6 +94,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
9194
fs: fs,
9295
process: process,
9396
httpClient: httpClient,
97+
abi: Abi.current(),
9498
);
9599
} else {
96100
goldenFileComparator = await FlutterLocalFileComparator.fromLocalFileComparator(
@@ -100,6 +104,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
100104
fs: fs,
101105
process: process,
102106
httpClient: httpClient,
107+
abi: Abi.current(),
103108
);
104109
}
105110
await testMain();
@@ -293,6 +298,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
293298
required FileSystem fs,
294299
required ProcessManager process,
295300
required io.HttpClient httpClient,
301+
required Abi abi,
296302
}) async {
297303
final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
298304
localFileComparator,
@@ -309,6 +315,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
309315
fs: fs,
310316
process: process,
311317
httpClient: httpClient,
318+
abi: abi,
312319
);
313320
await goldens.auth();
314321
return FlutterPostSubmitFileComparator(
@@ -386,6 +393,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
386393
required FileSystem fs,
387394
required ProcessManager process,
388395
required io.HttpClient httpClient,
396+
required Abi abi,
389397
}) async {
390398
final Directory baseDirectory = testBasedir ?? FlutterGoldenFileComparator.getBaseDirectory(
391399
localFileComparator,
@@ -405,6 +413,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
405413
fs: fs,
406414
process: process,
407415
httpClient: httpClient,
416+
abi: abi,
408417
);
409418

410419
await goldens.auth();
@@ -487,6 +496,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
487496
required FileSystem fs,
488497
required ProcessManager process,
489498
required io.HttpClient httpClient,
499+
required Abi abi,
490500
}) {
491501
final Uri basedir = localFileComparator.basedir;
492502
final SkiaGoldClient skiaClient = SkiaGoldClient(
@@ -496,6 +506,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
496506
fs: fs,
497507
process: process,
498508
httpClient: httpClient,
509+
abi: abi,
499510
);
500511
return FlutterSkippingFileComparator(
501512
basedir,
@@ -584,6 +595,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
584595
required FileSystem fs,
585596
required ProcessManager process,
586597
required io.HttpClient httpClient,
598+
required Abi abi,
587599
}) async {
588600
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
589601
localFileComparator,
@@ -602,6 +614,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
602614
fs: fs,
603615
process: process,
604616
httpClient: httpClient,
617+
abi: abi,
605618
);
606619
try {
607620
// Check if we can reach Gold.

‎packages/flutter_goldens/lib/skia_client.dart

+4-6
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ class SkiaGoldClient {
5252
required this.fs,
5353
required this.process,
5454
required this.platform,
55-
Abi? abi,
55+
required this.abi,
5656
required this.httpClient,
5757
required this.log,
58-
}) : abi = abi ?? Abi.current();
58+
});
5959

6060
/// The file system to use for storing the local clone of the repository.
6161
///
@@ -78,8 +78,6 @@ class SkiaGoldClient {
7878
final io.HttpClient httpClient;
7979

8080
/// The ABI of the current host platform.
81-
///
82-
/// If not overridden for testing, defaults to [Abi.current];
8381
final Abi abi;
8482

8583
/// The local [Directory] within the [comparisonRoot] for the current test
@@ -498,7 +496,7 @@ class SkiaGoldClient {
498496
final String? webRenderer = _webRendererValue;
499497
final Map<String, dynamic> keys = <String, dynamic>{
500498
'Platform' : platform.operatingSystem,
501-
'Abi': abi.toString(),
499+
'Abi': '$abi',
502500
'CI' : 'luci',
503501
if (_isImpeller)
504502
'impeller': 'swiftshader',
@@ -581,7 +579,7 @@ class SkiaGoldClient {
581579
final Map<String, Object?> parameters = <String, Object?>{
582580
if (_isBrowserTest)
583581
'Browser' : _browserKey,
584-
'Abi': abi.toString(),
582+
'Abi': '$abi',
585583
'CI' : 'luci',
586584
'Platform' : platform.operatingSystem,
587585
if (webRenderer != null)

‎packages/flutter_goldens/test/flutter_goldens_test.dart

+23
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ const List<int> _kTestPngBytes = <int>[
3131
78, 68, 174, 66, 96, 130,
3232
];
3333

34+
// We intentionally use an ABI that is unlikely to be the one anyone editing this code
35+
// would assume, so that if there any any assumptions made about the ABI, they will
36+
// be more likely to fail the tests.
37+
const Abi testAbi = Abi.fuchsiaRiscv64;
38+
3439
void main() {
3540
group('SkiaGoldClient', () {
3641
test('web HTML test', () async {
@@ -55,6 +60,7 @@ void main() {
5560
process: process,
5661
platform: platform,
5762
httpClient: fakeHttpClient,
63+
abi: testAbi,
5864
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
5965
);
6066

@@ -105,6 +111,7 @@ void main() {
105111
process: process,
106112
platform: platform,
107113
httpClient: fakeHttpClient,
114+
abi: testAbi,
108115
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
109116
);
110117

@@ -147,6 +154,7 @@ void main() {
147154
process: process,
148155
platform: platform,
149156
httpClient: fakeHttpClient,
157+
abi: testAbi,
150158
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
151159
);
152160
final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
@@ -175,6 +183,7 @@ void main() {
175183
process: process,
176184
platform: platform,
177185
httpClient: fakeHttpClient,
186+
abi: testAbi,
178187
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
179188
);
180189
final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
@@ -207,6 +216,7 @@ void main() {
207216
process: process,
208217
platform: platform,
209218
httpClient: fakeHttpClient,
219+
abi: testAbi,
210220
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
211221
);
212222

@@ -238,6 +248,7 @@ void main() {
238248
process: process,
239249
platform: platform,
240250
httpClient: fakeHttpClient,
251+
abi: testAbi,
241252
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
242253
);
243254

@@ -288,6 +299,7 @@ void main() {
288299
process: process,
289300
platform: platform,
290301
httpClient: fakeHttpClient,
302+
abi: testAbi,
291303
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
292304
);
293305

@@ -346,6 +358,7 @@ void main() {
346358
process: process,
347359
platform: platform,
348360
httpClient: fakeHttpClient,
361+
abi: testAbi,
349362
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
350363
);
351364

@@ -408,6 +421,7 @@ void main() {
408421
process: process,
409422
platform: platform,
410423
httpClient: fakeHttpClient,
424+
abi: testAbi,
411425
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
412426
);
413427
const RunInvocation goldctlInvocation = RunInvocation(
@@ -453,6 +467,7 @@ void main() {
453467
process: process,
454468
platform: platform,
455469
httpClient: fakeHttpClient,
470+
abi: testAbi,
456471
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
457472
);
458473

@@ -587,6 +602,7 @@ void main() {
587602
process: process,
588603
platform: platform,
589604
httpClient: fakeHttpClient,
605+
abi: testAbi,
590606
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
591607
);
592608
const RunInvocation goldctlInvocation = RunInvocation(
@@ -636,6 +652,7 @@ void main() {
636652
process: process,
637653
platform: platform,
638654
httpClient: fakeHttpClient,
655+
abi: testAbi,
639656
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
640657
);
641658
const RunInvocation goldctlInvocation = RunInvocation(
@@ -681,6 +698,7 @@ void main() {
681698
process: process,
682699
platform: platform,
683700
httpClient: fakeHttpClient,
701+
abi: testAbi,
684702
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
685703
);
686704
final Uri imageUrl = Uri.parse(
@@ -860,6 +878,7 @@ void main() {
860878
fs: fs,
861879
process: FakeProcessManager(),
862880
httpClient: FakeHttpClient(),
881+
abi: testAbi,
863882
);
864883
expect(fakeSkiaClient.initCalls, 0);
865884
});
@@ -948,6 +967,7 @@ void main() {
948967
fs: fs,
949968
process: FakeProcessManager(),
950969
httpClient: FakeHttpClient(),
970+
abi: testAbi,
951971
);
952972
expect(fakeSkiaClient.tryInitCalls, 0);
953973
});
@@ -1053,6 +1073,7 @@ void main() {
10531073
fs: fs,
10541074
process: FakeProcessManager(),
10551075
httpClient: FakeHttpClient(),
1076+
abi: testAbi,
10561077
);
10571078
expect(comparator1.runtimeType, FlutterSkippingFileComparator);
10581079

@@ -1066,6 +1087,7 @@ void main() {
10661087
fs: fs,
10671088
process: FakeProcessManager(),
10681089
httpClient: FakeHttpClient(),
1090+
abi: testAbi,
10691091
);
10701092
expect(comparator2.runtimeType, FlutterSkippingFileComparator);
10711093

@@ -1079,6 +1101,7 @@ void main() {
10791101
fs: fs,
10801102
process: FakeProcessManager(),
10811103
httpClient: FakeHttpClient(),
1104+
abi: testAbi,
10821105
);
10831106
expect(comparator3.runtimeType, FlutterSkippingFileComparator);
10841107

0 commit comments

Comments
 (0)