Skip to content

Commit

Permalink
Issue #10932: remove dependency on Powermock from tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@pbludov
pbludov committed Dec 4, 2021
1 parent d870e61 commit 99c9f0d
Show file tree
Hide file tree
Showing 37 changed files with 669 additions and 1,240 deletions.
36 changes: 2 additions & 34 deletions .ci/pitest.sh
View file
Expand Up @@ -41,6 +41,8 @@ pitest-annotation|pitest-design \
|pitest-coding \
|pitest-regexp \
|pitest-meta \
|pitest-tree-walker \
|pitest-utils \
|pitest-java-ast-visitor)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=();
Expand All @@ -59,8 +61,6 @@ pitest-main)
pitest-header)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=(
"AbstractHeaderCheck.java.html:<td class='uncovered'><pre><span class=''> catch (final IOException ex) {</span></pre></td></tr>"
"AbstractHeaderCheck.java.html:<td class='uncovered'><pre><span class='survived'> throw new IllegalArgumentException(&#34;unable to load header&#34;, ex);</span></pre></td></tr>"
"RegexpHeaderCheck.java.html:<td class='covered'><pre><span class='survived'> isMatch = headerLineNo == headerSize</span></pre></td></tr>"
"RegexpHeaderCheck.java.html:<td class='covered'><pre><span class='survived'> || isMatch(line, headerLineNo);</span></pre></td></tr>"
);
Expand All @@ -81,27 +81,15 @@ pitest-common)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=(
"AuditEventDefaultFormatter.java.html:<td class='covered'><pre><span class='survived'> if (lastDotIndex == -1) {</span></pre></td></tr>"
"AuditEventDefaultFormatter.java.html:<td class='uncovered'><pre><span class=''> checkShortName = checkFullName.substring(0, checkFullName.lastIndexOf(SUFFIX));</span></pre></td></tr>"
"AuditEventDefaultFormatter.java.html:<td class='uncovered'><pre><span class=''> checkShortName = checkFullName;</span></pre></td></tr>"
"AuditEventDefaultFormatter.java.html:<td class='uncovered'><pre><span class='survived'> if (checkFullName.endsWith(SUFFIX)) {</span></pre></td></tr>"
"Checker.java.html:<td class='covered'><pre><span class='survived'> if (cacheFile != null &#38;&#38; cacheFile.isInCache(fileName, timestamp)</span></pre></td></tr>"
"DefaultLogger.java.html:<td class='covered'><pre><span class='survived'> closeError = errorStreamOptions == OutputStreamOptions.CLOSE;</span></pre></td></tr>"
"DefaultLogger.java.html:<td class='covered'><pre><span class='survived'> closeInfo = infoStreamOptions == OutputStreamOptions.CLOSE;</span></pre></td></tr>"
"DefaultLogger.java.html:<td class='covered'><pre><span class='survived'> if (closeError) {</span></pre></td></tr>"
"DefaultLogger.java.html:<td class='covered'><pre><span class='survived'> if (closeInfo) {</span></pre></td></tr>"
"DefaultLogger.java.html:<td class='covered'><pre><span class='survived'> if (severityLevel != SeverityLevel.IGNORE) {</span></pre></td></tr>"
"ConfigurationLoader.java.html:<td class='uncovered'><pre><span class=''> catch (final CheckstyleException ex) {</span></pre></td></tr>"
"ConfigurationLoader.java.html:<td class='uncovered'><pre><span class='survived'> + recentModule.getName(), ex);</span></pre></td></tr>"
"ConfigurationLoader.java.html:<td class='uncovered'><pre><span class='survived'> throw new SAXException(</span></pre></td></tr>"
"PackageObjectFactory.java.html:<td class='covered'><pre><span class='survived'> if (instance == null</span></pre></td></tr>"
"PackageObjectFactory.java.html:<td class='covered'><pre><span class='survived'> if (!name.contains(PACKAGE_SEPARATOR)) {</span></pre></td></tr>"
"PackageObjectFactory.java.html:<td class='covered'><pre><span class='survived'> if (thirdPartyNameToFullModuleNames == null) {</span></pre></td></tr>"
"PackageObjectFactory.java.html:<td class='uncovered'><pre><span class=''> returnValue = Collections.emptyMap();</span></pre></td></tr>"
"PackageObjectFactory.java.html:<td class='uncovered'><pre><span class=''> catch (IOException ignore) {</span></pre></td></tr>"
"PropertyCacheFile.java.html:<td class='covered'><pre><span class='survived'> if (!cachedHashSum.equals(contentHashSum)) {</span></pre></td></tr>"
"PropertyCacheFile.java.html:<td class='uncovered'><pre><span class=''> changed = true;</span></pre></td></tr>"
"PropertyCacheFile.java.html:<td class='uncovered'><pre><span class=''> catch (final IOException | NoSuchAlgorithmException ex) {</span></pre></td></tr>"
"PropertyCacheFile.java.html:<td class='uncovered'><pre><span class='survived'> throw new IllegalStateException(&#34;Unable to calculate hashcode.&#34;, ex);</span></pre></td></tr>"
);
checkPitestReport "${ignoredItems[@]}"
;;
Expand All @@ -111,7 +99,6 @@ pitest-ant)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=(
"CheckstyleAntTask.java.html:<td class='covered'><pre><span class='survived'> if (toFile == null || !useFile) {</span></pre></td></tr>"
"CheckstyleAntTask.java.html:<td class='covered'><pre><span class='survived'> if (toFile == null || !useFile) {</span></pre></td></tr>"
"CheckstyleAntTask.java.html:<td class='covered'><pre><span class='survived'> log(&#34;To process the files took &#34; + (processingEndTime - processingStartTime)</span></pre></td></tr>"
"CheckstyleAntTask.java.html:<td class='covered'><pre><span class='survived'> log(&#34;Total execution took &#34; + (endTime - startTime) + TIME_SUFFIX,</span></pre></td></tr>"
"CheckstyleAntTask.java.html:<td class='covered'><pre><span class='survived'> log(&#34;To locate the files took &#34; + (endTime - startTime) + TIME_SUFFIX,</span></pre></td></tr>"
Expand Down Expand Up @@ -159,25 +146,6 @@ pitest-javadoc)
checkPitestReport "${ignoredItems[@]}"
;;

