Skip to content

Add support for displaying coverage in CodeView#4700

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.

Sign up for GitHub

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
bkonyi merged 3 commits into from
Nov 5, 2022
Merged

Add support for displaying coverage in CodeView#4700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f1b625
Add support for displaying coverage in `CodeView`
bkonyi Nov 4, 2022
cb15451
Address comments
bkonyi Nov 4, 2022
10f96db
mockConnectedApp
bkonyi Nov 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions packages/devtools_app/lib/src/screens/debugger/codeview.dart
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,13 +346,16 @@ class _CodeViewState extends State<CodeView>
final pausedFrame = frame == null
? null
: (frame.scriptRef == scriptRef ? frame : null);

final currentParsedScript = parsedScript;
return Row(
children: [
ValueListenableBuilder<List<BreakpointAndSourcePosition>>(
valueListenable:
DualValueListenableBuilder<
List<BreakpointAndSourcePosition>, bool>(
firstListenable:
breakpointManager.breakpointsWithLocation,
builder: (context, breakpoints, _) {
secondListenable:
widget.codeViewController.showCodeCoverage,
builder: (context, breakpoints, showCodeCoverage, _) {
return Gutter(
gutterWidth: gutterWidth,
scrollController: gutterController,
Expand All@@ -362,14 +365,21 @@ class _CodeViewState extends State<CodeView>
breakpoints: breakpoints
.where((bp) => bp.scriptRef == scriptRef)
.toList(),
executableLines: parsedScript != null
? parsedScript!.executableLines
: <int>{},
executableLines:
currentParsedScript?.executableLines ??
const <int>{},
coverageHitLines:
currentParsedScript?.coverageHitLines ??
const <int>{},
coverageMissedLines:
currentParsedScript?.coverageMissedLines ??
const <int>{},
onPressed: _onPressed,
// Disable dots for possible breakpoint locations.
allowInteraction:
!(widget.debuggerController?.isSystemIsolate ??
false),
showCodeCoverage: showCodeCoverage,
);
},
),
Expand DownExpand Up@@ -547,6 +557,9 @@ class Gutter extends StatelessWidget {
required this.executableLines,
required this.onPressed,
required this.allowInteraction,
required this.coverageHitLines,
required this.coverageMissedLines,
required this.showCodeCoverage,
});

final double gutterWidth;
Expand All@@ -556,13 +569,17 @@ class Gutter extends StatelessWidget {
final StackFrameAndSourcePosition? pausedFrame;
final List<BreakpointAndSourcePosition> breakpoints;
final Set<int> executableLines;
final Set<int> coverageHitLines;
final Set<int> coverageMissedLines;
final IntCallback onPressed;
final bool allowInteraction;
final bool showCodeCoverage;

@override
Widget build(BuildContext context) {
final bpLineSet = Set.from(breakpoints.map((bp) => bp.line));
final theme = Theme.of(context);
final coverageLines = coverageHitLines.union(coverageMissedLines);
return Container(
width: gutterWidth,
decoration: BoxDecoration(
Expand All@@ -575,13 +592,18 @@ class Gutter extends StatelessWidget {
itemCount: lineCount,
itemBuilder: (context, index) {
final lineNum = lineOffset + index + 1;
bool? coverageHit;
if (showCodeCoverage && coverageLines.contains(lineNum)) {
coverageHit = coverageHitLines.contains(lineNum);
}
return GutterItem(
lineNumber: lineNum,
onPressed: () => onPressed(lineNum),
isBreakpoint: bpLineSet.contains(lineNum),
isExecutable: executableLines.contains(lineNum),
isPausedHere: pausedFrame?.line == lineNum,
allowInteraction: allowInteraction,
coverageHit: coverageHit,
);
},
),
Expand All@@ -598,6 +620,7 @@ class GutterItem extends StatelessWidget {
required this.isPausedHere,
required this.onPressed,
required this.allowInteraction,
required this.coverageHit,
}) : super(key: key);

final int lineNumber;
Expand All@@ -608,6 +631,8 @@ class GutterItem extends StatelessWidget {

final bool allowInteraction;

final bool? coverageHit;

/// Whether the execution point is currently paused here.
final bool isPausedHere;

Expand All@@ -622,13 +647,20 @@ class GutterItem extends StatelessWidget {

final bpBoxSize = breakpointRadius * 2;
final executionPointIndent = scaleByFontFactor(10.0);

Color? color;
final hasCoverage = coverageHit;
if (hasCoverage != null && isExecutable) {
color = hasCoverage
? theme.colorScheme.coverageHitColor
: theme.colorScheme.coverageMissColor;
}
return InkWell(
onTap: onPressed,
// Force usage of default mouse pointer when gutter interaction is
// disabled.
mouseCursor: allowInteraction ? null : SystemMouseCursors.basic,
child: Container(
color: color,
height: CodeView.rowHeight,
padding: const EdgeInsets.only(right: 4.0),
child: Stack(
Expand Down
75 changes: 74 additions & 1 deletion packages/devtools_app/lib/src/screens/debugger/codeview_controller.dart
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,13 @@ class CodeViewController extends DisposableController
final ScriptsHistory scriptsHistory = ScriptsHistory();
late VoidCallback _scriptHistoryListener;

ValueListenable<bool> get showCodeCoverage => _showCodeCoverage;
final _showCodeCoverage = ValueNotifier<bool>(false);

void toggleShowCodeCoverage() {
_showCodeCoverage.value = !_showCodeCoverage.value;
}

void clearState() {
// It would be nice to not clear the script history but it is currently
// coupled to ScriptRef objects so that is unsafe.
Expand DownExpand Up@@ -119,6 +126,30 @@ class CodeViewController extends DisposableController
scriptsHistory.current.addListener(_scriptHistoryListener);
}

Future<void> refreshCodeCoverage() async {
final hitLines = <int>{};
final missedLines = <int>{};
final current = parsedScript.value;
if (current == null) {
return;
}
final isolateRef = serviceManager.isolateManager.selectedIsolate.value!;
await _getCoverageReport(
isolateRef,
current.script,
hitLines,
missedLines,
);

parsedScript.value = ParsedScript(
script: current.script,
highlighter: current.highlighter,
executableLines: current.executableLines,
coverageHitLines: hitLines,
coverageMissedLines: missedLines,
);
}

/// Resets the current script information before invoking [showScriptLocation].
void resetScriptLocation(ScriptLocation scriptLocation) {
_scriptLocation.value = null;
Expand All@@ -143,6 +174,30 @@ class CodeViewController extends DisposableController
_scriptLocation.value = scriptLocation;
}

Future<void> _getCoverageReport(
IsolateRef isolateRef,
ScriptRef script,
Set<int> hitLines,
Set<int> missedLines,
) async {
try {
final report = await serviceManager.service!.getSourceReport(
isolateRef.id!,
const [SourceReportKind.kCoverage],
scriptId: script.id!,
reportLines: true,
);
for (final range in report.ranges!) {
final coverage = range.coverage!;
hitLines.addAll(coverage.hits!);
missedLines.addAll(coverage.misses!);
}
} catch (e) {
// Ignore - not supported for all vm service implementations.
log('$e');
}
}

/// Parses the current script into executable lines and prepares the script
/// for syntax highlighting.
Future<void> _parseCurrentScript() async {
Expand All@@ -160,9 +215,12 @@ class CodeViewController extends DisposableController
// Gather the data to display breakable lines.
var executableLines = <int>{};

final hitLines = <int>{};
final missedLines = <int>{};

if (script != null) {
final isolateRef = serviceManager.isolateManager.selectedIsolate.value!;
try {
final isolateRef = serviceManager.isolateManager.selectedIsolate.value;
final positions = await breakpointManager.getBreakablePositions(
isolateRef,
script,
Expand All@@ -172,10 +230,20 @@ class CodeViewController extends DisposableController
// Ignore - not supported for all vm service implementations.
log('$e');
}

await _getCoverageReport(
isolateRef,
script,
hitLines,
missedLines,
);

parsedScript.value = ParsedScript(
script: script,
highlighter: highlighter,
executableLines: executableLines,
coverageHitLines: hitLines,
coverageMissedLines: missedLines,
);
}
}
Expand DownExpand Up@@ -261,6 +329,8 @@ class ParsedScript {
required this.script,
required this.highlighter,
required this.executableLines,
required this.coverageHitLines,
required this.coverageMissedLines,
}) : lines = (script.source?.split('\n') ?? const []).toList();

final Script script;
Expand All@@ -269,6 +339,9 @@ class ParsedScript {

final Set<int> executableLines;

final Set<int> coverageHitLines;
final Set<int> coverageMissedLines;

final List<String> lines;

int get lineCount => lines.length;
Expand Down
56 changes: 55 additions & 1 deletion packages/devtools_app/lib/src/screens/debugger/controls.dart
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,7 +18,7 @@ import 'debugger_controller.dart';
class DebuggingControls extends StatefulWidget {
const DebuggingControls({Key? key}) : super(key: key);

static const minWidthBeforeScaling = 1300.0;
static const minWidthBeforeScaling = 1600.0;

@override
_DebuggingControlsState createState() => _DebuggingControlsState();
Expand DownExpand Up@@ -53,6 +53,8 @@ class _DebuggingControlsState extends State<DebuggingControls>
_stepButtons(canStep: canStep),
const SizedBox(width: denseSpacing),
BreakOnExceptionsControl(controller: controller),
const SizedBox(width: denseSpacing),
CodeCoverageToggle(controller: controller),
const Expanded(child: SizedBox(width: denseSpacing)),
_librariesButton(),
],
Expand DownExpand Up@@ -138,6 +140,58 @@ class _DebuggingControlsState extends State<DebuggingControls>
}
}

class CodeCoverageToggle extends StatelessWidget {
const CodeCoverageToggle({
super.key,
required this.controller,
});

final DebuggerController controller;

@override
Widget build(BuildContext context) {
return RoundedOutlinedBorder(
Comment thread
kenzieschmoll marked this conversation as resolved.
child: ValueListenableBuilder<bool>(
valueListenable: controller.codeViewController.showCodeCoverage,
builder: (context, selected, _) {
final isInSmallMode = MediaQuery.of(context).size.width <
DebuggingControls.minWidthBeforeScaling;
return Row(
children: [
ToggleButton(
label: isInSmallMode ? null : 'Show Coverage',
message: 'Show code coverage',
icon: Codicons.checklist,
isSelected: selected,
outlined: false,
shape: const RoundedRectangleBorder(

@kenzieschmollkenzieschmollNov 4, 2022•
edited
Loading

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

@bkonyibkonyiNov 4, 2022

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

borderRadius: BorderRadius.only(
topLeft: Radius.circular(4),
bottomLeft: Radius.circular(4),
),
),
onPressed: controller.codeViewController.toggleShowCodeCoverage,
),
LeftBorder(
child: IconLabelButton(
label: '',
tooltip: 'Refresh code coverage statistics',
outlined: false,
onPressed: selected
? controller.codeViewController.refreshCodeCoverage
: null,
minScreenWidthForTextBeforeScaling: 20000,
icon: Icons.refresh,
),
),
],
);
},
),
);
}
}

class BreakOnExceptionsControl extends StatelessWidget {
const BreakOnExceptionsControl({
Key? key,
Expand Down
2 changes: 1 addition & 1 deletion ...ls_app/lib/src/screens/memory/panes/allocation_profile/allocation_profile_table_view.dart
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -391,7 +391,7 @@ class _RefreshOnGCToggleButton extends StatelessWidget {
builder: (context, refreshOnGc, _) {
return ToggleButton(
message: 'Auto-refresh on garbage collection',
label: const Text('Refresh on GC'),
label: 'Refresh on GC',
icon: Icons.autorenew_outlined,
isSelected: refreshOnGc,
onPressed: allocationProfileController.toggleRefreshOnGc,
Expand Down
Loading