drewbourne / mockolate

fake chocolate, mock objects and test spies for AS3
http://mockolate.org/
MIT License
146 stars 26 forks source link

strict expect() bug #54

Open jbarrus opened 12 years ago

jbarrus commented 12 years ago

This is a pretty far out edge case, but there exists a problem with an expectation being carried over from one test to another. Here's an example test case. testStrictBug2 will fail with "examples::Engine<"niceEngine">#talk(ANYTHING,an instance of String)".

If the order is reversed, both of the tests below will succeed.

I found this while doing some experimentation with times(). I was surprised to find out that with a nice, .times() will not cause verification to fail if the expected method was called too many times. It appears that verification that something was called the correct number of times can only be done with a strict or with a test spy (received()).

Also, it had not occurred to me that expect() would not work with a strict. This is actually what led me to discover this bug.

package flexUnitTests
{
    import examples.Engine;

    import mockolate.arg;
    import mockolate.expect;
    import mockolate.runner.MockolateRule;
    import mockolate.strict;

    import org.hamcrest.core.anything;
    import org.hamcrest.object.instanceOf;

    public class StrictBugTestCase
    {
        [Rule]
        public var mockolate:MockolateRule = new MockolateRule();

        [Mock(type="nice")]
        public var niceEngine:Engine;

        [Test(order="1", expects="mockolate.errors.InvocationError")]
        public function testStrictBug1():void
        {
            var strictEngine:Engine = strict( Engine );

            expect( strictEngine.talk( arg( anything() )));
        }

        [Test(order="2")]
        public function testStrictBug2():void
        {
            expect( niceEngine.talk( arg( instanceOf( String ))));

            niceEngine.talk( "test" );
        }
    }
}
package examples
{
    public class Engine
    {
        public function talk( words:String ):void
        {
            trace( words );
        }
    }
}
drewbourne commented 12 years ago

I've pushed some changes today that should fix the case where on a nice mock an expectation with a defined invocation count (eg .once(), .times(2)) will pass if invoked more than the set number of times. If you are comfortable building from source I would appreciate if you could try to replicate this issue in your test suite.

WRT to using expect() and strict mocks, you can do so using expecting() to define a context within which expectations can be defined without triggering the errors a strict mock will usually throw for an unexpected invocation:

var engine:Engine = strict( Engine );
expecting(function():void {
    expect( engine.talk( arg( anything() )));
});

// use the engine
jbarrus commented 12 years ago

It appears to work. I ran it on a large test suite and my mockolate examples tests.

[Test(expects="mockolate.errors.ExpectationError")]
/**
* Previously did not work, but does now.
*/
public function testExpectNiceNumbersOfTimesFail():void
{
    expect( mockDispatcher.dispatchEvent( arg( anything())))
        .once();

    fixtureToTest.testDispatch();
    fixtureToTest.testDispatch();
}

On a side note, why is a different error thrown for never() violation than times(0) violation?

        [Test(expects="org.flexunit.internals.runners.model.MultipleFailureException")]
        public function testMultipleError():void
        {
            expect( mockEngine.start() )
                .never();

            mockEngine.start();
        }

        [Test(expects="mockolate.errors.ExpectationError")]
        public function testTimesZero():void
        {
            expect( mockEngine.start() )
                .times( 0 );

            mockEngine.start();
        }
drewbourne commented 12 years ago

.never() was implemented differently so that it would actually work. That said it should be throwing an InvocationError. I can probably unify it with the new verification logic now so that it is an ExpectationError.