drunkcod / Cone

8 stars 1 forks source link

Matcher objects and Cone ? #15

Closed lucaminudel closed 13 years ago

lucaminudel commented 13 years ago

Matcher objects helps to improve test diagnostic so the test failure message make it clear immediately what went wrong. I.e. Payment date -expected: [start date] -actual: [end date] instead of Payment date -expected: [Thu Jan 01 01:00:01 GMT 1970] -actual: [Thu Jan 01 01:00:02 GMT 1970]

Look this presentation from minute 22: http://www.infoq.com/presentations/Sustainable-Test-Driven-Development

A well known matcher objects library is the .NET NHamcrest port from the java project: https://github.com/grahamrhay/NHamcrest/wiki

These matcher objects expect to be used with the a Assert.AreEqual(expected, actual) type of operator because the match is tested via the equality operator and the diagnostic message is displayed via the ToString() method of the actual and expected value.

But Verify.That instead expect a boolean function.

Would make sense to add a binary operator to Cone i.e. Verify.That(object, object) so to reuse the existing matcher libraries? Or would make sense to extend the existing Cone method Verify.That to accept a "matcher" expression that is an extended bool with additional diagnostic message information, and write custom Cone matchers ?

Comments/ideas?

drunkcod commented 13 years ago

Ignore what I just wrote... It would work if Cone employed the same backwards equality checking that NUnit does. I'll create a sample on how to get the real intended effect "tm".

[Edit]: I'm having the best time EVAR here... 1) Seems the NHamcrest examples won't work with nunit 2.5.7. 2) They're definitly not built upon equality checking as I beleived 3) I've found a initialization ordering bug that prevents Cone from picking up method expect providers from external assemblies.

drunkcod commented 13 years ago

