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

Enable annotation engine as plugin #811

Merged
merged 6 commits into from
Dec 13, 2016

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Dec 12, 2016

Currently AnnotationEngine can be overrriden with the deprecated MockitoConfiguration, in order to be more consistent with the new way, I propose to get the AnnotationEngine from the PluginRegistry.

Regarding backward compatible behavior, if the MockitoConfiguration class exists and can be seen then Mockito will select the engine of this configuration instead of the Plugins one. If MockitoConfiguration class don't exist then Mockito wil chose the Plugins one.

Bonus I extended the classloader util to support some tests.

Old interface is still there but is now deprecated in favor of the plugin extension point.
Renamed DefaultAnnotationEngine as it was confusing ont the behavior, this engine only perform action on independent annotations.

Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
It could be bypassed by Unsafe or other tricks. At this moment this is probably enough.

Signed-off-by: Brice Dutheil <brice.dutheil@gmail.com>
@codecov-io
Copy link

codecov-io commented Dec 13, 2016

Current coverage is 87.14% (diff: 100%)

No coverage report found for release/2.x at 4cbcd9a.

Powered by Codecov. Last update 4cbcd9a...360983d

@bric3
Copy link
Contributor Author

bric3 commented Dec 13, 2016

For reference last commit skips static final fields otherwise the dead simple field copier raises an IllegalAccessException

java.lang.IllegalAccessException: Can not set static final [Z field org.mockitoutil.ClassLoadersTest$1.$jacocoData to [Z
	at sun.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:73)
	at sun.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:77)
	at sun.reflect.UnsafeQualifiedStaticObjectFieldAccessorImpl.set(UnsafeQualifiedStaticObjectFieldAccessorImpl.java:77)
	at java.lang.reflect.Field.set(Field.java:741)
	at org.mockitoutil.ClassLoaders$ClassLoaderExecutor.reloadTaskInClassLoader(ClassLoaders.java:153)
	... 6 more

This part of the classloader test have some limitations, but it should be ok for simple tests.

@bric3
Copy link
Contributor Author

bric3 commented Dec 13, 2016

I suggest to review this PR commit by commit.

Copy link
Member

@raphw raphw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the way to go. Much better to configure this with the plugin mechanism. Implementations looks good!

@raphw raphw merged commit ca5291c into release/2.x Dec 13, 2016
@raphw raphw removed the in progress label Dec 13, 2016
@bric3 bric3 deleted the enable-annoatation-engine-as-plugin branch December 20, 2016 13:43
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.

3 participants