Skip to content

Add support for displaying coverage in CodeView - #4700

Merged
bkonyi merged 3 commits into
flutter:masterfrom
bkonyi:code_coverage
Nov 5, 2022
Merged

Add support for displaying coverage in CodeView#4700
bkonyi merged 3 commits into
flutter:masterfrom
bkonyi:code_coverage

Conversation

@bkonyi

Copy link
Copy Markdown
Contributor

This changes adds a toggle to the debugger screen that shows coverage information for the currently selected script.

Fixes#3862

Coverage enabled:
Screen Shot 2022-11-04 at 11 12 41 AM

Coverage disabled:
Screen Shot 2022-11-04 at 11 12 56 AM

This changes adds a toggle to the debugger screen that shows coverage
information for the currently selected script.
Fixesflutter#3862
@bkonyi
bkonyi requested review from a team and elliette as code ownersNovember 4, 2022 15:22
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview.dart Outdated
final executionPointIndent = scaleByFontFactor(10.0);

Color? color;
if (coverageHit != null && isExecutable) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is the isExecutable check enough here? In your screenshot it looked like a line with only an end bracket } came back as executable (it was highlighted in red)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yeah, I don't think it is. I'll look at what Observatory does.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Observatory is using some private fields to further refine what should be included in coverage information, but this change at least won't include the end bracket anymore.

Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview_controller.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview_controller.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview_controller.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/codeview_controller.dart Outdated
Comment threadpackages/devtools_app/lib/src/screens/debugger/controls.dart
Comment threadpackages/devtools_app/lib/src/shared/common_widgets.dart Outdated
Comment threadpackages/devtools_app/lib/src/shared/theme.dart Outdated
@bkonyi
bkonyi requested a review from polina-c as a code ownerNovember 4, 2022 18:10
@bkonyi

Copy link
Copy Markdown
ContributorAuthor

Updated screenshot:
image

@bkonyi

Copy link
Copy Markdown
ContributorAuthor

With the proper "small mode" behavior:

image

isSelected: selected,
hideBorder: true,
outlined: false,
shape: const RoundedRectangleBorder(

@kenzieschmollkenzieschmollNov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do we have to manually add this for this button and not for all the other buttons in DevTools? OutlinedButton should have this by default

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

We only need this since it's part of a "group" of buttons with the refresh button on the right. If we don't do this and we're showing code coverage, the highlighted background won't fill the control completely.

With the custom shape:

image

Without (notice the rounded edges by the refresh button):

image

@bkonyi
bkonyi merged commit 56369ea into flutter:masterNov 5, 2022
@bkonyi
bkonyi deleted the code_coverage branch November 5, 2022 01:26

@trunghieuvntrunghieuvn left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Sign up for freeto 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.

DevTools debugger pane should display basic code coverage details

3 participants

@bkonyi@trunghieuvn@kenzieschmoll