Issue #15026: Added support to check input file against whole google configuration for google IT tests #15028

Zopsss · 2024-06-18T14:49:41Z

created a new method: verifyWithWholeGoogleConfig() which helps us to check the input file against whole google configuration. We just require to pass the file path to the method and everything else will work like a magic 🔮🪄

Zopsss · 2024-06-18T14:51:37Z

...google/checkstyle/test/chapter4formatting/rule4852classannotations/ClassAnnotationsTest.java

    @Test
    public void testAnnotation() throws Exception {
-        final String[] listOfModules = {
-            "AnnotationLocationMostCases",
-            "InvalidJavadocPosition",
-        };
-
        final String filePath = getPath("InputClassAnnotations.java");
-
-        verifyWithGoogleConfigParser(listOfModules, filePath);
+        verifyWithWholeGoogleConfig(filePath);
    }


Example of using the new method 🐪🐫

src/it/java/org/checkstyle/base/AbstractItModuleTestSupport.java

romani

last minors:

src/it/java/com/google/checkstyle/test/base/AbstractGoogleModuleTestSupport.java

...oogle/checkstyle/test/chapter4formatting/rule4852classannotations/InputClassAnnotations.java

romani

ok to merge.

@rnveach , please share your concerns and think if we can improve in separate PR on them.
This PR is "blocking" migration to whole configs, that will be significant effort, but it will NOT touch Abstract classes, so we can do fixes in parallel.

src/it/java/com/google/checkstyle/test/base/AbstractGoogleModuleTestSupport.java

rnveach · 2024-06-19T14:15:30Z

src/it/java/com/google/checkstyle/test/base/AbstractGoogleModuleTestSupport.java

+            googleConfig.addChild(treeWalkerConfig);
+        }
+
+        verifyWithItConfig(googleConfig, filePath);


Same as other verifyWithItConfig comment.

This is also dropped for the same reason, but also this whole method should be removed eventually when we switch everything to the whole config.

@Zopsss Where is issue to eventually go back to convert other tests? This should be added to the end there and a comment until http://.... should be added to this method.

added until message

checkstyle/src/it/java/org/checkstyle/base/AbstractItModuleTestSupport.java

Lines 459 to 467 in fb67e1c

// until https://github.com/checkstyle/checkstyle/issues/14937

/**

* Performs the verification of the file with the given file path and config.

*

* @param config config to check against.

* @param filePath input file path.

* @throws Exception if exception occurs during verification process.

*/

