Skip to content

Commit a66ac0a

Browse files
Use a more deterministic way of waiting for ad widgets to be ready. (#8920)
These unit tests were failing when a skwasm change caused a subtle timing difference. See flutter/flutter#165347
1 parent 20b5861 commit a66ac0a

File tree

4 files changed

+57
-21
lines changed

4 files changed

+57
-21
lines changed

packages/google_adsense/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.1.2
2+
3+
* Added a callback to the widget for testing to make unit tests more deterministic.
4+
15
## 0.1.1
26

37
* Adds `AdSenseCodeParameters` configuration object for `adSense.initialize`.

packages/google_adsense/example/integration_test/experimental_ad_unit_widget_test.dart

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ import 'js_interop_mocks/adsense_test_js_interop.dart';
1919
const String testClient = 'test_client';
2020
const String testSlot = 'test_slot';
2121

22+
class CallbackTracker {
23+
void Function() createCallback() {
24+
final int callbackIndex = _callbackStates.length;
25+
_callbackStates.add(false);
26+
return () => _callbackStates[callbackIndex] = true;
27+
}
28+
29+
bool get allCalled => _callbackStates.every((bool state) => state);
30+
final List<bool> _callbackStates = <bool>[];
31+
}
32+
2233
void main() async {
2334
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
2435

@@ -47,15 +58,17 @@ void main() async {
4758

4859
await adSense.initialize(testClient);
4960

61+
final CallbackTracker tracker = CallbackTracker();
5062
final Widget adUnitWidget = AdUnitWidget(
5163
configuration: AdUnitConfiguration.displayAdUnit(
5264
adSlot: testSlot,
5365
adFormat: AdFormat.AUTO, // Important!
5466
),
5567
adClient: adSense.adClient,
68+
onInjected: tracker.createCallback(),
5669
);
5770

58-
await pumpAdWidget(adUnitWidget, tester);
71+
await pumpAdWidget(adUnitWidget, tester, tracker);
5972

6073
// Then
6174
// Widget level
@@ -81,19 +94,21 @@ void main() async {
8194

8295
await adSense.initialize(testClient);
8396

97+
final CallbackTracker tracker = CallbackTracker();
8498
final Widget adUnitWidget = AdUnitWidget(
8599
configuration: AdUnitConfiguration.displayAdUnit(
86100
adSlot: testSlot,
87101
),
88102
adClient: adSense.adClient,
103+
onInjected: tracker.createCallback(),
89104
);
90105

91106
final Widget constrainedAd = Container(
92107
constraints: constraints,
93108
child: adUnitWidget,
94109
);
95110

96-
await pumpAdWidget(constrainedAd, tester);
111+
await pumpAdWidget(constrainedAd, tester, tracker);
97112

98113
// Then
99114
// Widget level
@@ -110,14 +125,17 @@ void main() async {
110125
mockAdsByGoogle(mockAd(adStatus: AdStatus.UNFILLED));
111126

112127
await adSense.initialize(testClient);
128+
129+
final CallbackTracker tracker = CallbackTracker();
113130
final Widget adUnitWidget = AdUnitWidget(
114131
configuration: AdUnitConfiguration.displayAdUnit(
115132
adSlot: testSlot,
116133
),
117134
adClient: adSense.adClient,
135+
onInjected: tracker.createCallback(),
118136
);
119137

120-
await pumpAdWidget(adUnitWidget, tester);
138+
await pumpAdWidget(adUnitWidget, tester, tracker);
121139

122140
// Then
123141
expect(find.byType(HtmlElementView), findsNothing,
@@ -142,6 +160,7 @@ void main() async {
142160

143161
await adSense.initialize(testClient);
144162

163+
final CallbackTracker tracker = CallbackTracker();
145164
final Widget bunchOfAds = Column(
146165
children: <Widget>[
147166
AdUnitWidget(
@@ -150,13 +169,15 @@ void main() async {
150169
adFormat: AdFormat.AUTO,
151170
),
152171
adClient: adSense.adClient,
172+
onInjected: tracker.createCallback(),
153173
),
154174
AdUnitWidget(
155175
configuration: AdUnitConfiguration.displayAdUnit(
156176
adSlot: testSlot,
157177
adFormat: AdFormat.AUTO,
158178
),
159179
adClient: adSense.adClient,
180+
onInjected: tracker.createCallback(),
160181
),
161182
Container(
162183
constraints: const BoxConstraints(maxHeight: 100),
@@ -165,12 +186,13 @@ void main() async {
165186
adSlot: testSlot,
166187
),
167188
adClient: adSense.adClient,
189+
onInjected: tracker.createCallback(),
168190
),
169191
),
170192
],
171193
);
172194

173-
await pumpAdWidget(bunchOfAds, tester);
195+
await pumpAdWidget(bunchOfAds, tester, tracker);
174196

175197
// Then
176198
// Widget level
@@ -192,7 +214,8 @@ void main() async {
192214
}
193215

194216
// Pumps an AdUnit Widget into a given tester, with some parameters
195-
Future<void> pumpAdWidget(Widget adUnit, WidgetTester tester) async {
217+
Future<void> pumpAdWidget(
218+
Widget adUnit, WidgetTester tester, CallbackTracker tracker) async {
196219
await tester.pumpWidget(
197220
MaterialApp(
198221
home: Scaffold(
@@ -203,10 +226,14 @@ Future<void> pumpAdWidget(Widget adUnit, WidgetTester tester) async {
203226
),
204227
);
205228

206-
// This extra pump is needed for the platform view to actually render in the DOM.
207-
await tester.pump();
208-
// One more for skwasm.
209-
await tester.pump();
229+
final Stopwatch timer = Stopwatch()..start();
230+
while (!tracker.allCalled) {
231+
if (timer.elapsedMilliseconds > 1000) {
232+
fail('timeout while waiting for ad widget to be injected');
233+
}
234+
// Pump until all the widgets have had their platform views injected into the dom.
235+
await tester.pump();
236+
}
210237
// This extra pump is needed to simulate the async behavior of the adsense JS mock.
211238
await tester.pumpAndSettle();
212239
}

packages/google_adsense/lib/src/adsense/ad_unit_widget.dart

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@ class AdUnitWidget extends StatefulWidget {
3333
super.key,
3434
required AdUnitConfiguration configuration,
3535
@visibleForTesting String? adClient,
36+
@visibleForTesting void Function()? onInjected,
3637
}) : _adClient = adClient ?? adSense.adClient,
38+
_onInjected = onInjected,
3739
_adUnitConfiguration = configuration {
3840
assert(_adClient != null,
3941
'Attempted to render an AdUnitWidget before calling adSense.initialize');
4042
}
4143

4244
final String? _adClient;
45+
final void Function()? _onInjected;
4346

4447
final AdUnitConfiguration _adUnitConfiguration;
4548

@@ -58,17 +61,19 @@ class _AdUnitWidgetWebState extends State<AdUnitWidget>
5861
@override
5962
bool get wantKeepAlive => true;
6063

61-
static final web.ResizeObserver _adSenseResizeObserver = web.ResizeObserver(
62-
(JSArray<web.ResizeObserverEntry> entries, web.ResizeObserver observer) {
63-
for (final web.ResizeObserverEntry entry in entries.toDart) {
64-
final web.Element target = entry.target;
65-
if (target.isConnected) {
66-
// First time resized since attached to DOM -> attachment callback from Flutter docs by David
67-
_onElementAttached(target as web.HTMLElement);
68-
observer.disconnect();
69-
}
70-
}
71-
}.toJS);
64+
web.ResizeObserver get _adSenseResizeObserver =>
65+
web.ResizeObserver((JSArray<web.ResizeObserverEntry> entries,
66+
web.ResizeObserver observer) {
67+
for (final web.ResizeObserverEntry entry in entries.toDart) {
68+
final web.Element target = entry.target;
69+
if (target.isConnected) {
70+
// First time resized since attached to DOM -> attachment callback from Flutter docs by David
71+
_onElementAttached(target as web.HTMLElement);
72+
widget._onInjected?.call();
73+
observer.disconnect();
74+
}
75+
}
76+
}.toJS);
7277

7378
@override
7479
Widget build(BuildContext context) {

packages/google_adsense/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: google_adsense
22
description: A wrapper plugin with convenience APIs allowing easier inserting Google Adsense HTML snippets withing a Flutter UI Web application
33
repository: https://github.com/flutter/packages/tree/main/packages/google_adsense
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_adsense%22
5-
version: 0.1.1
5+
version: 0.1.2
66

77
environment:
88
sdk: ^3.4.0

0 commit comments

Comments
 (0)