Rename Java Input files and test methods to specify Test functionality

[MANISH-K-07]

Response to discussion at #14369 (comment)

Findings from PR #14369 :

Input file -

public class InputJavadocVariableNoJavadocOne          //naming like '1' or 'One' need to be omitted
public class InputJavadocVariableOnInnerClassFields    //preferred based on test functionality

Test Method -

@Test
public void testScopesOne() {                              //naming like '1' or 'One' need to be omitted
public void testJavadocVariableOnInnerClassFields() {      //preferred based on test functionality

Nonsense test method/input file naming conventions like this need to be avoided. We must come up with some descriptive name to differentiate these tests, considering the content and their functionality. Naming conventions like "........One" in both test method names and test inputs are being followed at present which tell us nothing about the purpose of a given test. Imagine that it is required to come back to this test class in near future. Why does this test exist? What did the author want to validate?

A precise naming convention needs to be enforced to give a better idea of what we are testing, without even needing to open up the input file and inspect the configuration and code. Precise naming as mentioned above is an outcome of clear understanding of the check and what it tests, it's functionality or purpose.

Fixing all the Java Input files in checkstyle is possible only through active contributions. So, first of all, a huge Thanks to every new contributor. If you are looking forward to working on this issue, be sure to analyze the test and look at the example PR :

Examples of ideal updates :

• 6d4fe4f
• b08ea45
• 8673001

Each PR must contain one complete module with all existing filenames and test method names analysed and renamed.
In update, naming should be such that, just by looking at the names, anyone can figure out what the test validates from input file without even having to open it.

Test method name and input file name should be similar and describe what we are testing. Example from link above:

• Test method name is testJavadocVariableOnInnerClassFields
• Input file name is InputJavadocVariableCheckOnInnerClassFields.java

Update in compliance with #6207 (comment) : Input files should be named in the pattern "Input{CheckName}CheckXxxx.java" . Here, the word "check" is stressed upon for all renamed input files of src/test path.

Thanks a lot !!

[nrmancuso]

@MANISH-K-07 how will we find such test methods and input files? How can we prevent this from happening in the future? If we can't automate some solution for this, I can tell you that we will not be able to always remember to use good naming practices.

[nrmancuso]

Please keep communication in PRs and issues short and concise. Your comment at #14436 (comment) can basically be summed as "I don't know".

I could manually review all PRs

As stated at #14436 (comment), this will not be an option. if something can't be completely automated, it's just not going to happen consistently.

The maintainers of Checkstyle are working professionals.
Please do not waste their time and avoid sending PR with unsatisfactory naming.

Please keep condescending/off topic remarks like this completely out of all interactions in our repos/organization.

[nrmancuso]

Research similar issues in our issue tracker to get some ideas of what we could do here.

[romani]

I approved issue to fix what we detected/detecting manually. It can be long ongoing issue with many PRs

[MANISH-K-07]

I approved issue to fix what we detected/detecting manually. It can be long ongoing issue with many PRs

That was the idea, yes
:)

[MANISH-K-07]

I am on AnnotationOnSameLineCheckTest

[abhishekmaity]

I'm on NestedForDepthCheckTest

[romani]

I want to re-question necessity of this issue.
detected at https://github.com/checkstyle/checkstyle/pull/14633/files#diff-624d1aa1b8e49bbaed857fe116c7d91c06325ed8c348393eab80a03a5e6b1f87R17

- public class InputMagicNumberDefault3 {
+ public class InputMagicNumberCheckHashCodeValue {

InputMagicNumberCheckHashCodeValue is not valid name as there bunch of other test input code. We do not group code in Inputs by meaning, it is just a placeholders that are limited to 120 lines. that is why we got such indexes 1,2,3 . as has to split files, and there is no good name, for just another input code for default config. When we need to add few lines, contributr can create new file or reuse existing.
Is there good naming for numerous names for default?

[rnveach]

I also want to remind that longer file names is not always good. Windows constraints is putting some of its users on edge of not being able to clone the repo.
checkstyle/contribution#840

[MANISH-K-07]

We do not group code in Inputs by meaning,

I agree that the input files haven't been grouped/split to follow a specific pattern.
However, most of them have atleast one distinguishing feature that separates this file from the rest. For example, 'InnerClassFields' as mentioned in issue description, or maybe 'method def', 'local var', etc and so on.. I have been suggesting in all related PRs to find that distinguishing element and name the file/test based on this. This mostly holds true even for the files with default config.

I also want to remind that longer file names is not always good.

More than half of the name for any file in our test area is occupied by the pattern that we are enforcing
Input{CheckName}CheckXxxxx.java
Some of our {CheckName}s themself are too big, infact most of them are..

[romani]

I see how people are struggling to do updates for this issue.

I also struggling to suggest better name.

I don't see future of such updates. All existing are ok to stay as is. New inputs we will try to request better names.

If we will find good pattern we will reopen this issue