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

Issue #11214: specified violation message in EmptyLineSeparatorCheck #11408

Closed
wants to merge 1 commit into from
Closed

Issue #11214: specified violation message in EmptyLineSeparatorCheck #11408

wants to merge 1 commit into from

Conversation

Kevin222004
Copy link
Contributor

Issue #11214: specified violation message in EmptyLineSeparatorCheck

@romani
Copy link
Member

romani commented Mar 13, 2022

Tests are failing

@Kevin222004
Copy link
Contributor Author

@romani Is it important to remove the "com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyLineSeparatorCheck" This file from Inline config Parser [src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java] because if i remove it it is not excepting violation above and below message and in some file there is no other option rather than write violation above/below
or I have to find anything else

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

From CI:

Violation message should be specified on line 38 in /home/vsts/work/1/s/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorNewMethodDef.java
https://dev.azure.com/romanivanovjr/romanivanovjr/_build/results?buildId=8087&view=logs&j=c902ebb4-c9f8-5f09-4e17-ff78fbbc842e&t=9ca98c81-ff64-58f0-9d03-a23ac1c4a111&l=572

Please follow what is requested

Items

@Kevin222004
Copy link
Contributor Author

@romani should i shorten the name of class ?

@romani
Copy link
Member

romani commented Mar 16, 2022

I am not sure in context of what class you talking about.

@Kevin222004
Copy link
Contributor Author

I am not sure in context of what class you talking about.

https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorModifierUnderPackage.java

line no 14 is going to long

I am thinking to create a new class or rename the class

@nrmancuso
Copy link
Member

line no 14 is going to long

@Kevin222004 can you just wrap the line?

@Kevin222004
Copy link
Contributor Author

Kevin222004 commented Mar 16, 2022

line no 14 is going to long

@Kevin222004 can you just wrap the line?

I have tried it before but it is giving the error some thing like this
Screenshot from 2022-03-16 09-35-37

for this I think i have to use violation above but in violation above it is giving error
And I have to make changes in all the input files containing violation above/below pattern which is already defined because it is showing error after removing "com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyLineSeparatorCheck" from InlineConfigParser

@nrmancuso
Copy link
Member

nrmancuso commented Mar 16, 2022

@Kevin222004 try to wrap line after public class, put comment on same line as public class.

@Kevin222004
Copy link
Contributor Author

@Kevin222004 try to wrap line after public class, put comment on same line as public class.

@nick-mancuso Thank you for the help it is ok now

@Kevin222004
Copy link
Contributor Author

Kevin222004 commented Mar 16, 2022

@romani I am getting this error
Screenshot from 2022-03-17 00-40-35

I have run the coverage test in EmptylineseparaterCheck Test file it is successfully passed
Screenshot from 2022-03-17 00-44-52

@romani
Copy link
Member

romani commented Mar 16, 2022

Please learn about code coverage.
You modified test in such a way that reduces diversity of test data and some code is not covered any more.
https://github.com/checkstyle/checkstyle/wiki/How-to-run-certain-phases-and-validations#how-to-generate-ut-coverage-report

@Kevin222004
Copy link
Contributor Author

@romani can you please give a little more hint to solve coverage issue I have learnt about it But i Don't understand how to solve it

@nrmancuso
Copy link
Member

@Kevin222004 for future reference: it is more helpful to share CLI output than screenshots. In regards to code coverage, have you generated a Jacoco report as mentioned in #11408 (comment)?

@Kevin222004
Copy link
Contributor Author

@Kevin222004 for future reference: it is more helpful to share CLI output than screenshots. In regards to code coverage, have you generated a Jacoco report as mentioned in #11408 (comment)?

yes I have Generated the jacoco report but it is very big how i can share here

@nrmancuso
Copy link
Member

yes I have Generated the jacoco report but it is very big how i can share here

No need to share; take time to read and understand report, and try to figure out how we lost coverage as a result of your changes here.

@Kevin222004
Copy link
Contributor Author

Kevin222004 commented Mar 19, 2022

No need to share; take time to read and understand report, and try to figure out how we lost coverage as a result of your changes here.

@nick-mancuso according to error
[WARNING] Rule violated for class com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyLineSeparatorCheck: branches covered ratio is 0.99, but expected minimum is 1.00

Coverage is failing over here

Screenshot from 2022-03-19 11-01-10

