Skip to content

Commit

Permalink
Remove renderer on Dart:io platforms (#1723)
Browse files Browse the repository at this point in the history
* Remove renderer on dart:io platforms

* Tests + changelog

* Update CHANGELOG.md

* Fix tests

* fix more tests
ueman authored Nov 14, 2023
1 parent a97ee7c commit 48adddf
Showing 11 changed files with 85 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

### Fixes

- Flutter renderer information was removed on dart:io platforms since it didn't add the correct value ([#1723](https://github.com/getsentry/sentry-dart/pull/1723))
- Unsupported types with Expando ([#1690](https://github.com/getsentry/sentry-dart/pull/1690))

### Features
Original file line number Diff line number Diff line change
@@ -132,6 +132,8 @@ class FlutterEnricherEventProcessor implements EventProcessor {
// ignore: deprecated_member_use
final hasRenderView = _widgetsBinding?.renderViewElement != null;

final renderer = _options.rendererWrapper.getRenderer()?.name;

return <String, String>{
'has_render_view': hasRenderView.toString(),
if (tempDebugBrightnessOverride != null)
@@ -149,7 +151,7 @@ class FlutterEnricherEventProcessor implements EventProcessor {
// Also always fails in tests.
// See https://github.com/flutter/flutter/issues/83919
// 'window_is_visible': _window.viewConfiguration.visible,
'renderer': _options.rendererWrapper.getRenderer().name,
if (renderer != null) 'renderer': renderer,
};
}

Original file line number Diff line number Diff line change
@@ -32,10 +32,13 @@ class ScreenshotEventProcessor implements EventProcessor {
}

final renderer = _options.rendererWrapper.getRenderer();
if (renderer != FlutterRenderer.skia &&

if (_options.platformChecker.isWeb &&
renderer != FlutterRenderer.canvasKit) {
_options.logger(SentryLevel.debug,
'Cannot take screenshot with ${_options.rendererWrapper.getRenderer().name} renderer.');
_options.logger(
SentryLevel.debug,
'Cannot take screenshot with ${renderer?.name} renderer.',
);
return event;
}

2 changes: 1 addition & 1 deletion flutter/lib/src/renderer/html_renderer.dart
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import 'dart:js' as js;

import 'renderer.dart';

FlutterRenderer getRenderer() {
FlutterRenderer? getRenderer() {
return isCanvasKitRenderer ? FlutterRenderer.canvasKit : FlutterRenderer.html;
}

4 changes: 1 addition & 3 deletions flutter/lib/src/renderer/io_renderer.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import 'renderer.dart';

FlutterRenderer getRenderer() {
return FlutterRenderer.skia;
}
FlutterRenderer? getRenderer() => null;
10 changes: 9 additions & 1 deletion flutter/lib/src/renderer/renderer.dart
Original file line number Diff line number Diff line change
@@ -6,14 +6,22 @@ import 'unknown_renderer.dart'

@internal
class RendererWrapper {
FlutterRenderer getRenderer() {
FlutterRenderer? getRenderer() {
return implementation.getRenderer();
}
}

enum FlutterRenderer {
/// https://skia.org/
skia,

/// https://docs.flutter.dev/perf/impeller
impeller,

/// https://docs.flutter.dev/platform-integration/web/renderers
canvasKit,

/// https://docs.flutter.dev/platform-integration/web/renderers
html,
unknown,
}
3 changes: 1 addition & 2 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
@@ -167,8 +167,7 @@ mixin SentryFlutter {
integrations.add(LoadImageListIntegration(channel));
}
final renderer = options.rendererWrapper.getRenderer();
if (renderer == FlutterRenderer.skia ||
renderer == FlutterRenderer.canvasKit) {
if (!platformChecker.isWeb || renderer == FlutterRenderer.canvasKit) {
integrations.add(ScreenshotIntegration());
}

Original file line number Diff line number Diff line change
@@ -22,7 +22,38 @@ void main() {
fixture = Fixture();
});

testWidgets('flutter context', (WidgetTester tester) async {
testWidgets('flutter context on dart:io', (WidgetTester tester) async {
if (kIsWeb) {
// widget tests don't support onPlatform config
// https://pub.dev/packages/test#platform-specific-configuration
return;
}
// These two values need to be changed inside the test,
// otherwise the Flutter test framework complains that these
// values are changed outside of a test.
debugBrightnessOverride = Brightness.dark;
debugDefaultTargetPlatformOverride = TargetPlatform.android;
final enricher = fixture.getSut(
binding: () => tester.binding,
);

final event = await enricher.apply(SentryEvent());

debugBrightnessOverride = null;
debugDefaultTargetPlatformOverride = null;

final flutterContext = event?.contexts['flutter_context'];
expect(flutterContext, isNotNull);
expect(flutterContext, isA<Map<String, String>>());
}, skip: !kIsWeb);

testWidgets('flutter context on web', (WidgetTester tester) async {
if (!kIsWeb) {
// widget tests don't support onPlatform config
// https://pub.dev/packages/test#platform-specific-configuration
return;
}

// These two values need to be changed inside the test,
// otherwise the Flutter test framework complains that these
// values are changed outside of a test.
41 changes: 22 additions & 19 deletions flutter/test/event_processor/screenshot_event_processor_test.dart
Original file line number Diff line number Diff line change
@@ -19,11 +19,15 @@ void main() {
});

Future<void> _addScreenshotAttachment(
WidgetTester tester, FlutterRenderer renderer, bool added,
{int? expectedMaxWidthOrHeight}) async {
WidgetTester tester,
FlutterRenderer? renderer, {
required bool isWeb,
required bool added,
int? expectedMaxWidthOrHeight,
}) async {
// Run with real async https://stackoverflow.com/a/54021863
await tester.runAsync(() async {
final sut = fixture.getSut(renderer);
final sut = fixture.getSut(renderer, isWeb);

await tester.pumpWidget(SentryScreenshotWidget(
child: Text('Catching Pokémon is a snap!',
@@ -48,55 +52,54 @@ void main() {
});
}

testWidgets('adds screenshot attachment with skia renderer', (tester) async {
await _addScreenshotAttachment(tester, FlutterRenderer.skia, true);
testWidgets('adds screenshot attachment dart:io', (tester) async {
await _addScreenshotAttachment(tester, null, added: true, isWeb: false);
});

testWidgets('adds screenshot attachment with canvasKit renderer',
(tester) async {
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, true);
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: true);
});

testWidgets('does not add screenshot attachment with html renderer',
(tester) async {
await _addScreenshotAttachment(tester, FlutterRenderer.html, false);
});

testWidgets('does not add screenshot attachment with unknown renderer',
(tester) async {
await _addScreenshotAttachment(tester, FlutterRenderer.unknown, false);
await _addScreenshotAttachment(tester, FlutterRenderer.html,
added: false, isWeb: true);
});

testWidgets('does add screenshot in correct resolution for low',
(tester) async {
final height = SentryScreenshotQuality.low.targetResolution()!;
fixture.options.screenshotQuality = SentryScreenshotQuality.low;
await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
expectedMaxWidthOrHeight: height);
await _addScreenshotAttachment(tester, null,
added: true, isWeb: false, expectedMaxWidthOrHeight: height);
});

testWidgets('does add screenshot in correct resolution for medium',
(tester) async {
final height = SentryScreenshotQuality.medium.targetResolution()!;
fixture.options.screenshotQuality = SentryScreenshotQuality.medium;
await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
expectedMaxWidthOrHeight: height);
await _addScreenshotAttachment(tester, null,
added: true, isWeb: false, expectedMaxWidthOrHeight: height);
});

testWidgets('does add screenshot in correct resolution for high',
(tester) async {
final widthOrHeight = SentryScreenshotQuality.high.targetResolution()!;
fixture.options.screenshotQuality = SentryScreenshotQuality.high;
await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
expectedMaxWidthOrHeight: widthOrHeight);
await _addScreenshotAttachment(tester, null,
added: true, isWeb: false, expectedMaxWidthOrHeight: widthOrHeight);
});
}

class Fixture {
SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn);

ScreenshotEventProcessor getSut(FlutterRenderer flutterRenderer) {
ScreenshotEventProcessor getSut(
FlutterRenderer? flutterRenderer, bool isWeb) {
options.rendererWrapper = MockRendererWrapper(flutterRenderer);
options.platformChecker = MockPlatformChecker(isWebValue: isWeb);
return ScreenshotEventProcessor(options);
}
}
4 changes: 2 additions & 2 deletions flutter/test/mocks.dart
Original file line number Diff line number Diff line change
@@ -396,10 +396,10 @@ class MockNativeChannel implements SentryNativeBinding {
class MockRendererWrapper implements RendererWrapper {
MockRendererWrapper(this._renderer);

final FlutterRenderer _renderer;
final FlutterRenderer? _renderer;

@override
FlutterRenderer getRenderer() {
FlutterRenderer? getRenderer() {
return _renderer;
}
}
34 changes: 7 additions & 27 deletions flutter/test/sentry_flutter_test.dart
Original file line number Diff line number Diff line change
@@ -495,7 +495,7 @@ void main() {
await Sentry.close();
});

test('installed with skia renderer', () async {
test('installed on io platforms', () async {
List<Integration> integrations = [];

await SentryFlutter.init(
@@ -505,7 +505,8 @@ void main() {
integrations = options.integrations;
},
appRunner: appRunner,
platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
platformChecker:
getPlatformChecker(platform: MockPlatform.iOs(), isWeb: false),
rendererWrapper: MockRendererWrapper(FlutterRenderer.skia),
);

@@ -528,7 +529,8 @@ void main() {
integrations = options.integrations;
},
appRunner: appRunner,
platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
platformChecker:
getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true),
rendererWrapper: MockRendererWrapper(FlutterRenderer.canvasKit),
);

@@ -551,7 +553,8 @@ void main() {
integrations = options.integrations;
},
appRunner: appRunner,
platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
platformChecker:
getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true),
rendererWrapper: MockRendererWrapper(FlutterRenderer.html),
);

@@ -563,29 +566,6 @@ void main() {

await Sentry.close();
}, testOn: 'vm');

test('not installed with unknown renderer', () async {
List<Integration> integrations = [];

await SentryFlutter.init(
(options) async {
options.dsn = fakeDsn;
options.automatedTestMode = true;
integrations = options.integrations;
},
appRunner: appRunner,
platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
rendererWrapper: MockRendererWrapper(FlutterRenderer.unknown),
);

expect(
integrations
.map((e) => e.runtimeType)
.contains(ScreenshotIntegration),
false);

await Sentry.close();
}, testOn: 'vm');
});

group('initial values', () {

0 comments on commit 48adddf

Please sign in to comment.