-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 #11514: Improve code coverage by adding a new ParseTreeTableModelTest #13074
Conversation
thanks a lot !! Lines 834 to 849 in 70946a8
to make it clear that coverage improved. and/or: Lines 868 to 882 in 70946a8
if this PR does not cover all, we can just update coverage numbers, make CI green, and improve coverage in next PR (to operate by small PRs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
items:
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
Hi @romani, This PR doesn't cover everything, I only added test cases for the ParseTreeTableModel class. Can I remove only Thanks! |
just update to number got archived, to make sure nobody will not reduce % of coverage in this file in future. is some lines should stay , it is ok, we will cover them later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Items
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
Hi @romani, I've renamed the variable as Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Items
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
Hi @romani, I've updated the variable name and modified all other chained get calls to util method. Thanks. |
@alimhtsai , please reply done for each review items I opening. I will help a lot for you to double check that nothing is forgotten and for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
items:
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last item:
src/test/java/com/puppycrawl/tools/checkstyle/gui/ParseTreeTableModelTest.java
Outdated
Show resolved
Hide resolved
Please fix inspection CI failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to merge if CI pass
Solves issue #11514 Improve code coverage for GUI classes
mvn clean verify