I have write message where violation above/below is mentioned this time in my previous pr I have not make any changes in violation above/below pattern. In this EmptyLineSeparator check it is given error where violation above/below is mentioned if I cant write a proper message

@romani
Copy link
Member

romani commented Mar 19, 2022

Coverage was 100%, before your change, so it means you modified Input in such a way to cause it, please revert changes one by one to catch undesirable modification

@romani
Copy link
Member

romani commented Mar 21, 2022

Please fix drone CI failure

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Please be aware of #11446

Please remove line

<exclude>**/EmptyLineSeparatorCheckTest.class</exclude>

in your PR, this will guide you on what to update, just follow fixing failures of mvn verify

Item:

@Kevin222004
Copy link
Contributor Author

@romani please review

@Kevin222004
Copy link
Contributor Author

@romani I forget to fix drone ci, also fixed up pi testing

@romani
Copy link
Member

romani commented Mar 25, 2022

Please fix drone violationns

src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorRecordsAndCompactCtors.java:21: error: invalid method declaration; return type required
        public MyRecord1{} // violation ''COMPACT_CTOR_DEF' should be separated from previous line'
               ^

@Kevin222004
Copy link
Contributor Author

@romani can you please give any hint to solve this pi test whitespace error

Actual:
EmptyLineSeparatorCheck.java.html:

        if (ast.getLineNo() > 1 && !hasEmptyLineBefore(ast)) {

EmptyLineSeparatorCheck.java.html:
        if (lineNo >= number) {

Ignore:

Diff:
--- target/ignored.txt 2022-03-25 11:22:39.893156706 +0000
+++ target/actual.txt 2022-03-25 11:22:39.889156662 +0000
@@ -0,0 +1,2 @@
+EmptyLineSeparatorCheck.java.html:

        if (ast.getLineNo() > 1 && !hasEmptyLineBefore(ast)) {

+EmptyLineSeparatorCheck.java.html:
        if (lineNo >= number) {

Difference between 'Actual' and 'Ignore' lists is detected, lists should be equal.
build will be failed.

@romani
Copy link
Member

romani commented Mar 25, 2022

You placed comment somewhere in input and it resulted in this. You can revert comments one by one and run pitest to see which caused a problem.

@nrmancuso
Copy link
Member

@Kevin222004 please see remaining failure in pitest-whitespace, ping me for review once CI is happy.

@Kevin222004
Copy link
Contributor Author

Kevin222004 commented Mar 29, 2022

@nick-mancuso can you please give look to this drone ci error I have cleared it in my before commit But i feel that it might not be good

src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorRecordsAndCompactCtors.java:21: error: invalid method declaration; return type required
public MyRecord1{} // violation ''COMPACT_CTOR_DEF' should be separated from previous line'
^
src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorRecordsAndCompactCtors.java:22: error: invalid method declaration; return type required
MyRec1(int x){this("string");} // violation ''CTOR_DEF' .* separated from previous line'
^
src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorRecordsAndCompactCtors.java:35: error: invalid method declaration; return type required
public recCom1{} // violation ''COMPACT_CTOR_DEF' should be separated from previous line'
^
src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/whitespace/emptylineseparator/InputEmptyLineSeparatorRecordsAndCompactCtors.java:41: error: invalid method declaration; return type required
public recCom2{} // violation ''COMPACT_CTOR_DEF' should be separated from previous line'
^

I have change all the violation of 'COMPACT_CTOR_DEF' in 'METHOD_DEF'
and the Ci is successfully passing at that time pi-test coding is failing

Diff:
--- target/ignored.txt 2022-03-29 09:29:50.349910738 +0530
+++ target/actual.txt 2022-03-29 09:29:50.349910738 +0530
@@ -1,3 +1,4 @@
+EqualsAvoidNullCheck.java.html:

            if (field.findFirstToken(TokenTypes.IDENT) != null) {

FinalLocalVariableCheck.java.html:
                            && isSameVariables(storedVariable, variable)

FinalLocalVariableCheck.java.html:
                        == ast.getParent()) {

FinalLocalVariableCheck.java.html:
                if (ast.getParent().getType() == TokenTypes.SWITCH_RULE

Difference between 'Actual' and 'Ignore' lists is detected, lists should be equal.
build will be failed.

can you give some more hint on it

@romani
Copy link
Member

romani commented Mar 29, 2022

@Kevin222004, if this issue is hard for you please choose different Check and try to fix same problem in it. And come back to this pR later on.

This pull request was closed.
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.

None yet

3 participants