2
\$\begingroup\$

This time I'd like you to review my unit-test helpers for binary operators. I wrote them because I'm often to lazy to test these operators because writing those tests is so boring.

My little helpers are based on the MSTest.TestFramework and are built as extensions for Assert.That.

They start with a class that introduces the Operator() extension.

public static class AssertExtensions
{
    public static IBinaryOperatorAssert Operator(this Assert assert) => default(IBinaryOperatorAssert);
}

This returns the first interface that I use to chain other extensions to finally get to the operator I want to test:

public interface IBinaryOperatorAssert { }

[BinaryOperator("==")]
public interface IBinaryOperatorEqualAssert { }

[BinaryOperator("!=")]
public interface IBinaryOperatorNotEqualAssert { }

Concrete operator interfaces are decorated with an attribute that specifies its symbol for later usage in messages.

public class BinaryOperatorAttribute : Attribute
{
    private readonly string _op;
    public BinaryOperatorAttribute(string op) => _op = op;
    public override string ToString() => _op;
}

public static class BinaryOperatorAssertExtensions
{
    public static IBinaryOperatorEqualAssert Equal(this IBinaryOperatorAssert assert) => default(IBinaryOperatorEqualAssert);
    public static IBinaryOperatorNotEqualAssert NotEqual(this IBinaryOperatorAssert assert) => default(IBinaryOperatorNotEqualAssert);
}

The final extensions are implemented in this class that for each TLeft and TRight type generates, compiles and caches an expression (with some help of other classes).

public static class BinaryOperatorEqualAssertExtensions
{
    public static void IsTrue<TLeft, TRight>(this IBinaryOperatorEqualAssert assert, TLeft left, IEnumerable<TRight> others)
    {
        BinaryOperator<IBinaryOperatorEqualAssert>.Check(left, others, Assert.IsTrue, Expression.Equal);
    }

    public static void IsFalse<TLeft, TRight>(this IBinaryOperatorEqualAssert assert, TLeft left, IEnumerable<TRight> others)
    {
        BinaryOperator<IBinaryOperatorEqualAssert>.Check(left, others, Assert.IsFalse, Expression.Equal);
    }
}

As all operators are binary and return either true or false we can pass the acutal assert as an Action<bool, string> to check the condition and generate the message only in one place:

internal static class BinaryOperator<TBinaryOperator>
{
    public static void Check<TLeft, TRight>(TLeft left, IEnumerable<TRight> others, Action<bool, string> assert, Func<Expression, Expression, BinaryExpression> createBinaryExpressionFunc)
    {
        var opName = typeof(TBinaryOperator).GetCustomAttribute<BinaryOperatorAttribute>();
        var opFunc = BinaryExpressionCache.GetOperatorFunc<TLeft, TRight, TBinaryOperator>(createBinaryExpressionFunc);
        var i = 0;
        foreach (var right in others)
        {
            assert(
                opFunc(left, right),
                $"Operator {typeof(TLeft).Name.QuoteWith("'")} {opName} {typeof(TRight).Name.QuoteWith("'")} " +
                $"failed for {left.Stringify()} {opName} {right.Stringify()} at [{i++}]."
            );
        }
    }
}

Compiled operators are cached inside another helper:

internal static class BinaryExpressionCache
{
    private static readonly ConcurrentDictionary<(Type Left, Type Right, Type Operator), object> ExpressionCache =
        new ConcurrentDictionary<(Type Left, Type Right, Type Operator), object>();

    public static Func<TLeft, TRight, bool> GetOperatorFunc<TLeft, TRight, TOperator>(Func<Expression, Expression, BinaryExpression> createBinaryExpressionFunc)
    {
        var key =
        (
            typeof(TLeft),
            typeof(TRight),
            typeof(TOperator)
        );

        return (Func<TLeft, TRight, bool>)ExpressionCache.GetOrAdd(
            key, 
            k => BinaryExpressionFactory<TLeft, TRight>.Create(createBinaryExpressionFunc)
        );
    }
}

The expressions are created with a simple factory to reduce redundancy when creating other operators.

internal static class BinaryExpressionFactory<TLeft, TRight>
{
    public static Func<TLeft, TRight, bool> Create(Func<Expression, Expression, BinaryExpression> createBinaryExpressionFunc)
    {
        var equalExpr =
            createBinaryExpressionFunc(
                Expression.Parameter(typeof(TLeft), "left"),
                Expression.Parameter(typeof(TRight), "right")
            );

        return Compile(equalExpr);
    }

    private static Func<TLeft, TRight, bool> Compile(BinaryExpression binaryExpression)
    {
        return
            Expression.Lambda<Func<TLeft, TRight, bool>>(
                binaryExpression,
                (ParameterExpression)binaryExpression.Left,
                (ParameterExpression)binaryExpression.Right
            ).Compile();
    }
}

Example

With my unit-tests I can now pass it the left and several right parameters and check their equality in one go. This is an example from one of my tests:

    [TestMethod]
    public void opEqual_SameValues_True()
    {
        Assert.That.Operator().Equal().IsTrue(SoftString.Create("foo"), new[]
        {
            "foo",
            "fOo",
            "foo ",
            " fOob", // this is purposely invalid to check the message
            " foo "
        });
    }

