Skip to content

Migrate debugger_test and instance_test to null-safety #1708

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

Merged
merged 9 commits into from
Aug 8, 2022

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Aug 1, 2022

I did not refactor away the null-assert !s added by the Dart migration tool. Because this is a test, the null-assert !s should catch any regressions if something we expect to be non-null returns null.

@elliette elliette changed the title Migrate debugger_test, inspector_test, and instance_test to null-safety Migrate debugger_test and instance_test to null-safety Aug 8, 2022
@elliette elliette marked this pull request as ready for review August 8, 2022 16:37
@elliette elliette requested a review from annagrin August 8, 2022 16:37
@@ -163,29 +161,4 @@ void main() async {
expect(frames[3].kind, FrameKind.kAsyncSuspensionMarker);
expect(frames[4].kind, FrameKind.kAsyncCausal);
});

group('errors', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed because all this was checking was whether an error would be thrown if a null value was passed as a parameter (when a null value wasn't expected). With null-safety, this isn't possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea was to test that the error message is shown, passing the null was just the way to trigger it. Is there any other way to trigger the errors in the debugger? Maybe throwing somewhere in the fake inspector code? Or by introducing a fake stack computer that throws when calculating frames?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test back in, thanks!

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

Thanks Elliott! Left a comment about the test removal. Would be good to find a way to keep it.

@@ -163,29 +161,4 @@ void main() async {
expect(frames[3].kind, FrameKind.kAsyncSuspensionMarker);
expect(frames[4].kind, FrameKind.kAsyncCausal);
});

group('errors', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea was to test that the error message is shown, passing the null was just the way to trigger it. Is there any other way to trigger the errors in the debugger? Maybe throwing somewhere in the fake inspector code? Or by introducing a fake stack computer that throws when calculating frames?

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM!

@elliette elliette merged commit e34f5b7 into dart-lang:master Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants