Skip to content
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

DebuggerDisplay Stopwatch #65753

Merged
merged 4 commits into from
Jun 7, 2022
Merged

DebuggerDisplay Stopwatch #65753

merged 4 commits into from
Jun 7, 2022

Conversation

I-SER-I
Copy link
Contributor

@I-SER-I I-SER-I commented Feb 23, 2022

close #65724

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 23, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@I-SER-I I-SER-I changed the title feat: add DebuggerDisplay method and attribute DebuggerDisplay Stopwatch Feb 23, 2022
@ghost
Copy link

ghost commented Feb 23, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

close #65724

Author: I-SER-I
Assignees: -
Labels:

area-System.Diagnostics, community-contribution

Milestone: -

@deeprobin
Copy link
Contributor

@danmoseley Are tests for debugger displays required?

@I-SER-I I-SER-I requested a review from danmoseley March 14, 2022 09:15
@danmoseley
Copy link
Member

danmoseley commented Mar 15, 2022

@danmoseley Are tests for debugger displays required?

yes, I've seen cases where a debugger display threw an exception, like any code. In fact, that's a good reason to put it in a private property like this, so it's easier to test.

@I-SER-I perhaps you could add a simple test that creates a Stopwatch, verifies (through reflection) that its DebuggerDisplay property string matches $"{sw.Elapsed} (IsRunning = {sw.IsRunning})", and maybe again with it started?

@stephentoub
Copy link
Member

verifies (through reflection)

internal static string ValidateDebuggerDisplayReferences(object obj)

e.g.
Assert.Equal($"Key = {keyString}", DebuggerAttributes.ValidateDebuggerDisplayReferences(grouping));

@danmoseley danmoseley assigned danmoseley and unassigned danmoseley Mar 15, 2022
@I-SER-I
Copy link
Contributor Author

I-SER-I commented Mar 28, 2022

@danmoseley, Could you help me where I need to implement DebuggerDisplayTest?
Because I didn't find a StopwatchTest

@I-SER-I
Copy link
Contributor Author

I-SER-I commented Apr 11, 2022

@danmoseley, Could you help me where I need to implement DebuggerDisplayTest? Because I didn't find a StopwatchTest

@stephentoub )

@deeprobin
Copy link
Contributor

@I-SER-I
I think you have to create a new test class in src/libraries/System.Runtime/tests with a test validating the result of the DebuggerDisplay.

@danmoseley
Copy link
Member

Not sure how I didn't see the notification. Anyway, yes, what @deeprobin said. The reason the tests go there is that System.Runtime is where the type is exposed (https://github.com/dotnet/runtime/blob/4225cb0fff263e9fbc7ad007ab18e774a1b73d1e/src/libraries/System.Runtime/ref/System.Runtime.cs#L7345)

@danmoseley
Copy link
Member

I'll add the test.

@danmoseley
Copy link
Member

@steveisok any idea what's going on iwth the mono minjit lane?

@danmoseley
Copy link
Member

The test has passed across wasm, devices, all OS, AOT... I think the failure of the whole minijit lane does not materially affect coverage.

@danmoseley danmoseley merged commit 625b8ce into dotnet:main Jun 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Diagnostics community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DebuggerDisplay for System.Diagnostics.Stopwatch
4 participants