Unverified Commit 94216273 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Enable `only_throw_errors` (#91567)

parent 71c876f4
...@@ -137,7 +137,7 @@ linter: ...@@ -137,7 +137,7 @@ linter:
- null_closures - null_closures
# - omit_local_variable_types # opposite of always_specify_types # - omit_local_variable_types # opposite of always_specify_types
# - one_member_abstracts # too many false positives # - one_member_abstracts # too many false positives
# - only_throw_errors # https://github.com/flutter/flutter/issues/5792 - only_throw_errors # this does get disabled in a few places where we have legacy code that uses strings et al
- overridden_fields - overridden_fields
- package_api_docs - package_api_docs
- package_names - package_names
......
...@@ -3,3 +3,4 @@ include: ../analysis_options.yaml ...@@ -3,3 +3,4 @@ include: ../analysis_options.yaml
linter: linter:
rules: rules:
avoid_print: false # We use prints as debugging tools here all the time. avoid_print: false # We use prints as debugging tools here all the time.
only_throw_errors: false # Tests use a less... rigorous style.
include: ../../../analysis_options.yaml
linter:
rules:
only_throw_errors: false # We are more flexible for tests.
...@@ -249,7 +249,7 @@ class AsyncSnapshot<T> { ...@@ -249,7 +249,7 @@ class AsyncSnapshot<T> {
if (hasData) if (hasData)
return data!; return data!;
if (hasError) if (hasError)
throw error!; throw error!; // ignore: only_throw_errors, since we're just propagating an existing error
throw StateError('Snapshot has neither data nor error'); throw StateError('Snapshot has neither data nor error');
} }
......
...@@ -1132,8 +1132,10 @@ class _ImageState extends State<Image> with WidgetsBindingObserver { ...@@ -1132,8 +1132,10 @@ class _ImageState extends State<Image> with WidgetsBindingObserver {
_lastStack = stackTrace; _lastStack = stackTrace;
}); });
assert(() { assert(() {
if (widget.errorBuilder == null) if (widget.errorBuilder == null) {
// ignore: only_throw_errors, since we're just proxying the error.
throw error; // Ensures the error message is printed to the console. throw error; // Ensures the error message is printed to the console.
}
return true; return true;
}()); }());
} }
......
include: ../analysis_options.yaml
linter:
rules:
only_throw_errors: false # We are more flexible for tests.
...@@ -92,7 +92,7 @@ mixin CreateFinderFactory { ...@@ -92,7 +92,7 @@ mixin CreateFinderFactory {
case 'String': case 'String':
return find.byKey(ValueKey<String>(arguments.keyValue as String)); return find.byKey(ValueKey<String>(arguments.keyValue as String));
default: default:
throw 'Unsupported ByValueKey type: ${arguments.keyValueType}'; throw UnimplementedError('Unsupported ByValueKey type: ${arguments.keyValueType}');
} }
} }
...@@ -194,8 +194,10 @@ mixin CommandHandlerFactory { ...@@ -194,8 +194,10 @@ mixin CommandHandlerFactory {
Future<Result> _enterText(Command command) async { Future<Result> _enterText(Command command) async {
if (!_testTextInput.isRegistered) { if (!_testTextInput.isRegistered) {
throw 'Unable to fulfill `FlutterDriver.enterText`. Text emulation is ' throw StateError(
'disabled. You can enable it using `FlutterDriver.setTextEntryEmulation`.'; 'Unable to fulfill `FlutterDriver.enterText`. Text emulation is '
'disabled. You can enable it using `FlutterDriver.setTextEntryEmulation`.',
);
} }
final EnterText enterTextCommand = command as EnterText; final EnterText enterTextCommand = command as EnterText;
_testTextInput.enterText(enterTextCommand.text); _testTextInput.enterText(enterTextCommand.text);
......
...@@ -148,7 +148,7 @@ class VMServiceFlutterDriver extends FlutterDriver { ...@@ -148,7 +148,7 @@ class VMServiceFlutterDriver extends FlutterDriver {
return vms.Success(); return vms.Success();
} else { } else {
// Failed to resume due to another reason. Fail hard. // Failed to resume due to another reason. Fail hard.
throw e; throw e; // ignore: only_throw_errors, proxying the error from upstream.
} }
}); });
} }
......
...@@ -686,7 +686,7 @@ class FakeProcessManager extends Fake implements ProcessManager { ...@@ -686,7 +686,7 @@ class FakeProcessManager extends Fake implements ProcessManager {
// See also dev/automated_tests/flutter_test/flutter_gold_test.dart // See also dev/automated_tests/flutter_test/flutter_gold_test.dart
class FakeSkiaGoldClient extends Fake implements SkiaGoldClient { class FakeSkiaGoldClient extends Fake implements SkiaGoldClient {
Map<String, String> expectationForTestValues = <String, String>{}; Map<String, String> expectationForTestValues = <String, String>{};
Object? getExpectationForTestThrowable; Exception? getExpectationForTestThrowable;
@override @override
Future<String> getExpectationForTest(String testName) async { Future<String> getExpectationForTest(String testName) async {
if (getExpectationForTestThrowable != null) { if (getExpectationForTestThrowable != null) {
......
...@@ -10,8 +10,7 @@ import 'dart:ui'; ...@@ -10,8 +10,7 @@ import 'dart:ui';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:path/path.dart' as path; import 'package:path/path.dart' as path;
// ignore: deprecated_member_use import 'package:test_api/expect.dart' show fail;
import 'package:test_api/test_api.dart' as test_package show TestFailure;
import 'goldens.dart'; import 'goldens.dart';
import 'test_async_utils.dart'; import 'test_async_utils.dart';
...@@ -117,7 +116,7 @@ class LocalFileComparator extends GoldenFileComparator with LocalComparisonOutpu ...@@ -117,7 +116,7 @@ class LocalFileComparator extends GoldenFileComparator with LocalComparisonOutpu
Future<List<int>> getGoldenBytes(Uri golden) async { Future<List<int>> getGoldenBytes(Uri golden) async {
final File goldenFile = _getGoldenFile(golden); final File goldenFile = _getGoldenFile(golden);
if (!goldenFile.existsSync()) { if (!goldenFile.existsSync()) {
throw test_package.TestFailure( fail(
'Could not be compared against non-existent file: "$golden"' 'Could not be compared against non-existent file: "$golden"'
); );
} }
......
...@@ -6,8 +6,7 @@ import 'dart:convert'; ...@@ -6,8 +6,7 @@ import 'dart:convert';
import 'dart:html' as html; import 'dart:html' as html;
import 'dart:typed_data'; import 'dart:typed_data';
// ignore: deprecated_member_use import 'package:test_api/expect.dart' show fail;
import 'package:test_api/test_api.dart' as test_package show TestFailure;
import 'goldens.dart'; import 'goldens.dart';
...@@ -72,9 +71,8 @@ class DefaultWebGoldenComparator extends WebGoldenComparator { ...@@ -72,9 +71,8 @@ class DefaultWebGoldenComparator extends WebGoldenComparator {
final String response = request.response as String; final String response = request.response as String;
if (response == 'true') { if (response == 'true') {
return true; return true;
} else {
throw test_package.TestFailure(response);
} }
fail(response);
} }
@override @override
......
...@@ -81,14 +81,14 @@ class MatchesGoldenFile extends AsyncMatcher { ...@@ -81,14 +81,14 @@ class MatchesGoldenFile extends AsyncMatcher {
} }
imageFuture = captureImage(elements.single); imageFuture = captureImage(elements.single);
} else { } else {
throw 'must provide a Finder, Image, Future<Image>, List<int>, or Future<List<int>>'; throw AssertionError('must provide a Finder, Image, Future<Image>, List<int>, or Future<List<int>>');
} }
final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized() as TestWidgetsFlutterBinding; final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized() as TestWidgetsFlutterBinding;
return binding.runAsync<String?>(() async { return binding.runAsync<String?>(() async {
final ui.Image? image = await imageFuture; final ui.Image? image = await imageFuture;
if (image == null) { if (image == null) {
throw 'Future<Image> completed to null'; throw AssertionError('Future<Image> completed to null');
} }
final ByteData? bytes = await image.toByteData(format: ui.ImageByteFormat.png); final ByteData? bytes = await image.toByteData(format: ui.ImageByteFormat.png);
if (bytes == null) if (bytes == null)
......
...@@ -14,7 +14,8 @@ import 'package:flutter/scheduler.dart'; ...@@ -14,7 +14,8 @@ import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'package:stack_trace/stack_trace.dart' as stack_trace; import 'package:stack_trace/stack_trace.dart' as stack_trace;
import 'package:test_api/test_api.dart' as test_package; // ignore: deprecated_member_use import 'package:test_api/expect.dart' show fail;
import 'package:test_api/test_api.dart' as test_package show Timeout; // ignore: deprecated_member_use
import 'package:vector_math/vector_math_64.dart'; import 'package:vector_math/vector_math_64.dart';
import '_binding_io.dart' if (dart.library.html) '_binding_web.dart' as binding; import '_binding_io.dart' if (dart.library.html) '_binding_web.dart' as binding;
...@@ -1005,11 +1006,11 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { ...@@ -1005,11 +1006,11 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
assert(() { assert(() {
if (_pendingAsyncTasks == null) if (_pendingAsyncTasks == null)
return true; return true;
throw test_package.TestFailure( fail(
'Reentrant call to runAsync() denied.\n' 'Reentrant call to runAsync() denied.\n'
'runAsync() was called, then before its future completed, it ' 'runAsync() was called, then before its future completed, it '
'was called again. You must wait for the first returned future ' 'was called again. You must wait for the first returned future '
'to complete before calling runAsync() again.' 'to complete before calling runAsync() again.'
); );
}()); }());
...@@ -1573,11 +1574,11 @@ class LiveTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { ...@@ -1573,11 +1574,11 @@ class LiveTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
assert(() { assert(() {
if (!_runningAsyncTasks) if (!_runningAsyncTasks)
return true; return true;
throw test_package.TestFailure( fail(
'Reentrant call to runAsync() denied.\n' 'Reentrant call to runAsync() denied.\n'
'runAsync() was called, then before its future completed, it ' 'runAsync() was called, then before its future completed, it '
'was called again. You must wait for the first returned future ' 'was called again. You must wait for the first returned future '
'to complete before calling runAsync() again.' 'to complete before calling runAsync() again.'
); );
}()); }());
......
...@@ -29,7 +29,7 @@ class _BufferGoldenMatcher extends AsyncMatcher { ...@@ -29,7 +29,7 @@ class _BufferGoldenMatcher extends AsyncMatcher {
} else if (item is Future<List<int>>) { } else if (item is Future<List<int>>) {
buffer = Uint8List.fromList(await item); buffer = Uint8List.fromList(await item);
} else { } else {
throw 'Expected `List<int>` or `Future<List<int>>`, instead found: ${item.runtimeType}'; throw AssertionError('Expected `List<int>` or `Future<List<int>>`, instead found: ${item.runtimeType}');
} }
final Uri testNameUri = goldenFileComparator.getTestUri(key, version); final Uri testNameUri = goldenFileComparator.getTestUri(key, version);
if (autoUpdateGoldenFiles) { if (autoUpdateGoldenFiles) {
......
...@@ -620,7 +620,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker ...@@ -620,7 +620,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
assert(() { assert(() {
final TestWidgetsFlutterBinding widgetsBinding = binding; final TestWidgetsFlutterBinding widgetsBinding = binding;
return widgetsBinding is LiveTestWidgetsFlutterBinding && return widgetsBinding is LiveTestWidgetsFlutterBinding &&
widgetsBinding.framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark; widgetsBinding.framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark;
}()); }());
dynamic caughtException; dynamic caughtException;
...@@ -632,7 +632,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker ...@@ -632,7 +632,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
await idle(); await idle();
if (caughtException != null) { if (caughtException != null) {
throw caughtException as Object; throw caughtException as Object; // ignore: only_throw_errors, rethrowing caught exception.
} }
} }
...@@ -650,10 +650,12 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker ...@@ -650,10 +650,12 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
final WidgetsBinding binding = this.binding; final WidgetsBinding binding = this.binding;
if (binding is LiveTestWidgetsFlutterBinding && if (binding is LiveTestWidgetsFlutterBinding &&
binding.framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark) { binding.framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark) {
throw 'When using LiveTestWidgetsFlutterBindingFramePolicy.benchmark, ' test_package.fail(
'hasScheduledFrame is never set to true. This means that pumpAndSettle() ' 'When using LiveTestWidgetsFlutterBindingFramePolicy.benchmark, '
'cannot be used, because it has no way to know if the application has ' 'hasScheduledFrame is never set to true. This means that pumpAndSettle() '
'stopped registering new frames.'; 'cannot be used, because it has no way to know if the application has '
'stopped registering new frames.',
);
} }
return true; return true;
}()); }());
......
...@@ -109,7 +109,7 @@ void main() { ...@@ -109,7 +109,7 @@ void main() {
Uri.parse('/foo/bar/'), Uri.parse('/foo/bar/'),
); );
TestAsyncUtils.verifyAllScopesClosed(); TestAsyncUtils.verifyAllScopesClosed();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
expectSync(lines[0], 'Asynchronous call to guarded function leaked.'); expectSync(lines[0], 'Asynchronous call to guarded function leaked.');
......
...@@ -703,7 +703,7 @@ class _FakeComparator implements GoldenFileComparator { ...@@ -703,7 +703,7 @@ class _FakeComparator implements GoldenFileComparator {
case _ComparatorBehavior.returnFalse: case _ComparatorBehavior.returnFalse:
return Future<bool>.value(false); return Future<bool>.value(false);
case _ComparatorBehavior.throwTestFailure: case _ComparatorBehavior.throwTestFailure:
throw TestFailure('fake message'); fail('fake message');
} }
} }
......
...@@ -9,7 +9,7 @@ void main() { ...@@ -9,7 +9,7 @@ void main() {
test('stack manipulation: reportExpectCall', () { test('stack manipulation: reportExpectCall', () {
try { try {
expect(false, isTrue); expect(false, isTrue);
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} catch (e, stack) { } catch (e, stack) {
final List<DiagnosticsNode> information = <DiagnosticsNode>[]; final List<DiagnosticsNode> information = <DiagnosticsNode>[];
expect(reportExpectCall(stack, information), 4); expect(reportExpectCall(stack, information), 4);
...@@ -19,12 +19,8 @@ void main() { ...@@ -19,12 +19,8 @@ void main() {
expect(lines[1], matches(r'^ .*stack_manipulation_test.dart line [0-9]+$')); expect(lines[1], matches(r'^ .*stack_manipulation_test.dart line [0-9]+$'));
} }
try { final List<DiagnosticsNode> information = <DiagnosticsNode>[];
throw Object(); expect(reportExpectCall(StackTrace.current, information), 0);
} catch (e, stack) { expect(information, isEmpty);
final List<DiagnosticsNode> information = <DiagnosticsNode>[];
expect(reportExpectCall(stack, information), 0);
expect(information, isEmpty);
}
}); });
} }
...@@ -29,9 +29,13 @@ class TestAPISubclass extends TestAPI { ...@@ -29,9 +29,13 @@ class TestAPISubclass extends TestAPI {
} }
} }
class RecognizableTestException implements Exception {
const RecognizableTestException();
}
Future<Object> _guardedThrower() { Future<Object> _guardedThrower() {
return TestAsyncUtils.guard<Object>(() async { return TestAsyncUtils.guard<Object>(() async {
throw 'Hello'; throw const RecognizableTestException();
}); });
} }
...@@ -42,7 +46,7 @@ void main() { ...@@ -42,7 +46,7 @@ void main() {
f1 = testAPI.testGuard1(); f1 = testAPI.testGuard1();
try { try {
f2 = testAPI.testGuard2(); f2 = testAPI.testGuard2();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Guarded function conflict.'); real_test.expect(lines[0], 'Guarded function conflict.');
...@@ -64,7 +68,7 @@ void main() { ...@@ -64,7 +68,7 @@ void main() {
f1 = testAPI.testGuard1(); f1 = testAPI.testGuard1();
try { try {
f2 = testAPI.testGuard2(); f2 = testAPI.testGuard2();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Guarded function conflict.'); real_test.expect(lines[0], 'Guarded function conflict.');
...@@ -86,7 +90,7 @@ void main() { ...@@ -86,7 +90,7 @@ void main() {
f1 = testAPI.testGuard1(); f1 = testAPI.testGuard1();
try { try {
f2 = testAPI.testGuard3(); f2 = testAPI.testGuard3();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Guarded function conflict.'); real_test.expect(lines[0], 'Guarded function conflict.');
...@@ -108,7 +112,7 @@ void main() { ...@@ -108,7 +112,7 @@ void main() {
f1 = testAPI.testGuard1(); f1 = testAPI.testGuard1();
try { try {
flutter_test.expect(0, 0); flutter_test.expect(0, 0);
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Guarded function conflict.'); real_test.expect(lines[0], 'Guarded function conflict.');
...@@ -129,7 +133,7 @@ void main() { ...@@ -129,7 +133,7 @@ void main() {
try { try {
f1 = tester.pump(); f1 = tester.pump();
f2 = tester.pump(); f2 = tester.pump();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Guarded function conflict.'); real_test.expect(lines[0], 'Guarded function conflict.');
...@@ -171,7 +175,7 @@ void main() { ...@@ -171,7 +175,7 @@ void main() {
try { try {
f1 = tester.pump(); f1 = tester.pump();
TestAsyncUtils.verifyAllScopesClosed(); TestAsyncUtils.verifyAllScopesClosed();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Asynchronous call to guarded function leaked.'); real_test.expect(lines[0], 'Asynchronous call to guarded function leaked.');
...@@ -196,7 +200,7 @@ void main() { ...@@ -196,7 +200,7 @@ void main() {
try { try {
f1 = tester.pump(); f1 = tester.pump();
TestAsyncUtils.verifyAllScopesClosed(); TestAsyncUtils.verifyAllScopesClosed();
throw 'unexpectedly did not throw'; fail('unexpectedly did not throw');
} on FlutterError catch (e) { } on FlutterError catch (e) {
final List<String> lines = e.message.split('\n'); final List<String> lines = e.message.split('\n');
real_test.expect(lines[0], 'Asynchronous call to guarded function leaked.'); real_test.expect(lines[0], 'Asynchronous call to guarded function leaked.');
...@@ -220,8 +224,8 @@ void main() { ...@@ -220,8 +224,8 @@ void main() {
try { try {
await _guardedThrower(); await _guardedThrower();
expect(false, true); // _guardedThrower should throw and we shouldn't reach here expect(false, true); // _guardedThrower should throw and we shouldn't reach here
} on String catch (s) { } on RecognizableTestException catch (e) {
expect(s, 'Hello'); expect(e, const RecognizableTestException());
} }
}); });
......
...@@ -7,12 +7,16 @@ import 'dart:ui' as ui; ...@@ -7,12 +7,16 @@ import 'dart:ui' as ui;
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
class RecognizableTestException implements Exception {
const RecognizableTestException();
}
class TestDelegate extends BinaryMessenger { class TestDelegate extends BinaryMessenger {
@override @override
Future<ByteData?>? send(String channel, ByteData? message) async { Future<ByteData?>? send(String channel, ByteData? message) async {
expect(channel, ''); expect(channel, '');
expect(message, isNull); expect(message, isNull);
throw 'Vic Fontaine'; throw const RecognizableTestException();
} }
// Rest of the API isn't needed for this test. // Rest of the API isn't needed for this test.
...@@ -32,7 +36,7 @@ void main() { ...@@ -32,7 +36,7 @@ void main() {
await TestDefaultBinaryMessenger(delegate).send('', null); await TestDefaultBinaryMessenger(delegate).send('', null);
expect(true, isFalse); // should not reach here expect(true, isFalse); // should not reach here
} catch (error) { } catch (error) {
expect(error, 'Vic Fontaine'); expect(error, const RecognizableTestException());
} }
}); });
} }
...@@ -6,6 +6,7 @@ linter: ...@@ -6,6 +6,7 @@ linter:
curly_braces_in_flow_control_structures: true curly_braces_in_flow_control_structures: true
library_private_types_in_public_api: false # Tool does not have any public API. library_private_types_in_public_api: false # Tool does not have any public API.
no_runtimeType_toString: false # We use runtimeType for debugging in the tool. no_runtimeType_toString: false # We use runtimeType for debugging in the tool.
only_throw_errors: false # For historical reasons we throw strings a lot.
prefer_relative_imports: true prefer_relative_imports: true
public_member_api_docs: false # Tool does not have any public API. public_member_api_docs: false # Tool does not have any public API.
unawaited_futures: true unawaited_futures: true
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment