New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delegate should throw a consistent and descriptive exception on any sort of method signature mismatch #473

Closed
karypid opened this Issue Oct 17, 2017 · 0 comments

Comments

2 participants
@karypid

karypid commented Oct 17, 2017

Tested with version 1.36

When using mockit.Delegate<T> I have found that there are 3 different exceptions that may be thrown:

  1. IllegalArgumentException: when the number of arguments does not match, or an argument's type does not match. (see testNumArgs and testTypeArgs in the test below)
  2. ClassCastException: when the return type does not match
  3. NullPointerException: when the return type of the delegate is Void instead of what is expected

I also believe that (3) does not provide a useful message in it. It seems to be a side-effect of jmockit trying to access the delegate's result while transferring to the caller. It does not point to the problem (as opposed to 1 and 2 which are quite descriptive). Here are the stack traces:

testNumArgs: java.lang.IllegalArgumentException: Failure to invoke method: public int DelegateProblemTest$3$1.delegate(byte)
at DelegateProblemTest.testTypeArgs(DelegateProblemTest.java:70)
Caused by: java.lang.IllegalArgumentException: argument type mismatch
... 1 more

testNumArgs: java.lang.IllegalArgumentException: Failure to invoke method: public int DelegateProblemTest$2$1.delegate(float,int)
at DelegateProblemTest.testNumArgs(DelegateProblemTest.java:52)
Caused by: java.lang.IllegalArgumentException: wrong number of arguments
... 1 more

testRetValType: java.lang.ClassCastException: java.lang.Byte cannot be cast to java.lang.Integer
at DelegateProblemTest.testRetValType(DelegateProblemTest.java:88)

testRetValVoid: java.lang.NullPointerException
at DelegateProblemTest.testRetValVoid(DelegateProblemTest.java:106)

I suggest that the return type is checked explicitly for the case of Void in (3) to produce something similar to (2) (java.lang.Void cannot be cast to java.lang.Integer). I also suggest that the ClassCastException be converted to an IllegalArgument exception as in (1), since from the perspective of JMockIt you are providing an incorrect argument (the delegate does not contain what it should) and is more appropriate.

Unit test which demonstrates the issue follows below:

import org.junit.Assert;
import org.junit.Test;

import mockit.Delegate;
import mockit.Expectations;
import mockit.Mocked;

public class DelegateProblemTest
{
    interface Whatever
    {
        int round(float value);
    }

    @Mocked
    Whatever myFunction;

    @Test
    public void testProperDelegate()
    {
        new Expectations() {
            {
                myFunction.round(anyFloat);
                result = new Delegate<Float>() {
                    @SuppressWarnings("unused")
                    public int delegate(float value)
                    {
                        return Math.round(value);
                    }
                };
            }
        };
        Assert.assertThat(myFunction.round(5.0f), Matchers.equalTo(5));
    }

    @Test(expected = IllegalArgumentException.class)
    public void testNumArgs()
    {
        new Expectations() {
            {
                myFunction.round(anyFloat);
                result = new Delegate<Float>() {
                    @SuppressWarnings("unused")
                    public int delegate(float value, /* oops */ int something)
                    {
                        return Math.round(value);
                    }
                };
            }
        };
        Assert.assertThat(myFunction.round(5.0f), Matchers.equalTo(5));
    }

    @Test(expected = IllegalArgumentException.class)
    public void testTypeArgs()
    {
        new Expectations() {
            {
                myFunction.round(anyFloat);
                result = new Delegate<Float>() {
                    @SuppressWarnings("unused")
                    public int delegate(/* oops */ byte value)
                    {
                        return (int) Math.round(value);
                    }
                };
            }
        };
        Assert.assertThat(myFunction.round(22335.0f), Matchers.equalTo(22335));
    }

    @Test(expected = ClassCastException.class)
    public void testRetValType()
    {
        new Expectations() {
            {
                myFunction.round(anyFloat);
                result = new Delegate<Float>() {
                    @SuppressWarnings("unused")
                    public byte delegate(float value)
                    {
                        return (byte) Math.round(value); // oops
                    }
                };
            }
        };
        Assert.assertThat(myFunction.round(5.0f), Matchers.equalTo(5));
    }

    @Test(expected = NullPointerException.class)
    public void testRetValVoid()
    {
        new Expectations() {
            {
                myFunction.round(anyFloat);
                result = new Delegate<Float>() {
                    @SuppressWarnings("unused")
                    public void delegate(float value)
                    {
                        Math.round(value); // oops
                    }
                };
            }
        };
        Assert.assertThat(myFunction.round(5.0f), Matchers.equalTo(5));
    }
}

@rliesenfeld rliesenfeld self-assigned this Oct 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment