New assertion caller-argument-expression strings are not overridden by custom error messages

[ogiarch]

Describe the bug

(Affected versions: MSTest 4.x, all platforms)

In the "CallerArgumentExpression" section of https://github.com/microsoft/testfx/releases/tag/v4.0.0, it is mentioned that the expressions used in each assertion method are now explicitly named in error messages returned by these methods, in the following format: 'expected' expression: '"Giraffe"', 'actual' expression: 'animal'..

Breaking from the general behavior of MSTest 3.x, these new messages cannot be overridden by custom user-provided error messages or any other parameters unless users willingly go against recommended practice and supply empty strings to the caller-argument-expression parameters. This makes custom assertion behavior, particularly wrappers around basic assertion functions, less flexible than before.

Steps To Reproduce

Type and run the following in an example test project with MSTest 4.0 or above installed:

[TestClass]
public sealed class Test1
{
    private static void AssertEqualReversed<T>(
        T actual,
        T expected,
        [CallerArgumentExpression(nameof(actual))] string actualName = "",
        [CallerArgumentExpression(nameof(expected))] string expectedName = "") =>
        Assert.AreEqual(expected, actual,
            $"Value under test, {actualName}, is not equal to expected value {expectedName}.");

    [TestMethod]
    public void TestMethod1()
    {
        var actualAnimal = "Giraffe";
        var expectedAnimal = "Okapi";
        AssertEqualReversed(actualAnimal, expectedAnimal);
    }
}

Expected behavior

The resulting exception text is as follows:

Assert.AreEqual failed. Expected string length 5 but was 7. Value under test, actualAnimal, is not equal to expected value expectedAnimal.
Expected: "Okapi"
But was:  "Giraffe"

Actual behavior

The resulting exception text is as follows:

Assert.AreEqual failed. Expected string length 5 but was 7. 'expected' expression: 'expected', 'actual' expression: 'actual'. Value under test, actualAnimal, is not equal to expected value expectedAnimal.
Expected: "Okapi"
But was:  "Giraffe"

Additional context

My company's codebase provides a lot of wrappers around MSTest assertions which we consider very useful (certainly more useful than my example above). These wrappers, when calling MSTest assertion methods, will always pass in arguments with the same name. It is not useful in these cases for us to see the same information over and over again regarding the names of the arguments to the base Assert.Is/AreX methods; this adds unnecessary and unwanted information to our error outputs.

We are currently working around this by purposefully passing "" to all caller-argument-expression parameters in these base MSTest assertion methods, but the documentation makes it very clear that this workaround is discouraged (example below):

testfx/src/TestFramework/TestFramework/Assertions/Assert.IsTrue.cs

Lines 180 to 187 in 291dccb

	/// <param name="conditionExpression">
	/// The syntactic expression of condition as given by the compiler via caller argument expression.
	/// Users shouldn't pass a value for this parameter.
	/// </param>
	/// <exception cref="AssertFailedException">
	/// Thrown if <paramref name="condition"/> is true.
	/// </exception>
	public static void IsFalse([DoesNotReturnIf(true)] bool? condition, string? message = "", [CallerArgumentExpression(nameof(condition))] string conditionExpression = "")

If this is all intended behavior, we would like to know if any suggested/approved workaround for this issue exists. Thanks for your time!

[nohwnd]

Thanks for reporting, we are already looking into making the error messages of assertions more useful, using the strings from the expressions.

Can you tell us a bit more about the problem your are having? Do you want to fully suppress the values for expressions, or provide your own values?

[mirek-rynly]

We were also quite surprised by the new behavior here. Providing information about parameter names makes sense (and is desirable) as default behavior when the framework is generating an error message on my behalf.

So soon as I'm providing my own desired error message, however, I would expect that framework-provided information about parameter names is not included.

In other words: if I don't express any preference for what I'd like the error message to say, try to provide something useful on my behalf. If I do express a preference for an error message, however, present it as-is instead of assuming I want it padded with additional information.

[Evangelink]

Just as an FYI, the user message has never been a replacement for the default message but just a complement. We have added showing values of parameters to improve the default message.

I understand the will and need you are describing but I would consider this as a feature request.