Update checkstyle config and CodeClimate exclusions

- Add new checkstyle checks: require Javadoc on large private methods, default in switch, declaration order & others
- Update path exclusions in CodeClimate config to match newly renamed classes (e.g. PHPBB -> PhpBB)
  - Create consistency check testing that excluded paths exist as classes
- Fix some trivial violations
This commit is contained in:
ljacqu
2017-03-23 10:34:28 +01:00
parent e77828b228
commit 32a664ef59
27 changed files with 185 additions and 93 deletions
@@ -0,0 +1,51 @@
package fr.xephi.authme;
import fr.xephi.authme.util.Utils;
import org.bukkit.configuration.file.FileConfiguration;
import org.bukkit.configuration.file.YamlConfiguration;
import org.junit.Test;
import java.io.File;
import java.util.List;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertThat;
/**
* Consistency test for the CodeClimate configuration file.
*/
public class CodeClimateConfigTest {
private static final String CONFIG_FILE = ".codeclimate.yml";
@Test
public void shouldHaveExistingClassesInExclusions() {
// given
FileConfiguration configuration = YamlConfiguration.loadConfiguration(new File(CONFIG_FILE));
List<String> excludePaths = configuration.getStringList("exclude_paths");
// when / then
assertThat(excludePaths, not(empty()));
for (String path : excludePaths) {
String className = convertPathToQualifiedClassName(path);
assertThat("No class corresponds to excluded path '" + path + "'",
Utils.isClassLoaded(className), equalTo(true));
}
}
private static String convertPathToQualifiedClassName(String path) {
// Note ljacqu 20170323: In the future, we could have legitimate exclusions that don't fulfill these checks,
// in which case this test needs to be adapted accordingly.
if (!path.startsWith(TestHelper.SOURCES_FOLDER)) {
throw new IllegalArgumentException("Unexpected path '" + path + "': expected to start with sources folder");
} else if (!path.endsWith(".java")) {
throw new IllegalArgumentException("Expected path '" + path + "' to end with '.java'");
}
return path.substring(0, path.length() - ".java".length()) // strip ending .java
.substring(TestHelper.SOURCES_FOLDER.length()) // strip starting src/main/java
.replace('/', '.'); // replace '/' to '.'
}
}
@@ -26,7 +26,7 @@ import static fr.xephi.authme.command.help.HelpProvider.SHOW_COMMAND;
import static fr.xephi.authme.command.help.HelpProvider.SHOW_DESCRIPTION;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.given;
@@ -1,6 +1,7 @@
package fr.xephi.authme.command.executable.authme.debug;
import fr.xephi.authme.ClassCollector;
import fr.xephi.authme.TestHelper;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -22,8 +23,8 @@ public class DebugSectionConsistencyTest {
@BeforeClass
public static void collectClasses() {
debugClasses = new ClassCollector("src/main/java", "fr/xephi/authme/command/executable/authme/debug")
.collectClasses();
debugClasses = new ClassCollector(
TestHelper.SOURCES_FOLDER, TestHelper.PROJECT_ROOT + "command/executable/authme/debug").collectClasses();
}
@Test
@@ -48,7 +48,7 @@ public class EmailServiceTest {
@Mock
private Server server;
@Mock
private SendMailSSL sendMailSSL;
private SendMailSsl sendMailSsl;
@DataFolder
private File dataFolder;
@@ -66,7 +66,7 @@ public class EmailServiceTest {
given(server.getServerName()).willReturn("serverName");
given(settings.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("mail@example.org");
given(settings.getProperty(EmailSettings.MAIL_PASSWORD)).willReturn("pass1234");
given(sendMailSSL.hasAllInformation()).willReturn(true);
given(sendMailSsl.hasAllInformation()).willReturn(true);
}
@Test
@@ -82,17 +82,17 @@ public class EmailServiceTest {
.willReturn("Hi <playername />, your new password for <servername /> is <generatedpass />");
given(settings.getProperty(EmailSettings.PASSWORD_AS_IMAGE)).willReturn(false);
HtmlEmail email = mock(HtmlEmail.class);
given(sendMailSSL.initializeMail(anyString())).willReturn(email);
given(sendMailSSL.sendEmail(anyString(), eq(email))).willReturn(true);
given(sendMailSsl.initializeMail(anyString())).willReturn(email);
given(sendMailSsl.sendEmail(anyString(), eq(email))).willReturn(true);
// when
boolean result = emailService.sendPasswordMail("Player", "user@example.com", "new_password");
// then
assertThat(result, equalTo(true));
verify(sendMailSSL).initializeMail("user@example.com");
verify(sendMailSsl).initializeMail("user@example.com");
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMailSSL).sendEmail(messageCaptor.capture(), eq(email));
verify(sendMailSsl).sendEmail(messageCaptor.capture(), eq(email));
assertThat(messageCaptor.getValue(),
equalTo("Hi Player, your new password for serverName is new_password"));
}
@@ -100,15 +100,15 @@ public class EmailServiceTest {
@Test
public void shouldHandleMailCreationError() throws EmailException {
// given
doThrow(EmailException.class).when(sendMailSSL).initializeMail(anyString());
doThrow(EmailException.class).when(sendMailSsl).initializeMail(anyString());
// when
boolean result = emailService.sendPasswordMail("Player", "user@example.com", "new_password");
// then
assertThat(result, equalTo(false));
verify(sendMailSSL).initializeMail("user@example.com");
verify(sendMailSSL, never()).sendEmail(anyString(), any(HtmlEmail.class));
verify(sendMailSsl).initializeMail("user@example.com");
verify(sendMailSsl, never()).sendEmail(anyString(), any(HtmlEmail.class));
}
@Test
@@ -117,17 +117,17 @@ public class EmailServiceTest {
given(settings.getPasswordEmailMessage()).willReturn("Hi <playername />, your new pass is <generatedpass />");
given(settings.getProperty(EmailSettings.PASSWORD_AS_IMAGE)).willReturn(false);
HtmlEmail email = mock(HtmlEmail.class);
given(sendMailSSL.initializeMail(anyString())).willReturn(email);
given(sendMailSSL.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(false);
given(sendMailSsl.initializeMail(anyString())).willReturn(email);
given(sendMailSsl.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(false);
// when
boolean result = emailService.sendPasswordMail("bobby", "user@example.com", "myPassw0rd");
// then
assertThat(result, equalTo(false));
verify(sendMailSSL).initializeMail("user@example.com");
verify(sendMailSsl).initializeMail("user@example.com");
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMailSSL).sendEmail(messageCaptor.capture(), eq(email));
verify(sendMailSsl).sendEmail(messageCaptor.capture(), eq(email));
assertThat(messageCaptor.getValue(), equalTo("Hi bobby, your new pass is myPassw0rd"));
}
@@ -138,32 +138,32 @@ public class EmailServiceTest {
given(settings.getRecoveryCodeEmailMessage())
.willReturn("Hi <playername />, your code on <servername /> is <recoverycode /> (valid <hoursvalid /> hours)");
HtmlEmail email = mock(HtmlEmail.class);
given(sendMailSSL.initializeMail(anyString())).willReturn(email);
given(sendMailSSL.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(true);
given(sendMailSsl.initializeMail(anyString())).willReturn(email);
given(sendMailSsl.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(true);
// when
boolean result = emailService.sendRecoveryCode("Timmy", "tim@example.com", "12C56A");
// then
assertThat(result, equalTo(true));
verify(sendMailSSL).initializeMail("tim@example.com");
verify(sendMailSsl).initializeMail("tim@example.com");
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMailSSL).sendEmail(messageCaptor.capture(), eq(email));
verify(sendMailSsl).sendEmail(messageCaptor.capture(), eq(email));
assertThat(messageCaptor.getValue(), equalTo("Hi Timmy, your code on serverName is 12C56A (valid 7 hours)"));
}
@Test
public void shouldHandleMailCreationErrorForRecoveryCode() throws EmailException {
// given
given(sendMailSSL.initializeMail(anyString())).willThrow(EmailException.class);
given(sendMailSsl.initializeMail(anyString())).willThrow(EmailException.class);
// when
boolean result = emailService.sendRecoveryCode("Player", "player@example.org", "ABC1234");
// then
assertThat(result, equalTo(false));
verify(sendMailSSL).initializeMail("player@example.org");
verify(sendMailSSL, never()).sendEmail(anyString(), any(HtmlEmail.class));
verify(sendMailSsl).initializeMail("player@example.org");
verify(sendMailSsl, never()).sendEmail(anyString(), any(HtmlEmail.class));
}
@Test
@@ -173,17 +173,17 @@ public class EmailServiceTest {
given(settings.getRecoveryCodeEmailMessage()).willReturn("Hi <playername />, your code is <recoverycode />");
EmailService sendMailSpy = spy(emailService);
HtmlEmail email = mock(HtmlEmail.class);
given(sendMailSSL.initializeMail(anyString())).willReturn(email);
given(sendMailSSL.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(false);
given(sendMailSsl.initializeMail(anyString())).willReturn(email);
given(sendMailSsl.sendEmail(anyString(), any(HtmlEmail.class))).willReturn(false);
// when
boolean result = sendMailSpy.sendRecoveryCode("John", "user@example.com", "1DEF77");
// then
assertThat(result, equalTo(false));
verify(sendMailSSL).initializeMail("user@example.com");
verify(sendMailSsl).initializeMail("user@example.com");
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMailSSL).sendEmail(messageCaptor.capture(), eq(email));
verify(sendMailSsl).sendEmail(messageCaptor.capture(), eq(email));
assertThat(messageCaptor.getValue(), equalTo("Hi John, your code is 1DEF77"));
}
@@ -28,13 +28,13 @@ import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
/**
* Test for {@link SendMailSSL}.
* Test for {@link SendMailSsl}.
*/
@RunWith(DelayedInjectionRunner.class)
public class SendMailSSLTest {
public class SendMailSslTest {
@InjectDelayed
private SendMailSSL sendMailSSL;
private SendMailSsl sendMailSsl;
@Mock
private Settings settings;
@@ -57,7 +57,7 @@ public class SendMailSSLTest {
@Test
public void shouldHaveAllInformation() {
// given / when / then
assertThat(sendMailSSL.hasAllInformation(), equalTo(true));
assertThat(sendMailSsl.hasAllInformation(), equalTo(true));
}
@Test
@@ -73,7 +73,7 @@ public class SendMailSSLTest {
given(settings.getProperty(PluginSettings.LOG_LEVEL)).willReturn(LogLevel.DEBUG);
// when
HtmlEmail email = sendMailSSL.initializeMail("recipient@example.com");
HtmlEmail email = sendMailSsl.initializeMail("recipient@example.com");
// then
assertThat(email, not(nullValue()));
@@ -99,7 +99,7 @@ public class SendMailSSLTest {
given(settings.getProperty(EmailSettings.MAIL_SENDER_NAME)).willReturn(senderName);
// when
HtmlEmail email = sendMailSSL.initializeMail("recipient@example.com");
HtmlEmail email = sendMailSsl.initializeMail("recipient@example.com");
// then
assertThat(email, not(nullValue()));
@@ -122,7 +122,7 @@ public class SendMailSSLTest {
given(settings.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn(senderMail);
// when
HtmlEmail email = sendMailSSL.initializeMail("recipient@example.com");
HtmlEmail email = sendMailSsl.initializeMail("recipient@example.com");
// then
assertThat(email, not(nullValue()));
@@ -9,6 +9,7 @@ import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Multimap;
import fr.xephi.authme.ClassCollector;
import fr.xephi.authme.ReflectionTestUtils;
import fr.xephi.authme.TestHelper;
import fr.xephi.authme.datasource.DataSourceType;
import fr.xephi.authme.settings.properties.AuthMeSettingsRetriever;
import fr.xephi.authme.settings.properties.DatabaseSettings;
@@ -126,7 +127,7 @@ public class SettingsConsistencyTest {
private List<Method> getSectionCommentMethods() {
// Find all SettingsHolder classes
List<Class<? extends SettingsHolder>> settingsClasses =
new ClassCollector("src/main/java", "fr/xephi/authme/settings/properties/")
new ClassCollector(TestHelper.SOURCES_FOLDER, TestHelper.PROJECT_ROOT + "settings/properties/")
.collectClasses(SettingsHolder.class);
checkArgument(!settingsClasses.isEmpty(), "Could not find any SettingsHolder classes");