testfx icon indicating copy to clipboard operation
testfx copied to clipboard

Assert.IComparable not compatible with AzDoReport extension

Open nohwnd opened this issue 2 months ago • 5 comments

Describe the bug

Image

Assert.IComparable is not compatible with azdo reporter plugin, where we skip *Assert.cs files to avoid pointing at the assertion implementation (for a lack of better metadata).

Here we don't skip it because the file name does not end with Assert.cs

https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/AzureDevOpsReporter.cs#L216-L224

Expected behavior

We see the assertion usage, not the assertion implementation.

Actual behavior

The opposite.

nohwnd avatar Nov 11 '25 13:11 nohwnd

Just add StackTraceHidden to Assert class and boom? (won't work for .NET Framework though)

Youssef1313 avatar Nov 11 '25 13:11 Youssef1313

I agree with @Youssef1313. I definitely understand the idea to improve stack for users but I prefer displaying too much than having the current filter. It's too broad filter and could lead to removing user code lines from stack.

Evangelink avatar Nov 16 '25 10:11 Evangelink

What part do you agree with? :) The proposal from Youssef will hide it from anywhere, but then you say you prefer showing the whole stack.

The naming convention that the AzDO plugin uses achieves both. It walks down the stack trace until it finds a location that does not end with Assert.cs, so we point to the error in user code, instead of pointing to the implementation of the assertion. The goal is that the newly changed code in the PR gets annotated correctly, with correctly placed error pointing at the changed file, rather than pointing at the assertion detail where we did not make any change.

We don't lose information from the stack trace, we don't filter the stack trace, we just tell GH to point to the place that we think is more likely in user code.

nohwnd avatar Nov 18 '25 10:11 nohwnd

I think @Evangelink is referring to some code that manually altered stack traces back in MSTest 3 that was removed in 4.x (https://github.com/microsoft/testfx/pull/6171)

Youssef1313 avatar Nov 18 '25 10:11 Youssef1313

btw we check if the file is on disk, so we are the ones that see it point to assertion details, users won't

nohwnd avatar Nov 18 '25 11:11 nohwnd