Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jun 17, 2022

#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.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 17, 2022
Jonah Williams added 3 commits June 16, 2022 21:35
@jonahwilliams jonahwilliams marked this pull request as ready for review June 17, 2022 04:59
Jonah Williams added 2 commits June 16, 2022 23:15
// found in the LICENSE file.

// @dart = 2.8

Copy link
Member

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.

Copy link
Contributor Author

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}'");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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}'");

Copy link
Contributor Author

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;
Copy link
Member

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

Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be late

Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late

Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
late bool supportsHotReload;
bool supportsHotReload = false;

Copy link
Member

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
late bool supportsHotRestart;
bool supportsHotRestart = false;

Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late

Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late

Copy link
Contributor Author

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;
Copy link
Member

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 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

@jonahwilliams jonahwilliams left a 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}'");
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 68 to 69
MemoryFileSystem? memoryFileSystem;
FakePlatform? fakePlatform;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@jmagman jmagman left a 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more.

Copy link
Contributor Author

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;
Copy link
Member

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();
Copy link
Member

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>()

@jonahwilliams
Copy link
Contributor Author

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

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 18, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 19, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 20, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 20, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 20, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 21, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 21, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 21, 2022
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants