Make ctor FrameworkField(Field) public?

[pholser]

Hello,

In junit-quickcheck I found myself wanting to make both FrameworkMethods and FrameworkFields. FrameworkMethod's constructor is public; FrameworkField's constructor is package-private. I tried to be sneaky and make the class that uses the FrameworkField constructor a package cohabitant of FrameworkField. 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 that FrameworkField contains package-private abstract methods that I won't be able to override (better for those to be marked protected?)

Longer term -- would it be possible to promote FrameworkField's actor to public?

Thanks for your consideration.

[kcooney]

Could you use TestClass.getAnnotatedFields() or TestCass.getAnnotatedFields(Class<? extends Annotation> annotationClass)?

Edit: Stated another way, why does JUnitQuickcheckTestClass need to have a custom implementation for finding methods and fields?

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.

[pholser]

@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 TestClass#scanAnnotatedMembers was the reasonable way to accomplish substituting how such methods and fields are scraped. TestClass#scanAnnotatedMembers looks like this:

        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 JUnitQuickcheckTestClass kind of mimics the pattern, but traversing more classes/interfaces up in the hierarchy.

Maybe there's a cleaner way?

[kcooney]

@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 FrameworkField constructor public or we could add extension points to TestCass to make it easier to do what you want. We have some classes that have become harder to maintain because we have overridable methods in non-final classes, and making the constructor public is simpler, so I'm personally fine with your pull.

Assuming that none of the other maintainers object to your pull, you could update junit-quickcheck to create FrameworkField via reflection until we release the next version of JUnit and you are comfortable with having your users being forced to upgrade JUnit to upgrade your library.

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).

[pholser]

@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!

[marcphilipp]

@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.

[pholser]

@marcphilipp Thanks -- I figured as much. Working around it shouldn't be a big deal.

[kcooney]

@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 FrameworkField is expected to remain in its current form, so is safe to be called reflectively.

[marcphilipp]

@kcooney No objections.