Skip to content

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
@ghost
Copy link

ghost commented Feb 23, 2022

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