The message it produces is:

Operator 'SoftString' == 'String' failed for 'foo' == ' fOob' at [3].


== and != are the only operators I've implemented so far and I would like to hear your opinion about it before I fully dive into at and implement the other ones too.

\$\endgroup\$
1
\$\begingroup\$

My impression

That's an interesting piece of code.

I like the fluent interface and especially the clear failure messages that it generates 'for free' (from the callers perspective).

What I don't like is that the operator that's being tested is not visible anywhere (Operator().Equal() isn't quite as obvious as ==), and the amount of relatively complicated infrastructure code that's required for all this. Granted, it's only written once, and if you have lots of classes and operators to test it'll pay for itself, but I can't help but think that there are more economical and flexible solutions for this.

Alternatives

Without any custom Assert methods, your test would probably look something like this:

var softString = SoftString.Create("foo");
foreach (var value in new [] { "foo", "fOo", "foo ", " fOob", " foo " })
{
    Assert.IsTrue(
        softString == value,
        $"Operator 'SoftString' == 'String' failed for '{softString}' == '{value}'");
}

Several things jump out: the == operator is clearly visible, it's testing multiple inputs, and the failure message code is quite verbose.


There are already several ways to handle multiple inputs:

  • Use a loop (obviously).
  • Add input parameter(s) to your test method and give the method a [DataRow(..value(s)..)] attribute for each input.
  • Store the inputs in a database or file and reference it with a [DataSource(...)] attribute, then fetch the input via TestContext.DataRow.

So I'm not sure how useful it is to also have a 'vectorized' IsTrue(TLeft left, IEnumerable<TRight> others) method.


With regards to failure messages, your code essentially boils down to:

Assert.IsOperatorEqualTrue(softString, value);  // generates failure message

A generic method like that is a little tricky to implement, because there are no generic constraints for operators. But you don't need to create, compile and cache expressions:

public static void IsOperatorEqualTrue<TLeft, TRight>(this Assert assert, TLeft left, TRight right)
{
    Assert.IsTrue((dynamic)left == right, GetOperatorFailureMessage("==", left, right);
}

If there is no suitable == operator then this will throw a different exception than your code, but both provide sufficient information. Other than that it should achieve the same as your code - though I haven't compared performance.

The only 'problem' here is that the operator isn't visible again, but I don't see many alternatives to solve that:

// 1. Manually call a get-message helper method (both values occur twice in the code):
Assert.IsTrue(softString == value, GetOperatorFailureMessage("==", softString, value);

// 2. Pass in an expression, which can be compiled and executed,
//    but also inspected and used to generate a failure message:
Assert.That.IsTrue(() => softString == value);

I like the idea of having an expression-based IsTrue method: it can be used for various other tests, not just for operators. If you only allow expressions of a certain form then generating a message may not even be that much work, but that's a slippery slope.

\$\endgroup\$
  • \$\begingroup\$ if you have lots of classes and operators to test I do ;-) or rather I always wanted to have but never really implemented because of the tedious testing so I thought about this workaround for all the loops and nearly identical asserts and messages. I admit, the operator is not clearly visible but I have an improved version where the first extension is now called BinaryOperator as in the meantime I also added a UnaryOperator for implicit/explicit casting tests. \$\endgroup\$ – t3chb0t Sep 17 '17 at 21:45
  • 1
    \$\begingroup\$ Most of what I found about the DataSource attribute was about databases, but I find files much easier to work with for the amount of data I'm using. Here's how I got it working for xml files: [DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", @"|DataDirectory|\Subfolder\filename.xml", "childnode", DataAccessMethod.Sequential)], with xml like <root><childnode field1="foo" field2="bar" /></root> - each child node is a row, and each attribute a column/cell value. \$\endgroup\$ – Pieter Witvoet Sep 17 '17 at 21:58
  • 1
    \$\begingroup\$ About the many operators you need to test, how about something like this: <test softstring="foo" input=" fOo" equal="true" concat="foo fOo" /> could be used to create a "foo" soft string and a " fOo" string, equal=true indicates that == should return true and != false, and concat="..." indicates the expected result of the + operator, for example. Each data row can be used to test several operators. This data can be used by multiple operator-specific test methods, or you do all tests in a single 'test all operators' method per type. Just an idea. \$\endgroup\$ – Pieter Witvoet Sep 18 '17 at 7:08
  • 1
    \$\begingroup\$ So much to MSTest being a framework. Xml and Csv datasources are hardcoded TestDataConnectionFactory.cs see lines 18 and 28 :-( \$\endgroup\$ – t3chb0t Sep 18 '17 at 7:20
  • 1
    \$\begingroup\$ The architecture and the note at line 26 seem to suggest that it was meant to be customizable - but that probably didn't get a high priority. Perhaps there's a way to do it with a custom DbConnection implementation? \$\endgroup\$ – Pieter Witvoet Sep 18 '17 at 8:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.