pitest-tree-walker)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=(
"TreeWalker.java.html:<td class='covered'><pre><span class='survived'> if (!commentChecks.isEmpty()) {</span></pre></td></tr>"
"TreeWalker.java.html:<td class='covered'><pre><span class='survived'> if (!ordinaryChecks.isEmpty()) {</span></pre></td></tr>"
"TreeWalker.java.html:<td class='covered'><pre><span class='survived'> if (filters.isEmpty()) {</span></pre></td></tr>"
);
checkPitestReport "${ignoredItems[@]}"
;;

pitest-utils)
mvn --no-transfer-progress -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
declare -a ignoredItems=(
"CommonUtil.java.html:<td class='uncovered'><pre><span class=''> catch (final URISyntaxException ex) {</span></pre></td></tr>"
"CommonUtil.java.html:<td class='uncovered'><pre><span class='survived'> throw new CheckstyleException(UNABLE_TO_FIND_EXCEPTION_PREFIX + filename, ex);</span></pre></td></tr>"
);
checkPitestReport "${ignoredItems[@]}"
;;

# pitesttyle-gui)
# mvn -e -P$1 clean test org.pitest:pitest-maven:mutationCoverage;
# # post validation is skipped, we do not test gui throughly
Expand Down
13 changes: 0 additions & 13 deletions config/import-control-test.xml
View file
Expand Up @@ -11,17 +11,9 @@
|org\.checkstyle.*)"
regex="true">

<!-- Disallow obsolete Junit API -->
<disallow pkg="org.junit" exact-match="true"/>
<disallow pkg="junit.framework" />
<!-- until https://github.com/checkstyle/checkstyle/issues/9142 -->
<disallow pkg="org.junit.jupiter.api.Assertions"/>

