Skip to content
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

Jmockit 1.27: Faking private method not possible #324

Closed
numeralEasy opened this issue Aug 31, 2016 · 6 comments
Closed

Jmockit 1.27: Faking private method not possible #324

numeralEasy opened this issue Aug 31, 2016 · 6 comments
Labels

Comments

@numeralEasy
Copy link

Version of JMockit 1.27
Both tests throwing an exception. With 1.25 or 1.26 there is no errror.

java.lang.IllegalArgumentException: Invalid mock method testbox.TestBox$2#getName2() for private method of internal class
at testbox.TestBox$2.(TestBox.java:42)
at testbox.TestBox.testGetName_fails2(TestBox.java:42)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

public final class TestBox {

    public static class Sut {


        public String getName() {
            return getName2();
        }

        private String getName2() {
            return "private";
        }

    }

    @Test
    public void testGetName_fails() {

        Sut mockInstance = new MockUp<Sut>() {
            @Mock
            public String getName2() {
                return "mocked";
            }
        }.getMockInstance();

        assertEquals("mocked", mockInstance.getName());
    }


    @Test
    public void testGetName_fails2() {

        new MockUp<Sut>() {
            @Mock
            public String getName2() {
                return "mocked";
            }
        };

        Sut sut = new Sut();
        assertEquals("mocked", sut.getName());
    }

}
@rliesenfeld
Copy link
Member

Yes, this was a change in version 1.27. More info in the release notes.

@numeralEasy
Copy link
Author

We have chosen the jmockit lib among others because of the possibility to mock private methods.
Is it possible that you undo this change? As of jmockit 1.25 usage of Deencapsulation inside the Expectation block was set to deprecated. So we ported all the affected tests.
At this time it would have been nice to know that the mocking of private methods will not be possible in the future. We have about 5000 tests and 1000+ of them are unusable since 1.27.

@rliesenfeld
Copy link
Member

Mocking of private methods in expectation blocks was dropped in version 1.23 - see release note.

@stevenjackson121
Copy link

stevenjackson121 commented Sep 8, 2016

Can you help me to understand intended support for mocking private methods internal to the codebase going forward?
Mocking of private methods in expectation blocks was dropped in 1.23, with the note that using MockUp was still possible.
MockUp of classes internal to the codebase was dropped in 1.27, which invalidates the workaround suggested in 1.23 release notes.
What is the current suggested way of mocking a private method internal to the codebase, or was it intentional that the 1.27 fully removed support for this?

Consider the following class:

public class Validator{
   public static boolean isValid(Object o){
      return fastIsValid(o) && fullIsValid(o);
   }
   private static boolean fastIsValid(Object o){
     // very fast function that returns true for 100% of valid objects
     // returns false 99+% of invalid objects, returns true for <1%
   }
   private static boolean fullIsValid(Object o){
     // very slow function that returns false for 100% of invalid objects
     // returns true for 100% of valid objects
   }
}

We'd like to verify some properties of this function using unit tests:

  • the function never returns true without calling fullIsValid first.
  • fullIsValid should be called very rarely for invalid objects.
  • fastIsValid is adding low overhead to the overall method for valid objects.

We do not want to make fastIsValid() or fullIsValid() public, in this class or in another class: users should call isValid() in every conceivable circumstance.

  • Users should never only call fullIsValid(): fastIsValid() adds negligible overhead compared to calling only fullIsValid() for valid objects, but provides amazing speedups for invalid objects.
  • Users should never only call fastIsValid(): It's tempting because it's, well, fast, but they aren't actually sure whether the object is valid or not.

Previously we were very happy with jmockit's mocking of private methods internal to our codebase, which allowed us to gain insight into this method during unit tests. If this feature is not going to be restored, could you please advise us as to the suggested approach for refactoring our class to enable testing these properties?

@rliesenfeld
Copy link
Member

rliesenfeld commented Sep 9, 2016

I completely agree that the fastIsValid and fullIsValid methods should not be made public, since they clearly are implementation details of the Validator class, not meant for direct use in client code.

For this same reason, they should not be mocked (or faked). Good tests (whether unit or integration tests) are supposed to verify behavior, not implementation.

So, what you really need when testing the Validator class, is not to do those three bulleted points, which are obviously expressed in terms of the internal implementation of the class, but to write the tests in terms of its externally observed behavior. And since the class has no dependencies on other classes/types (or maybe fullIsValid has), there is nothing to mock in such tests.

The comments inside those two private methods already suggest a better testing strategy. I can imagine the following three tests:

  1. Call Validator.isValid with a few simple valid objects, checking that each call actually runs "slow".
  2. Call Validator.isValid with several invalid objects of various complexities and which are detectable as such through partial evaluation, checking that each call runs fast.
  3. Call Validator.isValid with a few invalid objects which are not detectable as such without a full evaluation, checking that each call runs slow.

@stevenjackson121
Copy link

stevenjackson121 commented Sep 12, 2016

Thanks for the prompt reply.
I'm not sure why I couldn't make the transition from the bullet points I wrote out to the testing strategy you've provided. I was stuck on the level of testing the methods in the class rather than testing the class.
As painful as it is to see things that were previously supported get pulled out, I'm glad targeting the most recent JMockit release helps us detect poor testing practices instead of enabling them. (And I'm glad you're willing to explain the better way when you take away support for the old way)
Thank you for the wonderful product, and the wonderful support.

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

No branches or pull requests

3 participants