Restore previously deleted methods associated with MockUp classes. #158

yukkes · 2023-12-01T07:27:21Z

Thanks you for your excellent work, I have restored previously deleted methods associated with MockUp classes.
I have attempted to fully restore the processing of past versions of JMockit, but have had some setbacks.
I hope you recognize this as an area for future improvement.

The number of invocations is ignored because there is no judgment process such as @mock(invocations = 1).

The following errors have been resolved. (12/12/2023)

~~The javadoc generation fails because ConstructorReflection refers to "sun.reflect.*" of a private package in Java 9 or later. The error has been avoided by adding "--ignore-source-errors".~~
~~The test fails because only the last value set in the following test case can be retrieved.~~

   @Test
   public void multiplePublicMockUps(){
      AClass mock1 = new AClassMockUp("Abc").getMockInstance();
      AClass mock2 = new AClassMockUp("Xpto").getMockInstance();

      assertNotSame(mock1, mock2);
      assertEquals("Abc", mock1.getTextValue());
      assertEquals("Xpto", mock2.getTextValue());
      assertTrue(AClass.doSomething());
   }

hazendaz · 2023-12-02T04:18:59Z

Hi @yukkes, yes very much appreciate this. The original author I could never understand why he consistently deleted support making upgrades next to impossible. So I get everything is a work in progress and welcome all your efforts here to restore any needed features that will make your own migrations and others much easier. I think we all know this library is just maintenance but it needs to work and easily and hopefully up through higher jdks than current as the cost of migrating is high. I do know open rewrite is working on trying to have a migration path to mockito and are very well aware of this fork. I think their hurdles have been much of the lost support which makes it hard for them to have the latest here and somehow upgrade the older copies. So I think this will greatly help so keep it coming :)

hazendaz · 2023-12-02T04:19:48Z

As noted on the other PR, I think we need both 11 and 17 working here before we merge. 21 is going to fail. I can just remove it from the builds when I get some time as its an ongoing issue. But the master currently work son 11 and 17.

hazendaz · 2023-12-02T04:20:52Z

For the sun.reflect, can you write that any differently so it might work on 17? Presume that was when it was either entirely blocked or maybe removed, not sure. Would need to look. Its ok if the code doesn't come back exactly like it was. The main thing is that it just works for the folks that need it.

hazendaz · 2023-12-02T04:37:16Z

holding off this one for now since jdk 17 failed. Please look into that a bit more.

yukkes · 2023-12-02T05:53:40Z

Thanks for the code review. I'll check back later as I'm spending time with my family right now.

yukkes · 2023-12-02T14:49:26Z

Fixed not to reference the sun.reflect package.
The following blog in Japanese was used as a reference.
https://matsudamper.hatenablog.com/entry/2020/05/17/103231

    public static <T> T newUninitializedInstance(@Nonnull Class<T> aClass) {
        SunReflectionFactoryInstantiator<T> ref = new SunReflectionFactoryInstantiator<>(aClass);
        return ref.newInstance();
    }

yukkes · 2023-12-02T15:25:41Z

@hazendaz, thanks for the accurate review remarks.
We have been using JMockit 1.20 for our internal system development and the latest version (1.49) has removed existing functionality, making it difficult to upgrade.
We have now decided to update from Java 8 to 17, and the large amount of test code has made it difficult to migrate to anything but JMockit. So I made JMockit work without modifying the existing test code.

I committed to this project because we plan to continue using JMockit after Java 21. I hope others will find it useful in the future.

yukkes · 2023-12-02T18:14:16Z

The reason for the test failure in Java 21 is that the class to be mocked is now a sealed class.
The only solution seems to be to ignore the test.

JDK18 or earlier

