Assert.AreEqual string: consider displaying the full actual and expected in addition to the diff

[Evangelink]

Summary

Let's take the following error diff:

Assert.AreEqual failed. Expected string length 199 but was 139. 'expected' expression: 'Normalize(expected)', 'actual' expression: 'Normalize(actual)'.
Expected: "...n 1; }␊    #endregion␊␊    #region ..."
But was:  "...n 1; }␊    #endregion␊    #endregion␊}"

This is nice for spotting the exact difference but it's sometimes interesting to have access to the full context.

For examples:

1. When you are in a scenario where you are changing the output, it's useful to be able to copy the new expected content to replace current expected.

2. When there are more than one issue, you would need to run the assert multiple times when having the full actual/expected might allow you to do an extra check upfront.

[nohwnd]

I would love to see this solved by showing actual diff in the commandline and in UI.

In commandline by using tool similar to diffplex, or as diff from git, or something similar.

that gives us not only the whole text, but also multiple locations of where the difference is found, similarities etc.

I can see that as great value added especially for the outuput tests you mention, because pasting it in diff tool is exactly what I end up doing manually.

[Evangelink]

It would indeed be interesting to have an extensibility point in MSTest where we could register a diff tool. Diffplex looks pretty awesome, but the apache license is problematic to make it a default.

Maybe we could add the actual and expected to the exception and update Test Explorer to render that (cc @drognanar for complexity) and for MTP CLI, we could maybe display it or not based on output details mode.

[Evangelink]

While waiting for the diff solution, @nohwnd @Youssef1313 WDYT of showing it in CLI when --output detailed is set?

Alternatively, we could add a setting to the API (maybe that's too fine granularity) or in MSTest settings. In #6716 we may want to have the fine grained setting so maybe having 2 settings isn't bad.

[Youssef1313]

IMO --output detailed is a platform feature and we shouldn't use it to change how assertion messages show up.

[Evangelink]

This is a platform feature and on CLI it's the platform that is responsible of how to display test node results. My idea was to add some extra properties "actual" and "expected" that are optional and would be displayed when the flag is set. This way this is something any fwk can leverage

[nohwnd]

The mechanics are already there, shown unconditionally but mstest assertions don't take advantage of them. And at least HandleMethodException in TestAdapter drops the original exception data, so they don't propagate all the way from the test to the UI.

https://grep.app/search?f.repo.pattern=testfx&case=true&regexp=true&q=assert.actual%22

[Evangelink]

I have done some manual tests and I confirm things are correctly plugged on Test Explorer and MTP CLI side. We need a tweak in HandleMethodException as pointed out but then we will also need to add these properties to the VSTest TestResult object so that we can flow it done to MTP through the bridge.

It doesn't seem to be a massive additional work so I'll try to tackle it this week.

The next big piece (outside of this ticket) will be to create a "to string" representation of an arbitrary object (xUnit and FluentAssertions seem to be looking for properties of an object and do something like TypeName{ <public properties> } .

[nohwnd]

are correctly plugged on Test Explorer

It shows the expected and actual, but it does not show a real diff, right?

In vscode it might be different there the diff UI for it is already built-in.

We will also need to add these properties to the VSTest TestResult object so that we can flow it done to MTP through the bridge.

The bridge is only used by NUnit afaik, and we don't need to add a new feature for them into vstest. Unless I am misunderstanding what you mean here.

The next big piece (outside of this ticket) will be to create a "to string" representation of an arbitrary object (xUnit and FluentAssertions seem to be looking for properties of an object and do something like TypeName{ } .

True, but if we currently limit it only to the places where strings are compared, it seems very useful already if paired with diffing on the UI side.

[Evangelink]

It shows the expected and actual, but it does not show a real diff, right?

Correct.

The bridge is only used by NUnit afaik, and we don't need to add a new feature for them into vstest. Unless I am misunderstanding what you mean here.

I meant that in MSTest and NUnit, because we are using the bridge, we report a VSTest TestResult object that gets translated into the TestNode and TestNodeUpdate message which means that we need to have the VSTest TestResult hosting these 2 new properties so we can translate them.

True, but if we currently limit it only to the places where strings are compared, it seems very useful already if paired with diffing on the UI side.

Definitely!