protected void verifyWithItConfig(Configuration config, String filePath) throws Exception {

@Zopsss until was placed in wrong location. until is basically reminding us the mentioned method is to be removed. verifyWithItConfig is the way we will be proceeding now (unless something comes up), so this method should stay.

The note should be on verifyWithConfigParser in AbstractGoogle. We are only keeping it until all tests move over to full config testing. Once we do, it can be removed completely.

romani

items:

src/main/resources/google_checks.xml

rnveach · 2024-06-20T20:46:53Z

Blocked from merging by #15072 , but code updates and reviews can still be done.

romani · 2024-06-22T14:11:14Z

@rnveach , please review this PR as if it does not have update in google_config.xml (moved to separate PR).
if no other comments, please approve PR and I will make sure that #15072 is merged first and this PR is rebased.

rdiachenko

lgtm

src/it/java/com/google/checkstyle/test/base/AbstractGoogleModuleTestSupport.java

rnveach · 2024-06-23T03:53:26Z

@Zopsss Please rebase.

…le google configuration for google IT tests

romani

rebased in github
ok to merge

Zopsss commented Jun 18, 2024

View reviewed changes

Zopsss mentioned this pull request Jun 18, 2024

Issue #14937: Migrated section 3.2 Package Statement to follow chapter wise testing #15001

Merged

rnveach requested changes Jun 18, 2024

View reviewed changes

src/it/java/org/checkstyle/base/AbstractItModuleTestSupport.java Outdated Show resolved Hide resolved

src/it/java/org/checkstyle/base/AbstractItModuleTestSupport.java Outdated Show resolved Hide resolved

rnveach self-assigned this Jun 18, 2024

Zopsss force-pushed the new-verify-method branch 3 times, most recently from 332df57 to c3f84d0 Compare June 19, 2024 10:59

romani requested changes Jun 19, 2024

View reviewed changes

Zopsss force-pushed the new-verify-method branch from c3f84d0 to aff00e1 Compare June 19, 2024 14:04

romani approved these changes Jun 19, 2024

View reviewed changes

romani requested a review from rdiachenko June 19, 2024 14:10

rnveach requested changes Jun 19, 2024

View reviewed changes

Zopsss force-pushed the new-verify-method branch 3 times, most recently from 1b71a04 to fb67e1c Compare June 20, 2024 05:55

romani mentioned this pull request Jun 20, 2024

Define severity for google style config by variable #15053

Closed

romani requested changes Jun 20, 2024

View reviewed changes

src/main/resources/google_checks.xml Outdated Show resolved Hide resolved

Zopsss force-pushed the new-verify-method branch from fb67e1c to 2d4f429 Compare June 20, 2024 18:07

rnveach added the blocked label Jun 20, 2024

Zopsss force-pushed the new-verify-method branch from 2d4f429 to fd20d22 Compare June 20, 2024 20:47

rnveach mentioned this pull request Jun 20, 2024

Issue #15053: Made google style guide severity property define-able by variable #15072

Merged

Zopsss force-pushed the new-verify-method branch 2 times, most recently from 8a46eaf to 86a2044 Compare June 21, 2024 07:34

Zopsss mentioned this pull request Jun 21, 2024

Document System Variables usage for Windows OS #15089

Open

Zopsss force-pushed the new-verify-method branch from 86a2044 to 6550734 Compare June 22, 2024 11:06

rdiachenko approved these changes Jun 22, 2024

View reviewed changes

src/it/java/com/google/checkstyle/test/base/AbstractGoogleModuleTestSupport.java Show resolved Hide resolved

Zopsss force-pushed the new-verify-method branch from 221e3d8 to d789152 Compare June 23, 2024 01:25

Issue checkstyle#15026: Added support to check input file against who…

ee369eb

…le google configuration for google IT tests

romani force-pushed the new-verify-method branch from d789152 to ee369eb Compare June 23, 2024 13:45

romani approved these changes Jun 23, 2024

View reviewed changes

rnveach removed the blocked label Jun 23, 2024

rnveach approved these changes Jun 23, 2024

View reviewed changes

rnveach merged commit cf267f3 into checkstyle:master Jun 23, 2024
111 checks passed

Zopsss deleted the new-verify-method branch June 23, 2024 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #15026: Added support to check input file against whole google configuration for google IT tests #15028

Issue #15026: Added support to check input file against whole google configuration for google IT tests #15028

Zopsss commented Jun 18, 2024

Zopsss Jun 18, 2024

romani left a comment

romani left a comment •

edited

Loading

rnveach Jun 19, 2024

rnveach Jun 19, 2024

rnveach Jun 19, 2024

Zopsss Jun 20, 2024

Zopsss Jun 20, 2024 •

edited

Loading

rnveach Jun 20, 2024

Zopsss Jun 20, 2024

romani left a comment

rnveach commented Jun 20, 2024

romani commented Jun 22, 2024

rdiachenko left a comment

rnveach commented Jun 23, 2024

romani left a comment •

edited

Loading

	// until https://github.com/checkstyle/checkstyle/issues/14937
	/**
	* Performs the verification of the file with the given file path and config.
	*
	* @param config config to check against.
	* @param filePath input file path.
	* @throws Exception if exception occurs during verification process.
	*/
	protected void verifyWithItConfig(Configuration config, String filePath) throws Exception {

Issue #15026: Added support to check input file against whole google configuration for google IT tests #15028

Issue #15026: Added support to check input file against whole google configuration for google IT tests #15028

Conversation

Zopsss commented Jun 18, 2024

Zopsss Jun 18, 2024

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment • edited Loading

Choose a reason for hiding this comment

rnveach Jun 19, 2024

Choose a reason for hiding this comment

rnveach Jun 19, 2024

Choose a reason for hiding this comment

rnveach Jun 19, 2024

Choose a reason for hiding this comment

Zopsss Jun 20, 2024

Choose a reason for hiding this comment

Zopsss Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

rnveach Jun 20, 2024

Choose a reason for hiding this comment

Zopsss Jun 20, 2024

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

rnveach commented Jun 20, 2024

romani commented Jun 22, 2024

rdiachenko left a comment

Choose a reason for hiding this comment

rnveach commented Jun 23, 2024

romani left a comment • edited Loading

Choose a reason for hiding this comment

romani left a comment •

edited

Loading

Zopsss Jun 20, 2024 •

edited

Loading

romani left a comment •

edited

Loading