Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply spotless and checkstyle #5390

Merged
merged 13 commits into from May 25, 2022
  •  
  •  
  •  
1 change: 1 addition & 0 deletions .github/workflows/ci-docker-wormhole.yml
Expand Up @@ -16,5 +16,6 @@ jobs:
-v "$HOME:$HOME" \
-v "$PWD:$PWD" \
-w "$PWD" \
-e AUTO_APPLY_GIT_HOOKS=false \
openjdk:8-jdk-alpine \
./gradlew --no-daemon --continue --scan testcontainers:test --tests '*GenericContainerRuleTest'
10 changes: 8 additions & 2 deletions build.gradle
@@ -1,11 +1,13 @@
buildscript {
repositories {
mavenCentral()
maven { url 'https://jitpack.io' }
}

dependencies {
// https://github.com/melix/japicmp-gradle-plugin/issues/36
classpath 'com.google.guava:guava:30.1.1-jre'
classpath 'com.github.kiview:captain-hook:76a1c11a16'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiview do you remember why we had to fork it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make it work with windows I guess https://github.com/kiview/captain-hook/commits/master

Copy link
Member

@kiview kiview May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, PR was never merged in upstream, since upstream seems to be dead 😞
tjni/captain-hook#4

}
}

Expand All @@ -17,6 +19,12 @@ plugins {
}

apply from: "$rootDir/gradle/ci-support.gradle"
apply plugin: 'com.github.tjni.captainhook'

captainHook {
autoApplyGitHooks = Boolean.valueOf(System.getenv("AUTO_APPLY_GIT_HOOKS"))
preCommit = './gradlew spotlessApply'
}

