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

Allow fields, methods, and constructors in Jupiter to be private #2479

Closed
rmannibucau opened this issue Nov 22, 2020 · 14 comments
Closed

Allow fields, methods, and constructors in Jupiter to be private #2479

rmannibucau opened this issue Nov 22, 2020 · 14 comments

Comments

@rmannibucau
Copy link
Contributor

Hi,

In java there is no real reason to support package scope member but not private ones so let's drop that constraint in next release please.

Romain

@sbrannen
Copy link
Member

Which type of field are you referring to?

@sbrannen
Copy link
Member

Based on a quick textual search in the code base, I assume you are referring to @RegisterExtension fields, but a confirmation would be appreciated.

@rmannibucau
Copy link
Contributor Author

@sbrannen yes but guess update must be more general on all members (thinking to TempDirectory too), @BeforeEach/@AfterEach etc...IMHO opening the "not public" door should mean enabling all until "private".

@sbrannen
Copy link
Member

Thanks for the feedback.

We've intentionally not supported finding private fields, methods, etc. since the beginning of JUnit 5, but I suppose we can discuss it again amongst the team.


Tentatively slated for 5.8 M1 solely for the purpose of team discussion

@sbrannen sbrannen added this to the 5.8 M1 milestone Nov 23, 2020
@sbrannen sbrannen changed the title Relax "field must not be private" Allow fields, methods, and constructors in Jupiter to be private Nov 23, 2020
@marcphilipp
Copy link
Member

Team decision: We support package-private methods because they're shorter to write and are in accordance with the convention of placing test classes in the same package as production code.

@rmannibucau
Copy link
Contributor Author

Side note: it violates several style validations and common practises in terms of visibility (why Test2 can check Test1 extensions?), in particular when integrating with 3rd parties (which almost all allows private injections) so overal it makes the test class inconsistent. Any hope it gets revised with wider scope than just junit scope?

@marcphilipp
Copy link
Member

What violates style validations?

@rmannibucau
Copy link
Contributor Author

@marcphilipp cause it is common to enable private fields and forbids protected or package ones due to the leak it does (regarding encapsulation). In other words it means your test style can't be your main code style (mixed with nested test classes it starts to becomes hard to validate but even when physically split it not not good to code differently depending folders of the same project IMHO).
More techincally, on an IDE point of view you get completion from friend classes which is also a noise you don't desire when writing tests IMHO.
Hope it is clearer written this way.

@marcphilipp
Copy link
Member

IIUC you're mostly concerned about fields not allowed to be private with @RegisterExtension?

@rmannibucau
Copy link
Contributor Author

@marcphilipp yep, funnily I was not "hit" by that on methods (guess rules I used are not very strict for methods).

@marcphilipp
Copy link
Member

I can understand your use case but it would be inconsistent to allow private on fields but not on methods and I really don't want to allow it for methods. 😕

@rmannibucau
Copy link
Contributor Author

@marcphilipp I can understand that as well. How about an engine parameter "allowedVisibility"? I know it is a toggle but not a crazy one IMHO.

@marcphilipp
Copy link
Member

At this point I don't think it's worth the added complexity.

@rmannibucau
Copy link
Contributor Author

Fun thing (not sure if "fun" is that right ;)) is that if you drop the "if private fail" check it works on private fields :(.

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

No branches or pull requests

3 participants