-
Notifications
You must be signed in to change notification settings - Fork 29.7k
migrate more unit tests to null safety #106153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| // found in the LICENSE file. | ||
|
|
||
| // @dart = 2.8 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra lines in a lot of these files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| final dynamic data = jsonDecode(versionFile.readAsStringSync()); | ||
| if (data is! Map<String, Object>) { | ||
| if (data is! Map<String, Object?>) { | ||
| throw Exception("Expected object of type 'Map<String, Object>' but got one of type '${data.runtimeType}'"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| throw Exception("Expected object of type 'Map<String, Object>' but got one of type '${data.runtimeType}'"); | |
| throw Exception("Expected object of type 'Map<String, Object?>' but got one of type '${data.runtimeType}'"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| FileSystem fileSystem; | ||
| Artifacts artifacts; | ||
| Logger logger; | ||
| FakeProcessManager? processManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this should be late
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doine
| Environment environment; | ||
| FileSystem fileSystem; | ||
| late Environment environment; | ||
| FileSystem? fileSystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be late
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| Artifacts artifacts; | ||
| FileSystem fileSystem; | ||
| Logger logger; | ||
| FakeProcessManager? processManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
late
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| @override | ||
| bool supportsHotReload; | ||
| late bool supportsHotReload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| late bool supportsHotReload; | |
| bool supportsHotReload = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump
|
|
||
| @override | ||
| bool supportsHotRestart; | ||
| late bool supportsHotRestart; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| late bool supportsHotRestart; | |
| bool supportsHotRestart = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| FileSystem fs; | ||
| FakeFlutterProject flutterProject; | ||
| FakeFlutterManifest flutterManifest; | ||
| FileSystem? fs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
late
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| void main() { | ||
| FileSystem fileSystem; | ||
| FileSystem? fileSystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
late
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| BufferLogger logger; | ||
| FileSystem fs; | ||
| late BufferLogger logger; | ||
| FileSystem? fs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
late
I'm running out of steam here, please audit the rest 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I got em all
| final dynamic data = jsonDecode(versionFile.readAsStringSync()); | ||
| if (data is! Map<String, Object>) { | ||
| if (data is! Map<String, Object?>) { | ||
| throw Exception("Expected object of type 'Map<String, Object>' but got one of type '${data.runtimeType}'"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| Environment environment; | ||
| FileSystem fileSystem; | ||
| late Environment environment; | ||
| FileSystem? fileSystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| FileSystem fileSystem; | ||
| Artifacts artifacts; | ||
| Logger logger; | ||
| FakeProcessManager? processManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doine
| Artifacts artifacts; | ||
| FileSystem fileSystem; | ||
| Logger logger; | ||
| FakeProcessManager? processManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| late Environment androidEnvironment; | ||
| late Environment iosEnvironment; | ||
| late Artifacts artifacts; | ||
| FileSystem? fileSystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| FileSystem? fileSystem; | ||
|
|
||
| ProcessManager processManager; | ||
| ProcessManager? processManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| MemoryFileSystem? memoryFileSystem; | ||
| FakePlatform? fakePlatform; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| FileSystem fs; | ||
| FakeFlutterProject flutterProject; | ||
| FakeFlutterManifest flutterManifest; | ||
| FileSystem? fs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| void main() { | ||
| FileSystem fileSystem; | ||
| FileSystem? fileSystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| BufferLogger logger; | ||
| FileSystem fs; | ||
| late BufferLogger logger; | ||
| FileSystem? fs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nits, LGTM
| // found in the LICENSE file. | ||
|
|
||
| // @dart = 2.8 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| @override | ||
| bool supportsHotReload; | ||
| late bool supportsHotReload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump
| return <String, Object>{}; | ||
| } | ||
| return castStringKeyedMap(json.decode(file.readAsStringSync())); | ||
| return castStringKeyedMap(json.decode(file.readAsStringSync()))!.cast(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh I would have thought this would need to be .cast<String, Object>()
|
Both the google3 and customer testing errors I'm getting are stale. Actually the google3 test errors look like they're coming from the wrong PR |
#71511
Migrates unit tests that do not have any null unsafe code in their transitive closure. this allows the tests to correctly run in sound mode.