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

Use Unsafe to set final static fields, support JDK12+ #1026

Merged
merged 2 commits into from Jan 25, 2020

Conversation

eolivelli
Copy link
Contributor

@eolivelli eolivelli commented Dec 19, 2019

From JDK12+ is it no more possible to set a value to final static fields.
With this patch we are going to use Unsafe, that works on every JDK up to 13.
We are now also allowing to write to 'static final fields'

@thekingn0thing
Copy link
Member

Test are falling

org.powermock.reflect.WhiteBoxTest > testSetInternalStateWithInvalidArgumentType FAILED
java.lang.Exception: Unexpected exception, expected<java.lang.IllegalArgumentException> but was<java.lang.RuntimeException>

    Caused by:
    java.lang.RuntimeException: java.lang.IllegalArgumentException: Can not set int field org.powermock.reflect.testclasses.ClassWithInternalState.internalState to java.lang.Object
        at org.powermock.reflect.internal.WhiteboxImpl.setFieldUsingUnsafe(WhiteboxImpl.java:2374)
        at org.powermock.reflect.internal.WhiteboxImpl.setField(WhiteboxImpl.java:2309)
        at org.powermock.reflect.internal.WhiteboxImpl.setInternalState(WhiteboxImpl.java:305)
        at org.powermock.reflect.Whitebox.setInternalState(Whitebox.java:174)
        at org.powermock.reflect.WhiteBoxTest.testSetInternalStateWithInvalidArgumentType(WhiteBoxTest.java:400)

        Caused by:
        java.lang.IllegalArgumentException: Can not set int field org.powermock.reflect.testclasses.ClassWithInternalState.internalState to java.lang.Object
            at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
            at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
            at sun.reflect.UnsafeIntegerFieldAccessorImpl.set(UnsafeIntegerFieldAccessorImpl.java:98)
            at java.lang.reflect.Field.set(Field.java:764)
            at org.powermock.reflect.internal.WhiteboxImpl.setFieldUsingUnsafe(WhiteboxImpl.java:2367)
            ... 4 more

org.powermock.reflect.WhiteBoxTest > testStaticFinalStringState FAILED
java.lang.AssertionError: Expected test to throw (an instance of java.lang.IllegalArgumentException and exception with message a string containing "You are trying to set a private static final String. Cannot set such fields!")

@eolivelli
Copy link
Contributor Author

I will fix them.
Thank you for taking a look.

@eolivelli
Copy link
Contributor Author

@thekingn0thing I have fixes the test.
PTAL

@eolivelli eolivelli changed the title Use Unsafe to set final fields, support JDK12+ Use Unsafe to set final static fields, support JDK12+ Jan 13, 2020
@eolivelli
Copy link
Contributor Author

Hi @thekingn0thing
it would be great to have a release right after committing this fix, it will help a lot downstream projects that are migrating to jdk13/14 from jdk11

thanks

@thekingn0thing thekingn0thing merged commit c7406c4 into powermock:release/2.x Jan 25, 2020
@thekingn0thing
Copy link
Member

@eolivelli Merged. Thank you a lot for a pull request and sorry for such long delay.

@eolivelli eolivelli deleted the fix/support-jdk12 branch January 25, 2020 14:09
@eolivelli
Copy link
Contributor Author

@thekingn0thing thanks to you that you are maintaining this great project !

I hope we can get a release out soon, this patch brings back WhiteBox to full powers in modern jdks land

@thekingn0thing
Copy link
Member

@eolivelli the library was automated released by pipline after the PR had been merged.
https://repo.maven.apache.org/maven2/org/powermock/powermock-api-mockito2/2.0.5/

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

Successfully merging this pull request may close these issues.

None yet

2 participants