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 18, 2021
1 parent de4cb77 commit 2a98d5c
Show file tree
Hide file tree
Showing 22 changed files with 581 additions and 977 deletions.
35 changes: 2 additions & 33 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 Down Expand Up @@ -159,25 +147,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
10 changes: 0 additions & 10 deletions config/import-control-test.xml
View file
Expand Up @@ -17,11 +17,6 @@
<!-- 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 +30,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
23 changes: 4 additions & 19 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.1.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 @@ -316,24 +316,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>
<scope>test</scope>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>${mockito.version}</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
Expand Down
14 changes: 13 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,18 @@ 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 tests for
* easy mocking to gain 100% coverage.
*
* @param name name of the desired resource
* @return URI of the resource
*/
public 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
45 changes: 45 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,48 @@ public void testCalculateBufferLength() throws Exception {
assertEquals(54, result, "Buffer length is not expected");
}

/**
* This test mocks {@code AuditEvent} to emulate an event from a top level class
* to gain 100% coverage.
*/
@Test
public void testFormatTopLevelModuleNameContainsCheckSuffix() {
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);
}

/**
* This test mocks {@code AuditEvent} to emulate an event from a top level class
* to gain 100% coverage.
*/
@Test
public void testFormatTopLevelModuleNameDoesNotContainCheckSuffix() {
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
27 changes: 27 additions & 0 deletions src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java
View file
Expand Up @@ -22,8 +22,11 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
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 +39,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 +622,27 @@ public void testConstructors() throws Exception {
assertEquals(1, length, "Unexpected children size");
}

@Test
public void testConfigWithIgnoreExceptionalAttributes() {
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);
})) {
final CheckstyleException ex = assertThrows(CheckstyleException.class, () -> {
ConfigurationLoader.loadConfiguration(
getPath("InputConfigurationLoaderModuleIgnoreSeverity.xml"),
new PropertiesExpander(new Properties()), IgnoredModulesOptions.OMIT);
});
final String expectedMessage =
"Problem during accessing 'severity' attribute for MemberName";
assertWithMessage("Invalid exception cause message")
.that(ex)
.hasCauseThat()
.hasMessageThat()
.isEqualTo(expectedMessage);
}
}

}
52 changes: 52 additions & 0 deletions src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java
View file
Expand Up @@ -19,6 +19,7 @@

package com.puppycrawl.tools.checkstyle;

import static com.google.common.truth.Truth.assertWithMessage;
import static com.puppycrawl.tools.checkstyle.PackageObjectFactory.AMBIGUOUS_MODULE_NAME_EXCEPTION_MESSAGE;
import static com.puppycrawl.tools.checkstyle.PackageObjectFactory.BASE_PACKAGE;
import static com.puppycrawl.tools.checkstyle.PackageObjectFactory.CHECK_SUFFIX;
Expand All @@ -31,8 +32,10 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.mockStatic;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Arrays;
Expand All @@ -45,13 +48,17 @@
import java.util.Set;

import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;

import com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
import com.puppycrawl.tools.checkstyle.api.FileText;
import com.puppycrawl.tools.checkstyle.api.Violation;
import com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationLocationCheck;
import com.puppycrawl.tools.checkstyle.internal.utils.CheckUtil;
import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
import com.puppycrawl.tools.checkstyle.utils.ModuleReflectionUtil;

/**
* Enter a description of class PackageObjectFactoryTest.java.
Expand Down Expand Up @@ -393,6 +400,51 @@ public void testGetShortFromFullModuleNamesThirdParty() {
"Invalid simple check name");
}

/**
* This method is for testing the case of an exception caught inside
* {@code PackageObjectFactory.generateThirdPartyNameToFullModuleName}, a private method used
* to initialize private field {@code PackageObjectFactory.thirdPartyNameToFullModuleNames}.
* Since the method and the field both are private, the {@link TestUtil} is required to ensure
* that the field is changed. Also, the expected exception should be thrown from the static
* method {@link ModuleReflectionUtil#getCheckstyleModules}, so {@link Mockito#mockStatic}
* is required to mock this exception.
*
* @throws Exception when the code tested throws an exception
*/
@Test
public void testGenerateThirdPartyNameToFullModuleNameWithException() throws Exception {
final String name = "String";
final String packageName = "java.lang";
final ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
final Set<String> packages = Collections.singleton(packageName);
final PackageObjectFactory objectFactory = new PackageObjectFactory(packages, classLoader,
TRY_IN_ALL_REGISTERED_PACKAGES);

try (MockedStatic<ModuleReflectionUtil> utilities =
mockStatic(ModuleReflectionUtil.class)) {
utilities.when(() -> ModuleReflectionUtil.getCheckstyleModules(packages, classLoader))
.thenThrow(new IOException("mock exception"));

final String internalFieldName = "thirdPartyNameToFullModuleNames";
final Map<String, String> nullMap = TestUtil.getInternalState(objectFactory,
internalFieldName);
assertWithMessage("Expected uninitialized field")
.that(nullMap)
.isNull();

final Object instance = objectFactory.createModule(name);
assertWithMessage("Expected empty string")
.that(instance)
.isEqualTo("");

final Map<String, String> emptyMap = TestUtil.getInternalState(objectFactory,
internalFieldName);
assertWithMessage("Expected empty map")
.that(emptyMap)
.isEmpty();
}
}

private static final class FailConstructorFileSet extends AbstractFileSetCheck {

private FailConstructorFileSet() {
Expand Down

0 comments on commit 2a98d5c

Please sign in to comment.