subprojects {
apply plugin: 'java-library'
Expand Down Expand Up @@ -110,7 +118,6 @@ subprojects {
}

spotless {
enforceCheck false
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
java {
removeUnusedImports()
trimTrailingWhitespace()
Expand All @@ -134,6 +141,5 @@ subprojects {
checkstyle {
toolVersion = "9.3"
configFile = rootProject.file('config/checkstyle/checkstyle.xml')
ignoreFailures = true
}
}
8 changes: 6 additions & 2 deletions config/checkstyle/checkstyle.xml
Expand Up @@ -11,8 +11,12 @@
LITERAL_FOR,
LITERAL_IF,
LITERAL_WHILE,
LITERAL_CASE,
LITERAL_DEFAULT,
"
/>
</module>
<module name="NeedBraces">
<property name="tokens"
value="
LAMBDA,
"
/>
Expand Down
Expand Up @@ -5,8 +5,7 @@
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.Paths;

import static java.util.Collections.emptyMap;
import java.util.Collections;

public abstract class AbstractJarFileTest {

Expand All @@ -16,7 +15,7 @@ public abstract class AbstractJarFileTest {
try {
Path jarFilePath = Paths.get(System.getProperty("jarFile"));
URI jarFileUri = new URI("jar", jarFilePath.toUri().toString(), null);
FileSystem fileSystem = FileSystems.newFileSystem(jarFileUri, emptyMap());
FileSystem fileSystem = FileSystems.newFileSystem(jarFileUri, Collections.emptyMap());
root = fileSystem.getPath("/");
} catch (Exception e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -13,34 +13,26 @@ public class JarFileShadingTest extends AbstractJarFileTest {

@Test
public void testPackages() throws Exception {
assertThatFileList(root).containsOnly(
"org",
"META-INF"
);

assertThatFileList(root.resolve("org")).containsOnly(
"testcontainers"
);
assertThatFileList(root).containsOnly("org", "META-INF");

assertThatFileList(root.resolve("org")).containsOnly("testcontainers");
}

@Test
public void testMetaInf() throws Exception {
assertThatFileList(root.resolve("META-INF")).containsOnly(
"MANIFEST.MF",
"services"
);
assertThatFileList(root.resolve("META-INF")).containsOnly("MANIFEST.MF", "services");
}

@Test
public void testMetaInfServices() throws Exception {
assertThatFileList(root.resolve("META-INF").resolve("services"))
.allMatch(it -> it.startsWith("org.testcontainers."));
.allMatch(it -> it.startsWith("org.testcontainers."));
}

private ListAssert<String> assertThatFileList(Path path) throws IOException {
return (ListAssert) assertThat(Files.list(path))
.extracting(Path::getFileName)
.extracting(Path::toString)
.extracting(it -> it.endsWith("/") ? it.substring(0, it.length() - 1) : it);
.extracting(Path::getFileName)
.extracting(Path::toString)
.extracting(it -> it.endsWith("/") ? it.substring(0, it.length() - 1) : it);
}
}
Expand Up @@ -37,6 +37,7 @@
public class PublicBinaryAPITest extends AbstractJarFileTest {

private static String SHADED_PACKAGE = "org.testcontainers.shaded.";

private static String SHADED_PACKAGE_PATH = SHADED_PACKAGE.replaceAll("\\.", "/");

static {
Expand All @@ -49,36 +50,38 @@ public class PublicBinaryAPITest extends AbstractJarFileTest {
public static List<Object[]> data() throws Exception {
List<Object[]> result = new ArrayList<>();

Files.walkFileTree(root, new SimpleFileVisitor<Path>() {

@Override
public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
String fileName = path.toString();
Files.walkFileTree(
root,
new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
String fileName = path.toString();

if (!fileName.endsWith(".class")) {
return super.visitFile(path, attrs);
}
if (!fileName.endsWith(".class")) {
return super.visitFile(path, attrs);
}

if (!fileName.startsWith("/org/testcontainers/") ) {
return super.visitFile(path, attrs);
}
if (!fileName.startsWith("/org/testcontainers/")) {
return super.visitFile(path, attrs);
}

if (fileName.startsWith("/" + SHADED_PACKAGE_PATH)) {
return super.visitFile(path, attrs);
}
if (fileName.startsWith("/" + SHADED_PACKAGE_PATH)) {
return super.visitFile(path, attrs);
}

try(InputStream inputStream = Files.newInputStream(path)) {
ClassReader reader = new ClassReader(inputStream);
ClassNode node = new ClassNode();
reader.accept(node, ClassReader.SKIP_CODE);
if ((node.access & Opcodes.ACC_PUBLIC) != 0) {
result.add(new Object[]{ fileName, node });
try (InputStream inputStream = Files.newInputStream(path)) {
ClassReader reader = new ClassReader(inputStream);
ClassNode node = new ClassNode();
reader.accept(node, ClassReader.SKIP_CODE);
if ((node.access & Opcodes.ACC_PUBLIC) != 0) {
result.add(new Object[] { fileName, node });
}
}
}

return super.visitFile(path, attrs);
return super.visitFile(path, attrs);
}
}
});
);
return result;
}

Expand All @@ -100,39 +103,36 @@ public void setUp() {

@Test
public void testSuperClass() {
assertThat(classNode.superName)
.doesNotStartWith(SHADED_PACKAGE_PATH);
assertThat(classNode.superName).doesNotStartWith(SHADED_PACKAGE_PATH);
}

@Test
public void testInterfaces() {
assertThat(classNode.interfaces)
.allSatisfy(it -> assertThat(it).doesNotStartWith(SHADED_PACKAGE_PATH));
assertThat(classNode.interfaces).allSatisfy(it -> assertThat(it).doesNotStartWith(SHADED_PACKAGE_PATH));
}

@Test
public void testMethodReturnTypes() {
assertThat(classNode.methods)
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(it -> assertThat(Type.getReturnType(it.desc).getClassName()).doesNotStartWith(SHADED_PACKAGE));
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(it -> assertThat(Type.getReturnType(it.desc).getClassName()).doesNotStartWith(SHADED_PACKAGE));
}

@Test
public void testMethodArguments() {
assertThat(classNode.methods)
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(method -> assertThat(Arrays.asList(Type.getArgumentTypes(method.desc)))
.extracting(Type::getClassName)
.allSatisfy(it -> assertThat(it).doesNotStartWith(SHADED_PACKAGE))
);
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(method -> {
assertThat(Arrays.asList(Type.getArgumentTypes(method.desc)))
.extracting(Type::getClassName)
.allSatisfy(it -> assertThat(it).doesNotStartWith(SHADED_PACKAGE));
});
}

@Test
public void testFields() {
assertThat(classNode.fields)
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(it -> assertThat(Type.getType(it.desc).getClassName())
.doesNotStartWith(SHADED_PACKAGE)
);
.filteredOn(it -> (it.access & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED)) != 0)
.allSatisfy(it -> assertThat(Type.getType(it.desc).getClassName()).doesNotStartWith(SHADED_PACKAGE));
}
}