[cupertino_ui] Re-enable tab_scaffold_test.dart#12064
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the skip annotation from tab_scaffold_test.dart, updates its imports, and introduces a new test utility file widget_inspector_test_utils.dart containing DispatchedEventKey and TestWidgetInspectorService. Feedback suggests removing an unused import in the new utility file and replacing it with package:flutter/widgets.dart. Additionally, it is recommended to use .toString() instead of a direct cast to String in testBoolExtension to handle boolean values safely.
|
|
||
| import 'dart:convert'; | ||
|
|
||
| import 'package:cupertino_ui/cupertino_ui.dart'; |
There was a problem hiding this comment.
The import of package:cupertino_ui/cupertino_ui.dart is unused in this test utility file. To avoid unnecessary coupling and keep the test utilities clean, import package:flutter/widgets.dart instead, which provides WidgetInspectorService and WidgetsBinding.
| import 'package:cupertino_ui/cupertino_ui.dart'; | |
| import 'package:flutter/widgets.dart'; |
References
- Unused imports should be avoided to keep the codebase clean and maintainable, adhering to Dart's standard lint rules (unused_import). (link)
There was a problem hiding this comment.
No it's definitely used.
| Future<String> testBoolExtension(String name, Map<String, String> arguments) async { | ||
| expect(extensions, contains(name)); | ||
| // Encode and decode to JSON to match behavior using a real service | ||
| // extension where only JSON is allowed. | ||
| return (json.decode(json.encode(await extensions[name]!(arguments))) | ||
| as Map<String, dynamic>)['enabled'] | ||
| as String; | ||
| } |
There was a problem hiding this comment.
The testBoolExtension method casts the 'enabled' value directly to String using as String. However, some service extensions return actual boolean values (e.g., true or false) rather than string representations. To make this utility more robust and prevent runtime TypeErrors, convert the value to a string using .toString().
Future<String> testBoolExtension(String name, Map<String, String> arguments) async {
expect(extensions, contains(name));
// Encode and decode to JSON to match behavior using a real service
// extension where only JSON is allowed.
final Map<String, dynamic> result = json.decode(json.encode(await extensions[name]!(arguments))) as Map<String, dynamic>;
return result['enabled']?.toString() ?? 'false';
}There was a problem hiding this comment.
Sounds reasonable, though I know you just copied this code, so it's fine either way.
|
|
||
| import 'dart:convert'; | ||
|
|
||
| import 'package:cupertino_ui/cupertino_ui.dart'; |
| Future<String> testBoolExtension(String name, Map<String, String> arguments) async { | ||
| expect(extensions, contains(name)); | ||
| // Encode and decode to JSON to match behavior using a real service | ||
| // extension where only JSON is allowed. | ||
| return (json.decode(json.encode(await extensions[name]!(arguments))) | ||
| as Map<String, dynamic>)['enabled'] | ||
| as String; | ||
| } |
There was a problem hiding this comment.
Sounds reasonable, though I know you just copied this code, so it's fine either way.
This test re-enables
tab_scaffold_test.dart, and brings overwidget_inspector_test_utils.dartintotests/.Pre-Review Checklist
[shared_preferences]///).