<!-- Conflicts with normal tests and pitest.
See examples in https://github.com/checkstyle/checkstyle/issues/6439 -->
<allow class="org.mockito.internal.util.Checks" />
<disallow pkg="org\.powermock.*" regex="true" />
<disallow pkg="org\.mockito.*" regex="true" />
<!-- Reflection shouldn't be used in tests. -->
<disallow pkg="java\.lang\.reflect\.*" regex="true" />

Expand All @@ -35,11 +27,6 @@

</subpackage>
<subpackage name="internal">
<!-- Till https://github.com/checkstyle/checkstyle/issues/7368 -->
<subpackage name="powermock">
<allow pkg="java.lang.reflect" />
<allow pkg="org.junit" local-only="true"/>
</subpackage>
<subpackage name="utils">
<file name="CheckUtil">
<!-- Uses reflection to collect violation messages. -->
Expand Down
10 changes: 2 additions & 8 deletions config/intellij-idea-inspections.xml
View file
Expand Up @@ -4592,24 +4592,18 @@
enabled_by_default="true"/>
<inspection_tool class="TestMethodWithoutAssertion" enabled="true" level="ERROR"
enabled_by_default="true">
<!-- after https://github.com/powermock/powermock/issues/830
all junit4 methods should be removed -->
<option name="assertionMethods"
value="org.junit.Assert,assert.*|fail.*, ,
,org.junit.jupiter.api.Assertions,assert.*|fail.*, ,
,junit.framework.Assert,assert.*|fail.*, ,
value="org.junit.jupiter.api.Assertions,assert.*|fail.*, ,
,org.mockito.Mockito,verify.*, ,
,org.mockito.InOrder,verify, ,
,org.junit.rules.ExpectedException,expect.*, ,
,org.hamcrest.MatcherAssert,assertThat, ,
,com.google.common.truth.Truth,assert.*, ,
,com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport,verify.*, ,
,com.puppycrawl.tools.checkstyle.AbstractTreeTestSupport,verify.*, ,
,com.google.checkstyle.test.base.AbstractModuleTestSupport,verify.*, ,
,com.puppycrawl.tools.checkstyle.internal.TestUtil,assert.*, ,
,nl.jqno.equalsverifier.EqualsVerifier,verify.*, ,
,org.checkstyle.base.AbstractItModuleTestSupport,verify.*, ,
,org.powermock.api.mockito.PowerMockito,verify.*"/>
,org.checkstyle.base.AbstractItModuleTestSupport,verify.*"/>
<option name="assertKeywordIsAssertion" value="false"/>
</inspection_tool>
<inspection_tool class="TestNGDataProvider" enabled="true" level="ERROR"
Expand Down
5 changes: 2 additions & 3 deletions config/suppressions.xml
View file
Expand Up @@ -93,9 +93,6 @@
<!-- HandlerFactory crosses allowed limit for executable statements -->
<suppress checks="ExecutableStatementCount" files="HandlerFactory\.java"/>

<suppress id="ImportControlTest" files="[\\/]powermock[\\/]"
message=".* - org\.(powermock|mockito).*" />

