search

matkoch / resharper-verify

23 stars 4 forks source link

Use full path for verified files #6

Closed SimonCropp closed 2 years ago

SimonCropp commented 2 years ago

fixes #5 fixes #4

SimonCropp commented 2 years ago

good to go?

matkoch commented 2 years ago

If I understand correctly, then the new implementation has to parse the full exception output to gather received/accepted file names (they're now at the end of the exception?). This should work, but it does add some unpredictable performance hit.

SimonCropp commented 2 years ago

This should work, but it does add some unpredictable performance hit.

OK. i wasnt aware it was expensive to parse the exception message. how about i move the to be near the top. ie full path directly below each partial path?

matkoch commented 2 years ago

If it was guaranteed to be always at the very top, that would be awesome :)

SimonCropp commented 2 years ago

can we access the exception data property? then we could access the paths with a key lookup

SimonCropp commented 2 years ago

@matkoch bump on my last comment

SimonCropp commented 2 years ago

or can we access custom properties?

matkoch commented 2 years ago

Seems that only exception.ToString() is available :/

SimonCropp commented 2 years ago

how do i explicitly call ToString?

SimonCropp commented 2 years ago

is there any docs for UnitTestResultData.GetExceptionChunk ?

SimonCropp commented 2 years ago

i think we need an alternative to using the exception message. the exception message is displayed to the user, so it needs to be human readable and make sense. with the amount of information we need to pass to the addin, it pollutes the message displayed to the user. and not that we have not handled a few edge cases yet, eg tests that produce multiple snapshot files and deletion of redundant snapshots.

can the sdk be extended to have a feature where we can use Exception.Data or a custom property to pass a key/value list from the test to the addin?

matkoch commented 2 years ago

I forwarded your slack message. Can't really comment on that.

My thinking is that anything that would go into the Data property could as well be printed via exception.ToString(). Even if there are multiple files involved, wouldn't you want to show this as part of the message to let the user know?

SimonCropp commented 2 years ago

wouldn't you want to show this as part of the message to let the user know?

yes but showing that in a way that is human readable and machine parsable is difficult.

SimonCropp commented 2 years ago

raised a feature request in the plugin api https://youtrack.jetbrains.com/issue/RSPL-6990

matkoch commented 2 years ago

lets wait for that then?

SimonCropp commented 2 years ago

@matkoch eta? days/weeks/months?

SimonCropp commented 2 years ago

@matkoch and can i help? eg sign an nda and implement it myself?

matkoch commented 2 years ago

youtrack issue would be a better place to ask, but i'm afraid the real bottleneck is the release cadence.