-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Make ctor FrameworkField(Field) public? #1668
Comments
Prior to this change, custom runners could make FrameworkMethod instances, but not FrameworkField instances. This small change allows for both now, because FrameworkFields constructor has been promoted to public from package-private.
Could you use Edit: Stated another way, why does The request seems reasonable, but I'm wondering what larger problem you are trying to solve. If there's a way to solve it without upgrading JUnit, that may be better, since 1) if your solution required an upgrade your users would have to upgrade JUnit before they could upgrade your library, and 2) the JUnit team is focused no JUnit 5, so we might not release another version of 4.x for a while, if ever. |
@kcooney Thanks for responding. junit-quickcheck looks for the annotated methods it's interested in not just on the superclass hierarchy, but also the implemented interface hierarchy. I felt like overriding for (Class<?> eachClass : getSuperClasses(clazz)) {
for (Method eachMethod : MethodSorter.getDeclaredMethods(eachClass)) {
addToAnnotationLists(new FrameworkMethod(eachMethod), methodsForAnnotations);
}
// ensuring fields are sorted to make sure that entries are inserted
// and read from fieldForAnnotations in a deterministic order
for (Field eachField : getSortedDeclaredFields(eachClass)) {
addToAnnotationLists(new FrameworkField(eachField), fieldsForAnnotations);
}
} So Maybe there's a cleaner way? |
@pholser Thanks for the detailed response. It sounds really cool. Unfortunately, I don't see a way to do what you want now using existing public or protected extension points. Long term, we could either make the Assuming that none of the other maintainers object to your pull, you could update junit-quickcheck to create Note in general, using reflection to access non-public APIs in JUnit is risky. We could easily break you without knowing. But if your pull request is accepted, then the risk of using reflection to access the soon-to-be-public constructor is low (assuming the security manager and module system allows it). |
Prior to this change, custom runners could make `FrameworkMethod` instances, but not `FrameworkField` instances. This small change allows for both now, because `FrameworkField`'s constructor has been promoted to `public` from package-private.
@kcooney Thanks for your response. In the short term, I'll try reflectively invoking the actor and see how far that gets me. Appreciate the help! |
@pholser There's currently no plan to release another 4.x version of JUnit. Thus, I'm afraid you'll have to live with the workaround for the foreseeable future. |
@marcphilipp Thanks -- I figured as much. Working around it shouldn't be a big deal. |
@marcphilipp I personally think there is no harm in accepting the pull request (even if we never release another 4.x version of JUnit). If nothing else, it makes it clear that the constructor of |
@kcooney No objections. |
…d-ctor-public Make FrameworkField constructor public. Fixes #1668
Hello,
In junit-quickcheck I found myself wanting to make both
FrameworkMethod
s andFrameworkField
s.FrameworkMethod
's constructor is public;FrameworkField
's constructor is package-private. I tried to be sneaky and make the class that uses theFrameworkField
constructor a package cohabitant ofFrameworkField
. Now, however, some of my users are reporting an error when running junit-quickcheck tests, along the lines of this issue.In the short term, I'd try to clone
FrameworkField
, put it in a junit-quickcheck package, and move on -- except thatFrameworkField
contains package-private abstract methods that I won't be able to override (better for those to be markedprotected
?)Longer term -- would it be possible to promote
FrameworkField
's actor to public?Thanks for your consideration.
The text was updated successfully, but these errors were encountered: