Skip to content

Commit

Permalink
Issue checkstyle#6439: move powermock tests away from the normal tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rnveach
rnveach committed Feb 20, 2019
1 parent 88907c5 commit 68bb5b0
Show file tree
Hide file tree
Showing 50 changed files with 2,718 additions and 1,771 deletions.
7 changes: 7 additions & 0 deletions config/import-control-test.xml
View file
Expand Up @@ -7,6 +7,13 @@

<disallow pkg="junit.framework" />

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

<allow pkg=".*" regex="true" />

</import-control>
2 changes: 2 additions & 0 deletions config/suppressions.xml
View file
Expand Up @@ -97,4 +97,6 @@

<!-- HandlerFactory crosses allowed limit for executable statements -->
<suppress checks="ExecutableStatementCount" files="HandlerFactory\.java"/>

<suppress id="ImportControlTest" files="[\\/]powermock[\\/]" message=".* - org\.(powermock|mockito).*" />
</suppressions>
4 changes: 2 additions & 2 deletions src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java
View file
Expand Up @@ -57,7 +57,7 @@
* configuration has changed.
*
*/
final class PropertyCacheFile {
public final class PropertyCacheFile {

/**
* The property key to use for storing the hashcode of the
Expand Down Expand Up @@ -93,7 +93,7 @@ final class PropertyCacheFile {
* @param config the current configuration, not null
* @param fileName the cache file
*/
PropertyCacheFile(Configuration config, String fileName) {
public PropertyCacheFile(Configuration config, String fileName) {
if (config == null) {
throw new IllegalArgumentException("config can not be null");
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java
View file
Expand Up @@ -40,7 +40,7 @@
/**
* Responsible for loading the contents of an import control configuration file.
*/
final class ImportControlLoader extends XmlLoader {
public final class ImportControlLoader extends XmlLoader {

/** The public ID for the configuration dtd. */
private static final String DTD_PUBLIC_ID_1_0 =
Expand Down
39 changes: 0 additions & 39 deletions src/test/java/com/puppycrawl/tools/checkstyle/AuditEventDefaultFormatterTest.java
View file
Expand Up @@ -20,23 +20,16 @@
package com.puppycrawl.tools.checkstyle;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.when;

import java.lang.reflect.Method;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.reflect.Whitebox;

import com.puppycrawl.tools.checkstyle.api.AuditEvent;
import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
import com.puppycrawl.tools.checkstyle.api.SeverityLevel;

@RunWith(PowerMockRunner.class)
@PrepareForTest(AuditEvent.class)
public class AuditEventDefaultFormatterTest {

@Test
Expand Down Expand Up @@ -65,38 +58,6 @@ public void testFormatFullyQualifiedModuleNameDoesNotContainCheckSuffix() {
assertEquals("Invalid format", expected, formatter.format(event));
}

@Test
public void testFormatModuleNameContainsCheckSuffix() {
final AuditEvent mock = PowerMockito.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]";

assertEquals("Invalid format", expected, formatter.format(mock));
}

@Test
public void testFormatModuleNameDoesNotContainCheckSuffix() {
final AuditEvent mock = PowerMockito.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]";

assertEquals("Invalid format", expected, formatter.format(mock));
}

@Test
public void testFormatModuleWithModuleId() {
final LocalizedMessage message = new LocalizedMessage(1, 1, null, null, null,
Expand Down
220 changes: 0 additions & 220 deletions src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java
View file
Expand Up @@ -23,21 +23,17 @@
import static com.puppycrawl.tools.checkstyle.DefaultLogger.AUDIT_FINISHED_MESSAGE;
import static com.puppycrawl.tools.checkstyle.DefaultLogger.AUDIT_STARTED_MESSAGE;
import static com.puppycrawl.tools.checkstyle.checks.NewlineAtEndOfFileCheck.MSG_KEY_NO_NEWLINE_EOF;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.when;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOError;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
Expand All @@ -62,7 +58,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.reflect.Whitebox;

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
Expand Down Expand Up @@ -661,152 +656,6 @@ public void testClearCacheWhenCacheFileIsNotSet() {
Whitebox.getInternalState(checker, "cacheFile"));
}

@Test
public void testCatchErrorInProcessFilesMethod() throws Exception {
// The idea of the test is to satisfy coverage rate.
// An Error indicates serious problems that a reasonable application should not try to
// catch, but due to issue https://github.com/checkstyle/checkstyle/issues/2285
// we catch errors in 'processFiles' method. Most such errors are abnormal conditions,
// that is why we use PowerMockito to reproduce them.
final File mock = PowerMockito.mock(File.class);
// Assume that I/O error is happened when we try to invoke 'lastModified()' method.
final String errorMessage = "Java Virtual Machine is broken"
+ " or has run out of resources necessary for it to continue operating.";
final Error expectedError = new IOError(new InternalError(errorMessage));
when(mock.lastModified()).thenThrow(expectedError);
when(mock.getAbsolutePath()).thenReturn("testFile");
final Checker checker = new Checker();
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
// -@cs[IllegalCatchExtended] Testing for catch Error is part of 100% coverage.
catch (Error error) {
assertThat("Error cause differs from IOError",
error.getCause(), instanceOf(IOError.class));
assertThat("Error cause is not InternalError",
error.getCause().getCause(), instanceOf(InternalError.class));
assertEquals("Error message is not expected",
errorMessage, error.getCause().getCause().getMessage());
}
}

@Test
public void testCatchErrorWithNoFileName() throws Exception {
// The idea of the test is to satisfy coverage rate.
// An Error indicates serious problems that a reasonable application should not try to
// catch, but due to issue https://github.com/checkstyle/checkstyle/issues/2285
// we catch errors in 'processFiles' method. Most such errors are abnormal conditions,
// that is why we use PowerMockito to reproduce them.
final File mock = PowerMockito.mock(File.class);
// Assume that I/O error is happened when we try to invoke 'lastModified()' method.
final String errorMessage = "Java Virtual Machine is broken"
+ " or has run out of resources necessary for it to continue operating.";
final Error expectedError = new IOError(new InternalError(errorMessage));
when(mock.lastModified()).thenThrow(expectedError);
final Checker checker = new Checker();
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
// -@cs[IllegalCatchExtended] Testing for catch Error is part of 100% coverage.
catch (Error error) {
assertThat("Error cause differs from IOError",
error.getCause(), instanceOf(IOError.class));
assertThat("Error cause is not InternalError",
error.getCause().getCause(), instanceOf(InternalError.class));
assertEquals("Error message is not expected",
errorMessage, error.getCause().getCause().getMessage());
}
}

@Test
public void testCatchErrorWithCache() throws Exception {
final File cacheFile = temporaryFolder.newFile();

final DefaultConfiguration checkerConfig = new DefaultConfiguration("configuration");
checkerConfig.addAttribute("charset", StandardCharsets.UTF_8.name());
checkerConfig.addAttribute("cacheFile", cacheFile.getPath());

final File mock = PowerMockito.mock(File.class);
final String errorMessage = "Java Virtual Machine is broken"
+ " or has run out of resources necessary for it to continue operating.";
final Error expectedError = new IOError(new InternalError(errorMessage));
when(mock.getAbsolutePath()).thenReturn("testFile");
when(mock.getAbsoluteFile()).thenThrow(expectedError);
final Checker checker = new Checker();
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
// -@cs[IllegalCatchExtended] Testing for catch Error is part of 100% coverage.
catch (Error error) {
assertThat("Error cause differs from IOError",
error.getCause(), instanceOf(IOError.class));
assertEquals("Error message is not expected",
errorMessage, error.getCause().getCause().getMessage());

// destroy is called by Main
checker.destroy();

final Properties cache = new Properties();
cache.load(Files.newBufferedReader(cacheFile.toPath()));

assertEquals("Cache has unexpected size",
1, cache.size());
assertNull("testFile is not in cache",
cache.getProperty("testFile"));
}
}

@Test
public void testCatchErrorWithCacheWithNoFileName() throws Exception {
final File cacheFile = temporaryFolder.newFile();

final DefaultConfiguration checkerConfig = new DefaultConfiguration("configuration");
checkerConfig.addAttribute("charset", StandardCharsets.UTF_8.name());
checkerConfig.addAttribute("cacheFile", cacheFile.getPath());

final File mock = PowerMockito.mock(File.class);
final String errorMessage = "Java Virtual Machine is broken"
+ " or has run out of resources necessary for it to continue operating.";
final Error expectedError = new IOError(new InternalError(errorMessage));
when(mock.getAbsolutePath()).thenThrow(expectedError);
final Checker checker = new Checker();
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
// -@cs[IllegalCatchExtended] Testing for catch Error is part of 100% coverage.
catch (Error error) {
assertThat("Error cause differs from IOError",
error.getCause(), instanceOf(IOError.class));
assertEquals("Error message is not expected",
errorMessage, error.getCause().getCause().getMessage());

// destroy is called by Main
checker.destroy();

final Properties cache = new Properties();
cache.load(Files.newBufferedReader(cacheFile.toPath()));

assertEquals("Cache has unexpected size",
1, cache.size());
}
}

/**
* It is OK to have long test method name here as it describes the test purpose.
*/
Expand Down Expand Up @@ -1029,75 +878,6 @@ public void testExceptionWithCache() throws Exception {
}
}

@Test
public void testExceptionWithNoFileName() {
// The idea of the test is to satisfy coverage rate.
// An Error indicates serious problems that a reasonable application should not try to
// catch, but due to issue https://github.com/checkstyle/checkstyle/issues/2285
// we catch errors in 'processFiles' method. Most such errors are abnormal conditions,
// that is why we use PowerMockito to reproduce them.
final File mock = PowerMockito.mock(File.class);
final String errorMessage = "Security Exception";
final Exception expectedError = new SecurityException(errorMessage);
when(mock.getAbsolutePath()).thenThrow(expectedError);
final Checker checker = new Checker();
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
catch (CheckstyleException ex) {
assertThat("Error cause differs from SecurityException",
ex.getCause(), instanceOf(SecurityException.class));
assertEquals("Error message is not expected",
errorMessage, ex.getCause().getMessage());
}
}

@Test
public void testExceptionWithCacheAndNoFileName() throws Exception {
final File cacheFile = temporaryFolder.newFile();

final DefaultConfiguration checkerConfig = new DefaultConfiguration("configuration");
checkerConfig.addAttribute("charset", StandardCharsets.UTF_8.name());
checkerConfig.addAttribute("cacheFile", cacheFile.getPath());

// The idea of the test is to satisfy coverage rate.
// An Error indicates serious problems that a reasonable application should not try to
// catch, but due to issue https://github.com/checkstyle/checkstyle/issues/2285
// we catch errors in 'processFiles' method. Most such errors are abnormal conditions,
// that is why we use PowerMockito to reproduce them.
final File mock = PowerMockito.mock(File.class);
final String errorMessage = "Security Exception";
final Exception expectedError = new SecurityException(errorMessage);
when(mock.getAbsolutePath()).thenThrow(expectedError);
final Checker checker = new Checker();
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
final List<File> filesToProcess = new ArrayList<>();
filesToProcess.add(mock);
try {
checker.process(filesToProcess);
fail("IOError is expected!");
}
catch (CheckstyleException ex) {
assertThat("Error cause differs from SecurityException",
ex.getCause(), instanceOf(SecurityException.class));
assertEquals("Error message is not expected",
errorMessage, ex.getCause().getMessage());

// destroy is called by Main
checker.destroy();

final Properties cache = new Properties();
cache.load(Files.newBufferedReader(cacheFile.toPath()));

assertEquals("Cache has unexpected size",
1, cache.size());
}
}

@Test
public void testHaltOnExceptionOff() throws Exception {
final DefaultConfiguration checkConfig =
Expand Down

0 comments on commit 68bb5b0

Please sign in to comment.