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 #14937: Migrated section 3.2 Package Statement to follow chapter wise testing #15001

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

Zopsss
Copy link
Contributor

@Zopsss Zopsss commented Jun 16, 2024

issue #14937

migrated section 3.2 Package Statement to follow chapter wise testing.

This section uses 2 modules: LineLength & NoLineWrap.

section 3.3.2 uses LineLength module too, it had input file for it but did not had test class. It was using section 3.2's test class. So technically we had the line length's input file for section 3.3.2 but we were not using it anywhere in the test class.

And I had to combine the LineLength & NoLineWrap module test classes into single test class for section 3.2. So I created a new test class for LineLength module for section 3.3.2 which uses the correct input file.

We had a very long package statement input file for section 3.2 to test that we allow for very long package statements but we were not using it anywhere in the test class, so I created a new test class for it.

Deleted unnecessary code which did not contributed to ensure the correct implementation of the section from input files.

Also corrected the test url for section 4.4

@Zopsss Zopsss force-pushed the package-statement branch 3 times, most recently from 0b4dbed to 053df1d Compare June 16, 2024 18:34
@Zopsss
Copy link
Contributor Author

Zopsss commented Jun 17, 2024

GitHub, generate site

@Zopsss Zopsss force-pushed the package-statement branch 2 times, most recently from 5a08a7b to 0d506cd Compare June 17, 2024 05:35
@Zopsss
Copy link
Contributor Author

Zopsss commented Jun 17, 2024

I had to explicitly create a new test file to test very long package statement. It was not possible to do in single test file as the package names would be different ( rule32packagestatement & toolongpackagetotestcoveragegooglesjavastylerule):

https://github.com/checkstyle/checkstyle/blob/0d506cd06b758baa31c760fc838af4f0a9d1644a/src/it/java/com/google/checkstyle/test/chapter3filestructure/toolongpackagetotestcoveragegooglesjavastylerule/PackageStatementTest.java

@Zopsss
Copy link
Contributor Author

Zopsss commented Jun 17, 2024

GitHub, generate site

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.

item:

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.

items:

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.

Last

config/checkstyle-non-main-files-suppressions.xml Outdated Show resolved Hide resolved
@Zopsss
Copy link
Contributor Author

Zopsss commented Jun 18, 2024

can you restart the CI for cancelled checks? @romani

@romani
Copy link
Member

romani commented Jun 18, 2024

lets see if we can migrate - #15023
such failures are not blocking this PR.

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.

Last

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.

last

config/checkstyle-non-main-files-suppressions.xml Outdated Show resolved Hide resolved
@Zopsss
Copy link
Contributor Author

Zopsss commented Jun 19, 2024

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.

absolutely last :

config/checkstyle-non-main-files-suppressions.xml Outdated Show resolved Hide resolved
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.

ok to merge

@romani romani requested a review from rdiachenko June 19, 2024 17:03
Copy link
Contributor

@rdiachenko rdiachenko left a comment

Choose a reason for hiding this comment

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

lgtm

@rdiachenko rdiachenko assigned romani and unassigned rdiachenko Jun 19, 2024
@romani romani merged commit b03286a into checkstyle:master Jun 19, 2024
107 of 111 checks passed
@Zopsss Zopsss deleted the package-statement branch June 20, 2024 03:56
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