public class InetAddress implements java.io.Serializable {

JDK19 or later

public sealed class InetAddress implements Serializable permits Inet4Address, Inet6Address {

hazendaz · 2023-12-02T19:09:45Z

The reason for the test failure in Java 21 is that the class to be mocked is now a sealed class. The only solution seems to be to ignore the test.
* JDK18 or  earlier
public class InetAddress implements java.io.Serializable {
* JDK19 or later
public sealed class InetAddress implements Serializable permits Inet4Address, Inet6Address {

Agreed, I started looking at migrating most of the tests to junit 5 only using open rewrite but it had a very hard issue here. So my plan is to do that then simply disable for higher jdk. Thank you for finding the info. Do note this did not prevent this from working on java 21. I use it at work without issues already at that level. But for our audience faster we fix to show it works on each the more contributors may want to engage and feel more comfortable its working.

hazendaz · 2023-12-02T19:16:01Z

main/pom.xml

@@ -9,7 +9,6 @@
        <version>1.51.1-SNAPSHOT</version>
    </parent>

-    <groupId>com.github.hazendaz.jmockit</groupId>


I'll add this back, its a strong preference of mine to be strict in poms of the GAV. I know m2e in eclipse is all messed up since it has warnings even if set to not warn in recent releases. Won't bother you with that here, I'll take care of no issues.

hazendaz · 2023-12-02T19:21:13Z

main/src/main/java/mockit/internal/faking/FakedClassModifier.java

-            throw new IllegalArgumentException(
-                    "Unsupported fake for private " + kindOfMember + privateMemberDesc + " found");
-        }
+        // Supported fake for private method


@yukkes Can you add a simple further note here that we are keeping commented out code from original changes? Normally I'd say delete it but I think in this case I agree commenting it out is fine but want to make sure its obvious why we are keeping commented out code, otherwise someone will delete it it in the future.

hazendaz · 2023-12-02T19:23:23Z

main/src/test/java/mockit/MisusedFakingAPITest.java

-        thrown.expectMessage("Component");
-        thrown.expectMessage("checkCoalescing()");
-        thrown.expectMessage("found");
+        // Enabled to fake a private constructor.


Same on these, just a bit more clarlity as to why we are keeping commented out code.

hazendaz · 2023-12-02T19:24:46Z

@yukkes Very minor asks to make it a bit more clear why intention to keep commented out code. You may want to date it as well I typically will add a todo as well so its easily seen, something like this with XXX (your initials)

// TODO 12/2/2023 XXX Reason why we are doing something odd

yukkes · 2023-12-03T02:48:50Z

@hazendaz , thanks for the code review. I fixed the code.
I had included the company email address in last night's commit, so I overwrote it with force-push. Sorry it's hard to see the commit history.

yukkes · 2023-12-09T06:52:48Z

Please wait for the merge as we are in the process of resolving JMockit-related errors in our internal project.
It is expected to take about a week.

hazendaz · 2023-12-09T16:19:57Z

No problem. Thank you for all you are doing. Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: yukkes ***@***.***> Sent: Saturday, December 9, 2023 1:52:59 AM To: hazendaz/jmockit1 ***@***.***> Cc: Jeremy Landis ***@***.***>; Mention ***@***.***> Subject: Re: [hazendaz/jmockit1] Restore previously deleted methods associated with MockUp classes. (PR #158) Please wait for the merge as we are in the process of resolving JMockit-related errors in our internal project. It is expected to take about a week. — Reply to this email directly, view it on GitHub<#158 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHODI2UYL6IY5PGSSZUY43YIQDEXAVCNFSM6AAAAABACMZ7YWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYGI3DQMRYGU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

…1.35.

yukkes · 2023-12-11T20:16:26Z

I have restored MockUp class and test class based on JMockit ver. 1.35, please review and merge if there is no problem.

The following test class is restored and all tests PASS.

MockInvocationProceedTest.java
MockInvocationTest.java
MockLoginContextTest.java
MockUpForGenericsTest.java
MockUpForSingleClassInstanceTest.java
MockUpForSingleInterfaceInstanceTest.java

However, the MockUpForSingleInterfaceInstanceTest failed the test as is, so I commented it out with a TODO comment. The reason for this is that the FieldInjection class has changed and no longer sets default values.

hazendaz · 2023-12-13T05:08:56Z

@yukkes Are you good at this point for me to push a release?

yukkes · 2023-12-13T11:29:14Z

@hazendaz Yes, please release.
The changes include restoring the MockUp and Deencapsulation classes based on JMockit ver. 1.35.

Note that the annotation to determine the number of method invocations has been restored as shown below, but since there is no judgment process, there is no error even if the number of invocations is different.
@Mock(invocations=1)

hazendaz · 2023-12-15T08:39:17Z

Release is out now. Let me know how that works out for you.

yukkes · 2023-12-15T10:25:04Z

@hazendaz Thank you for the release.
I will check if the unit test passes using the latest version on our own project.

yukkes · 2023-12-16T03:27:06Z

@hazendaz
I have performed my own unit tests and the results match my own build and ver. 1.52.0, so it looks fine. Thanks.

yukkes added 3 commits December 1, 2023 16:18

Restore previously deleted methods associated with MockUp classes.

5b13a35

Restore previously deleted methods associated with MockUp classes.

d7f99ca

Delete unnecessary comments in test class

8a643d0

hazendaz self-assigned this Dec 2, 2023

hazendaz reviewed Dec 2, 2023

View reviewed changes

hazendaz mentioned this pull request Dec 2, 2023

The reason for the test failure in Java 19+ is that the class to be mocked is now a sealed class. #163

Closed

Fixes to not reference the sun.reflect package.

8bbc18a

yukkes force-pushed the develop-restore-mockup-classes branch from 10d7e67 to 8bbc18a Compare December 3, 2023 02:44

Delete unnecessary comments in MisusedFakingAPITest.java

9924c31

MockUp class and related classes were restored based on JMockit ver. …

5b65eb3

…1.35.

hazendaz approved these changes Dec 13, 2023

View reviewed changes

hazendaz merged commit a7bfa92 into hazendaz:master Dec 13, 2023
6 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore previously deleted methods associated with MockUp classes. #158

Restore previously deleted methods associated with MockUp classes. #158

yukkes commented Dec 1, 2023 •

edited

Loading

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

yukkes commented Dec 2, 2023

yukkes commented Dec 2, 2023

yukkes commented Dec 2, 2023 •

edited

Loading

yukkes commented Dec 2, 2023 •

edited

Loading

hazendaz commented Dec 2, 2023

hazendaz Dec 2, 2023

hazendaz Dec 2, 2023

hazendaz Dec 2, 2023

hazendaz commented Dec 2, 2023

yukkes commented Dec 3, 2023 •

edited

Loading

yukkes commented Dec 9, 2023

hazendaz commented Dec 9, 2023 via email

yukkes commented Dec 11, 2023

hazendaz commented Dec 13, 2023

yukkes commented Dec 13, 2023

hazendaz commented Dec 15, 2023

yukkes commented Dec 15, 2023

yukkes commented Dec 16, 2023

Restore previously deleted methods associated with MockUp classes. #158

Restore previously deleted methods associated with MockUp classes. #158

Conversation

yukkes commented Dec 1, 2023 • edited Loading

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

hazendaz commented Dec 2, 2023

yukkes commented Dec 2, 2023

yukkes commented Dec 2, 2023

yukkes commented Dec 2, 2023 • edited Loading

yukkes commented Dec 2, 2023 • edited Loading

hazendaz commented Dec 2, 2023

hazendaz Dec 2, 2023

Choose a reason for hiding this comment

hazendaz Dec 2, 2023

Choose a reason for hiding this comment

hazendaz Dec 2, 2023

Choose a reason for hiding this comment

hazendaz commented Dec 2, 2023

yukkes commented Dec 3, 2023 • edited Loading

yukkes commented Dec 9, 2023

hazendaz commented Dec 9, 2023 via email

yukkes commented Dec 11, 2023

hazendaz commented Dec 13, 2023

yukkes commented Dec 13, 2023

hazendaz commented Dec 15, 2023

yukkes commented Dec 15, 2023

yukkes commented Dec 16, 2023

yukkes commented Dec 1, 2023 •

edited

Loading

yukkes commented Dec 2, 2023 •

edited

Loading

yukkes commented Dec 2, 2023 •

edited

Loading

yukkes commented Dec 3, 2023 •

edited

Loading