<suppress id="noSourceforgeNetLinks" files="[\\/]releasenotes.xml"/>
<suppress id="noSourceforgeIoLinks" files="[\\/]google_style.xml" lines="31"/>
<suppress id="noSourceforgeIoLinks" files="[\\/]sun_style.xml" lines="31"/>
Expand Down Expand Up @@ -270,6 +267,8 @@
files="[\\/]src[\\/]test[\\/]java[\\/]com[\\/]puppycrawl[\\/]tools[\\/]checkstyle[\\/]checks[\\/]coding[\\/]IllegalTypeCheckTest.java"/>
<suppress id="ImportControlTest"
files="[\\/]src[\\/]test[\\/]java[\\/]com[\\/]puppycrawl[\\/]tools[\\/]checkstyle[\\/]checks[\\/]coding[\\/]InnerAssignmentCheckTest.java"/>
<suppress id="ImportControlTest"
files="[\\/]src[\\/]test[\\/]java[\\/]com[\\/]puppycrawl[\\/]tools[\\/]checkstyle[\\/]checks[\\/]coding[\\/]MatchXpathCheckTest.java"/>
<suppress id="ImportControlTest"
files="[\\/]src[\\/]test[\\/]java[\\/]com[\\/]puppycrawl[\\/]tools[\\/]checkstyle[\\/]checks[\\/]coding[\\/]MissingCtorCheckTest.java"/>
<suppress id="ImportControlTest"
Expand Down
40 changes: 4 additions & 36 deletions pom.xml
View file
Expand Up @@ -205,7 +205,7 @@
<maven.pmd.plugin.version>3.15.0</maven.pmd.plugin.version>
<pmd.version>6.41.0</pmd.version>
<maven.jacoco.plugin.version>0.8.7</maven.jacoco.plugin.version>
<powermock.version>2.0.9</powermock.version>
<mockito.version>4.0.0</mockito.version>
<saxon.version>10.6</saxon.version>
<maven.checkstyle.plugin.version>3.1.2</maven.checkstyle.plugin.version>
<maven.sevntu.checkstyle.plugin.version>1.40.0</maven.sevntu.checkstyle.plugin.version>
Expand Down Expand Up @@ -289,13 +289,6 @@
<version>1.5.0</version>
<scope>test</scope>
</dependency>
<!-- till https://github.com/checkstyle/checkstyle/issues/7368 -->
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<version>${junit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.truth</groupId>
<artifactId>truth</artifactId>
Expand All @@ -316,23 +309,9 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito2</artifactId>
<version>${powermock.version}</version>
<scope>test</scope>
</dependency>
<!-- till https://github.com/checkstyle/checkstyle/issues/7368 -->
<dependency>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
<version>3.28.0-GA</version>
<scope>test</scope>
</dependency>
<!-- till https://github.com/checkstyle/checkstyle/issues/7368 -->
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-module-junit4</artifactId>
<version>${powermock.version}</version>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>${mockito.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -1899,17 +1878,6 @@

<profiles>

<!-- Till https://github.com/powermock/powermock/issues/1094 -->
<profile>
<id>java16</id>
<activation>
<jdk>[16,)</jdk>
</activation>
<properties>
<surefire.options>--illegal-access=warn</surefire.options>
</properties>
</profile>

<profile>
<!-- To be used during development. Run the command -->
<!-- mvn -Pno-validations site -->
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
View file
Expand Up @@ -177,7 +177,7 @@ protected void processFiltered(File file, FileText fileText) throws CheckstyleEx
* @param rootAST root AST element {@link DetailAST} of the file
* @return filtered set of violations
*/
private SortedSet<Violation> getFilteredViolations(
/* package */ SortedSet<Violation> getFilteredViolations(
String fileName, FileContents fileContents, DetailAST rootAST) {
final SortedSet<Violation> result = new TreeSet<>(violations);
for (Violation element : violations) {
Expand Down Expand Up @@ -267,7 +267,7 @@ else if (TokenUtil.isCommentType(tokenId)) {
* @param contents the contents of the file the AST was generated from.
* @param astState state of AST.
*/
private void walk(DetailAST ast, FileContents contents,
/* package */ void walk(DetailAST ast, FileContents contents,
AstState astState) {
notifyBegin(ast, contents, astState);
processIter(ast, astState);
Expand Down Expand Up @@ -431,8 +431,10 @@ private static SortedSet<AbstractCheck> createNewCheckSortedSet() {
/**
* State of AST.
* Indicates whether tree contains certain nodes.
*
* @noinspection PackageVisibleInnerClass
*/
private enum AstState {
/* package */ enum AstState {

/**
* Ordinary tree.
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/com/puppycrawl/tools/checkstyle/utils/CommonUtil.java
View file
Expand Up @@ -447,7 +447,7 @@ private static URI getFilepathOrClasspathUri(String filename) throws CheckstyleE
public static URI getResourceFromClassPath(String filename) throws CheckstyleException {
final URL configUrl;
if (filename.charAt(0) == '/') {
configUrl = CommonUtil.class.getResource(filename);
configUrl = getCheckstyleResource(filename);
}
else {
configUrl = ClassLoader.getSystemResource(filename);
Expand All @@ -468,6 +468,17 @@ public static URI getResourceFromClassPath(String filename) throws CheckstyleExc
return uri;
}

/**
* Finds a resource with a given name in the Checkstyle resource bundle.
* This method is intended only for internal use in Checkstyle and tests.
*
* @param name name of the desired resource
* @return URI of the resource
*/
/* package */ static URL getCheckstyleResource(String name) {
return CommonUtil.class.getResource(name);
}

/**
* Puts part of line, which matches regexp into given template
* on positions $n where 'n' is number of matched part in line.
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/com/puppycrawl/tools/checkstyle/AuditEventDefaultFormatterTest.java
View file
Expand Up @@ -19,7 +19,10 @@

package com.puppycrawl.tools.checkstyle;

import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -80,6 +83,40 @@ public void testCalculateBufferLength() throws Exception {
assertEquals(54, result, "Buffer length is not expected");
}

@Test
public void testFormatModuleNameContainsCheckSuffix() {
final AuditEvent mock = mock(AuditEvent.class);
when(mock.getSourceName()).thenReturn("TestModuleCheck");
when(mock.getSeverityLevel()).thenReturn(SeverityLevel.WARNING);
when(mock.getLine()).thenReturn(1);
when(mock.getColumn()).thenReturn(1);
when(mock.getMessage()).thenReturn("Mocked message.");
when(mock.getFileName()).thenReturn("InputMockFile.java");
final AuditEventFormatter formatter = new AuditEventDefaultFormatter();

final String expected = "[WARN] InputMockFile.java:1:1: Mocked message. [TestModule]";
assertWithMessage("Invalid format")
.that(formatter.format(mock))
.isEqualTo(expected);
}

@Test
public void testFormatModuleNameDoesNotContainCheckSuffix() {
final AuditEvent mock = mock(AuditEvent.class);
when(mock.getSourceName()).thenReturn("TestModule");
when(mock.getSeverityLevel()).thenReturn(SeverityLevel.WARNING);
when(mock.getLine()).thenReturn(1);
when(mock.getColumn()).thenReturn(1);
when(mock.getMessage()).thenReturn("Mocked message.");
when(mock.getFileName()).thenReturn("InputMockFile.java");
final AuditEventFormatter formatter = new AuditEventDefaultFormatter();

final String expected = "[WARN] InputMockFile.java:1:1: Mocked message. [TestModule]";
assertWithMessage("Invalid format")
.that(formatter.format(mock))
.isEqualTo(expected);
}

private static class TestModuleCheck {

// no code
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java
View file
Expand Up @@ -24,6 +24,8 @@
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.when;

import java.io.File;
import java.lang.reflect.Constructor;
Expand All @@ -36,6 +38,7 @@
import java.util.Properties;

import org.junit.jupiter.api.Test;
import org.mockito.MockedConstruction;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

Expand Down Expand Up @@ -618,4 +621,30 @@ public void testConstructors() throws Exception {
assertEquals(1, length, "Unexpected children size");
}

@Test
public void testConfigWithIgnoreExceptionalAttributes() throws Exception {
try (MockedConstruction<DefaultConfiguration> mocked = mockConstruction(
DefaultConfiguration.class, (mock, context) -> {
when(mock.getPropertyNames()).thenReturn(new String[] {"severity"});
when(mock.getName()).thenReturn("MemberName");
when(mock.getProperty("severity")).thenThrow(CheckstyleException.class);
})) {
try {
ConfigurationLoader.loadConfiguration(
getPath("InputConfigurationLoaderModuleIgnoreSeverity.xml"),
new PropertiesExpander(new Properties()), IgnoredModulesOptions.OMIT);
fail("Exception is expected");
}
catch (CheckstyleException ex) {
final String expectedMessage =
"Problem during accessing 'severity' attribute for MemberName";
assertWithMessage("Invalid exception cause message")
.that(ex)
.hasCauseThat()
.hasMessageThat()
.isEqualTo(expectedMessage);
}
}
}

}

0 comments on commit 99c9f0d

Please sign in to comment.