It should be possible and I thought it would be farily easy (wasn't see reasons above) to integrate NHamcrest or similar matchers with Cone, so that's something that I'll fix for the next release hopefully within a week or so.

There's another point that I posted initialy that also deserves to be publicly discussed and it's the following: From a philsophical point of view Cone is built with the beleif that the need for matchers and custom verification messages most of the time (not always) is due to a undue constraint in one of the following:

drunkcod commented 13 years ago

Ok, after fixing the IMethodExpectProvider detection bug this is one example of how NHamcrest could be integrated with Cone without altering the framework code.


using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Cone;
using Cone.Expectations;
using NHamcrest;
using NHamcrest.Core;

namespace Issue15
{
    public class Check 
    {       
        public static bool That(string actual, IMatcher<string> matcher) {
            return matcher.Matches(actual);
        }
    }

    public class CheckThatProvider : IMethodExpectProvider
    {
        public IExpect GetExpectation(Expression body, MethodInfo method, object target, IEnumerable<object> args) {
            var a = args.ToArray();
            return new CheckThatExpect(body, method, a[0], a[1]);
        }

        public IEnumerable<MethodInfo> GetSupportedMethods() {
            return new[]{ typeof(Check).GetMethod("That") };
        }
    }

    class CheckThatExpect : IExpect
    {
        readonly MethodInfo method;
        readonly Expression body;
        readonly object actual, expected;

        public CheckThatExpect(Expression body, MethodInfo method, object actual, object expected) {
            this.body = body;
            this.method = method;
            this.actual = actual;
            this.expected = expected;
        }

        public ExpectResult Check() {
            return new ExpectResult {
                Actual = actual,
                Success = (bool)method.Invoke(null, new[]{ actual, expected })
            };
        }

        public string FormatExpression(IFormatter<Expression> formatter) {
            return formatter.Format(body);
        }

        public string FormatMessage(IFormatter<object> formatter) {
            return string.Format("Expected {0}\nbut was {1}", expected.ToString(), formatter.Format(actual));
        }
    }

    [Describe(typeof(UsingNHamcrest))]
    public class UsingNHamcrest
    {
        public void successful_match() {
            Verify.That(() => Check.That("the cat sat on the mat", Ends.With("mat")));
        }

        public void failing_match() {
            Verify.That(() => Check.That("the mat sat on the cat", Ends.With("mat")));
        }
    }
}

The failing match is reported like this:

Issue15.UsingNHamcrest.failing match:
Check.That("the mat sat on the cat", Ends.With("mat"))
Expected a string ending with "mat"
but was "the mat sat on the cat"
lucaminudel commented 13 years ago

There's another point that I posted initialy that also deserves to be publicly discussed and it's the following: From a philsophical point of view Cone is built with the beleif that the need for matchers and custom ...

yep, agree we should discuss this. to understand if the idea of using matcher with Cone is a good one or if there are better and simpler ways to achieves the same results.

I remember one practical case writing a unit test in pair with Johan. in that case naming the variables and refactoring the expression used in the Verify.That was not enough to make the test diagnostic message clear. should check with Johan if it's possible to find that code.

I think to remember that the error message in that case needed to be calculated at run-time, could not make it clear just in the static source code.

other 2 cases was due to the need to add the implementation of the Equal operator to an object just to make is possible to write the expression passed to the Verify.That() clear to read and understand in the test diagnostic message when the test fail. with matcher object that object comparison code added could be instead added only to a custom matcher.

so the question is: should strive to make the expression passed to the Verify.That more readable or in general it would be easier to use matcher objects ? and if so which matcher objects, like the NHarmcrest one or others that could be more aligned with the Cone philosophy ?

will think about this, if you have comments and opinion please add your comments (it will help me to think)

drunkcod commented 13 years ago

Re: NHamcrest or similar see above they can, when needed, be reasonably well integrated and I'll do some minor tweaks to make it even easier and release that update this Friday if all goes well.

That's a compromise from a neccessary "evil" in my eyes but also a sensible, pragmatic and sometimes even the cleanest option to achive some ends.

It would be interesting to get your specific cases into the discussion, hope you can find the code or equivalent samples.

What I've found is that the "need" to implement Equals and a "sensible" ToString method driven by the quest for test clarity is a positive thing, it enriches the domain and doesn't "bottle up" usefull funtionality within the test suite. If the test need to check equality it's probably something the domain should support, and having a reasonable ToString method makes logging, debugging and ad hoc testing much, much easier.

That's the key thing and concept in Cone the test should speak the language of the domain, the verifications should make sense in "domain speak". In essence Verifications should be something that could reasonably well happen within the system. That's something that NUnit fails miserably at often enough the abstraction leaks horribly you end up with two different "languages" and mental models.

I beleive the cognitive load of always mapping "Assert.That(actual, Is.EqualTo(expected))" to "actual == expected" (with the caveat that the test that ill happen is expected.Equals(actual)) is not at all trivial and greatly contribute to muddling up our models of how the code will really behave.

drunkcod commented 13 years ago

The discussion above shows how to integrate any given matcher library with Cone addtionally there's a sample here: https://github.com/drunkcod/Cone/blob/master/Samples/CustomMethodExpectProviderSample.cs that shows the "native Cone" route to things.

lucaminudel commented 13 years ago

found the code example mentioned before. in that code to properly understand the error it is needed to visualize the actual value that is retrieved at test run-time. so while the expected value is known, a way to show the actual value when the test fail was needed the code is something like this:

const someType expected = someValue; someType actual = someObject.getSomething(); Verify.That( () => expected == actual );

I admit it rare and in our code the comparison is behind a dsl so when comparison fails we resolved to throw an exception with actual value in the message.

still this example can help (me) to understand the philosophy of Cone. How should I deal this case following properly the philosophy of Cone ?

drunkcod commented 13 years ago

I'm not really sure I understand your use case here.

Verify.That(() => actual == expected); Will (as most other frameworks do) use obj.ToString() to display the result of user defined types. So in those cases supplying a reasonable to string method is way to go, and I don't really know of any framework that does it in any other way, examples welcome.

Maybe you could create a small sample using some other framework to illustrate your point and I could show how I would do that using Cone?

Or if the feature isn't currently available in any framework you're familiar with some real code annotated with the expected output from the verification would be a good substitute.

Such discussion would benefit many so please if you have the time (something I would appreciate) post it to the Cone users group found at http://groups.google.com/group/cone-users

Thanks.