Skip to content

Commit

Permalink
Issue #12586: Optimize testing methods to run check one time over a file
Browse files Browse the repository at this point in the history
  • Loading branch information
@mahfouz72 @nrmancuso
mahfouz72 authored and nrmancuso committed May 10, 2024
1 parent 14c818e commit c34d756
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 45 deletions.
53 changes: 51 additions & 2 deletions src/test/java/com/puppycrawl/tools/checkstyle/AbstractModuleTestSupport.java
View file
Expand Up @@ -254,8 +254,11 @@ protected final void verifyWithInlineConfigParser(String filePath, String... exp
InlineConfigParser.parse(filePath);
final DefaultConfiguration parsedConfig =
testInputConfiguration.createConfiguration();
verifyViolations(parsedConfig, filePath, testInputConfiguration.getViolations());
verify(parsedConfig, filePath, expected);
final List<String> actualViolations = getActualViolationsForFile(parsedConfig, filePath);
verifyViolations(filePath, testInputConfiguration.getViolations(), actualViolations);
assertWithMessage("Violations for %s differ.", filePath)
.that(actualViolations)
.containsExactlyElementsIn(expected);
}

/**
Expand Down Expand Up @@ -317,6 +320,25 @@ protected final void verifyWithInlineConfigParser(String filePath1,
filePath2, expectedFromFile2));
}

/**
* Performs verification of the file with the given file path using specified configuration
* and the array expected messages. Also performs verification of the config specified in
* input file
*
* @param filePath file path to verify.
* @param expected an array of expected messages.
* @throws Exception if exception occurs during verification process.
*/
protected void verifyWithInlineConfigParserTwice(String filePath, String... expected)
throws Exception {
final TestInputConfiguration testInputConfiguration =
InlineConfigParser.parse(filePath);
final DefaultConfiguration parsedConfig =
testInputConfiguration.createConfiguration();
verifyViolations(parsedConfig, filePath, testInputConfiguration.getViolations());
verify(parsedConfig, filePath, expected);
}

/**
* Performs verification of the file with the given file name. Uses specified configuration.
* Expected messages are represented by the array of strings.
Expand Down Expand Up @@ -509,6 +531,33 @@ private void verifyViolations(Configuration config,
}
}

/**
* Performs verification of violation lines.
*
* @param file file path.
* @param testInputViolations List of TestInputViolation objects.
* @param actualViolations for a file
*/
private static void verifyViolations(String file,
List<TestInputViolation> testInputViolations,
List<String> actualViolations) {
final List<Integer> actualViolationLines = actualViolations.stream()
.map(violation -> violation.substring(0, violation.indexOf(':')))
.map(Integer::valueOf)
.collect(Collectors.toUnmodifiableList());
final List<Integer> expectedViolationLines = testInputViolations.stream()
.map(TestInputViolation::getLineNo)
.collect(Collectors.toUnmodifiableList());
assertWithMessage("Violation lines for %s differ.", file)
.that(actualViolationLines)
.isEqualTo(expectedViolationLines);
for (int index = 0; index < actualViolations.size(); index++) {
assertWithMessage("Actual and expected violations differ.")
.that(actualViolations.get(index))
.matches(testInputViolations.get(index).toRegex());
}
}

/**
* Tests the file with the check config.
*
Expand Down
22 changes: 11 additions & 11 deletions src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java
View file
Expand Up @@ -98,7 +98,7 @@ public void testProperFileExtension() throws Exception {
"10:27: " + getCheckMessage(ConstantNameCheck.class,
MSG_INVALID_PATTERN, "k", "^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"),
};
verifyWithInlineConfigParser(path, expected);
verifyWithInlineConfigParserTwice(path, expected);
}

/**
Expand Down Expand Up @@ -133,7 +133,7 @@ public void testNoAuditEventsWithoutFilters() throws Exception {
Mockito.mockConstruction(TreeWalkerAuditEvent.class, (mock, context) -> {
throw new CheckstyleException("No audit events expected");
})) {
verifyWithInlineConfigParser(getPath("InputTreeWalker.java"), expected);
verifyWithInlineConfigParserTwice(getPath("InputTreeWalker.java"), expected);
}
}

Expand All @@ -158,7 +158,7 @@ public void testConditionRequiredWithoutOrdinaryChecks() throws Exception {
// This will re-enable walk(..., AstState.WITH_COMMENTS)
parser.when(() -> JavaParser.appendHiddenCommentNodes(mockAst)).thenReturn(realAst);

verifyWithInlineConfigParser(path, expected);
verifyWithInlineConfigParserTwice(path, expected);
}
}

Expand All @@ -178,7 +178,7 @@ public void testConditionRequiredWithoutCommentChecks() throws Exception {
parser.when(() -> JavaParser.appendHiddenCommentNodes(any(DetailAST.class)))
.thenThrow(IllegalStateException.class);

verifyWithInlineConfigParser(getPath("InputTreeWalker.java"), expected);
verifyWithInlineConfigParserTwice(getPath("InputTreeWalker.java"), expected);
}
}

Expand All @@ -189,7 +189,7 @@ public void testImproperFileExtension() throws Exception {
final File tempFile = new File(temporaryFolder, "file.pdf");
Files.copy(originalFile.toPath(), tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(tempFile.getPath(), expected);
verifyWithInlineConfigParserTwice(tempFile.getPath(), expected);
}

@Test
Expand Down Expand Up @@ -354,7 +354,7 @@ public void testProcessNonJavaFilesWithoutException() throws Exception {
public void testWithCacheWithNoViolation() throws Exception {
final String path = getPath("InputTreeWalkerWithCacheWithNoViolation.java");
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(path, expected);
verifyWithInlineConfigParserTwice(path, expected);
}

@Test
Expand Down Expand Up @@ -416,7 +416,7 @@ public void testRequiredTokenIsEmptyIntArray() throws Exception {
writer.write(configComment);
}
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(file.getPath(), expected);
verifyWithInlineConfigParserTwice(file.getPath(), expected);
}

@Test
Expand Down Expand Up @@ -496,7 +496,7 @@ public void testBehaviourWithChecksAndFilters() throws Exception {
"^[a-z][a-zA-Z0-9]*$"),
};

verifyWithInlineConfigParser(
verifyWithInlineConfigParserTwice(
getPath("InputTreeWalkerSuppressionCommentFilter.java"),
expected);
}
Expand All @@ -509,7 +509,7 @@ public void testMultiCheckOrder() throws Exception {
"13:9: " + getCheckMessage(WhitespaceAroundCheck.class, "ws.notFollowed", "if"),
};

verifyWithInlineConfigParser(
verifyWithInlineConfigParserTwice(
getPath("InputTreeWalkerMultiCheckOrder.java"),
expected);
}
Expand All @@ -524,7 +524,7 @@ public void testMultiCheckOfSameTypeNoIdResultsInOrderingByHash() throws Excepti
"name.invalidPattern", "b", "^[a-z][a-z0-9][a-zA-Z0-9]*$"),
};

verifyWithInlineConfigParser(
verifyWithInlineConfigParserTwice(
getPath("InputTreeWalkerMultiCheckOrder2.java"),
expected);
}
Expand Down Expand Up @@ -614,7 +614,7 @@ public void testTreeWalkerFilterAbsolutePath() throws Exception {
+ "/InputTreeWalkerSuppressionXpathFilterAbsolute.java";

final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(filePath, expected);
verifyWithInlineConfigParserTwice(filePath, expected);
}

@Test
Expand Down
24 changes: 6 additions & 18 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractJavadocCheckTest.java
View file
Expand Up @@ -196,9 +196,7 @@ public void testPositionOne() throws Exception {
verifyWithInlineConfigParser(getPath("InputAbstractJavadocPositionOne.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 21, but verifyWithInlineConfigParser verify file twice
.isEqualTo(42);
.isEqualTo(21);
}

@Test
Expand All @@ -208,9 +206,7 @@ public void testPositionTwo() throws Exception {
verifyWithInlineConfigParser(getPath("InputAbstractJavadocPositionTwo.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 29, but verifyWithInlineConfigParser verify file twice
.isEqualTo(58);
.isEqualTo(29);
}

@Test
Expand All @@ -220,9 +216,7 @@ public void testPositionThree() throws Exception {
verifyWithInlineConfigParser(getPath("InputAbstractJavadocPositionThree.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 15, but verifyWithInlineConfigParser verify file twice
.isEqualTo(30);
.isEqualTo(15);
}

@Test
Expand All @@ -233,9 +227,7 @@ public void testPositionWithSinglelineCommentsOne() throws Exception {
getPath("InputAbstractJavadocPositionWithSinglelineCommentsOne.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 21, but verifyWithInlineConfigParser verify file twice
.isEqualTo(42);
.isEqualTo(21);
}

@Test
Expand All @@ -246,9 +238,7 @@ public void testPositionWithSinglelineCommentsTwo() throws Exception {
getPath("InputAbstractJavadocPositionWithSinglelineCommentsTwo.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 29, but verifyWithInlineConfigParser verify file twice
.isEqualTo(58);
.isEqualTo(29);
}

@Test
Expand All @@ -259,9 +249,7 @@ public void testPositionWithSinglelineCommentsThree() throws Exception {
getPath("InputAbstractJavadocPositionWithSinglelineCommentsThree.java"), expected);
assertWithMessage("Invalid number of javadocs")
.that(JavadocCatchCheck.javadocsNumber)
// until https://github.com/checkstyle/checkstyle/issues/12586
// actual javadoc count is 15, but verifyWithInlineConfigParser verify file twice
.isEqualTo(30);
.isEqualTo(15);
}

@Test
Expand Down
38 changes: 24 additions & 14 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheckTest.java
View file
Expand Up @@ -60,15 +60,16 @@ public void testTag() throws Exception {
final String[] expected = {
"15: " + getCheckMessage(MSG_WRITE_TAG, "@author", "Daniel Grenner"),
};
verifyWithInlineConfigParser(getPath("InputWriteTag.java"), expected);
verifyWithInlineConfigParserTwice(getPath("InputWriteTag.java"), expected);
}

@Test
public void testMissingFormat() throws Exception {
final String[] expected = {
"15: " + getCheckMessage(MSG_WRITE_TAG, "@author", "Daniel Grenner"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagMissingFormat.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagMissingFormat.java"), expected);
}

@Test
Expand All @@ -77,7 +78,8 @@ public void testTagIncomplete() throws Exception {
"16: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
"This class needs more code..."),
};
verifyWithInlineConfigParser(getPath("InputWriteTagIncomplete.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagIncomplete.java"), expected);
}

@Test
Expand All @@ -86,23 +88,26 @@ public void testDoubleTag() throws Exception {
"18: " + getCheckMessage(MSG_WRITE_TAG, "@doubletag", "first text"),
"19: " + getCheckMessage(MSG_WRITE_TAG, "@doubletag", "second text"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagDoubleTag.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagDoubleTag.java"), expected);
}

@Test
public void testEmptyTag() throws Exception {
final String[] expected = {
"19: " + getCheckMessage(MSG_WRITE_TAG, "@emptytag", ""),
};
verifyWithInlineConfigParser(getPath("InputWriteTagEmptyTag.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagEmptyTag.java"), expected);
}

@Test
public void testMissingTag() throws Exception {
final String[] expected = {
"20: " + getCheckMessage(MSG_MISSING_TAG, "@missingtag"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagMissingTag.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagMissingTag.java"), expected);
}

@Test
Expand All @@ -112,35 +117,39 @@ public void testMethod() throws Exception {
"Add a constructor comment"),
"36: " + getCheckMessage(MSG_WRITE_TAG, "@todo", "Add a comment"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagMethod.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagMethod.java"), expected);
}

@Test
public void testSeverity() throws Exception {
final String[] expected = {
"16: " + getCheckMessage(MSG_WRITE_TAG, "@author", "Daniel Grenner"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagSeverity.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagSeverity.java"), expected);
}

@Test
public void testIgnoreMissing() throws Exception {
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(getPath("InputWriteTagIgnore.java"), expected);
verifyWithInlineConfigParserTwice(getPath("InputWriteTagIgnore.java"), expected);
}

@Test
public void testRegularEx() throws Exception {
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(getPath("InputWriteTagRegularExpression.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagRegularExpression.java"), expected);
}

@Test
public void testRegularExError() throws Exception {
final String[] expected = {
"15: " + getCheckMessage(MSG_TAG_FORMAT, "@author", "ABC"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagExpressionError.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagExpressionError.java"), expected);
}

@Test
Expand All @@ -155,15 +164,16 @@ public void testEnumsAndAnnotations() throws Exception {
"33: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
"This annotation field needs more code..."),
};
verifyWithInlineConfigParser(getPath("InputWriteTagEnumsAndAnnotations.java"), expected);
verifyWithInlineConfigParserTwice(
getPath("InputWriteTagEnumsAndAnnotations.java"), expected);
}

@Test
public void testNoJavadocs() throws Exception {
final String[] expected = {
"13: " + getCheckMessage(MSG_MISSING_TAG, "null"),
};
verifyWithInlineConfigParser(getPath("InputWriteTagNoJavadoc.java"), expected);
verifyWithInlineConfigParserTwice(getPath("InputWriteTagNoJavadoc.java"), expected);
}

@Test
Expand All @@ -184,7 +194,7 @@ public void testWriteTagRecordsAndCompactCtors() throws Exception {
"62: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
"Failed to recognize 'record' introduced in Java 14."),
};
verifyWithInlineConfigParser(
verifyWithInlineConfigParserTwice(
getNonCompilablePath("InputWriteTagRecordsAndCompactCtors.java"), expected);
}

Expand Down

0 comments on commit c34d756

Please sign in to comment.