Skip to content

Commit

Permalink
[test] Replace PowerMockito#mockStatic usages with mockito-inline (#1…
Browse files Browse the repository at this point in the history 
…4098)

* introduce NarClassLoaderBuilder since Mockito Inline cannot mock classloader classes
* fix spyWithClassAndConstructorArgs(PulsarService.class, svcConfig) tests
  • Loading branch information
@nicoloboschi
nicoloboschi committed Feb 11, 2022
1 parent 58af7a5 commit ca64c67
Show file tree
Hide file tree
Showing 44 changed files with 1,645 additions and 1,915 deletions.
16 changes: 11 additions & 5 deletions managed-ledger/src/main/java/org/apache/bookkeeper/mledger/offload/OffloaderUtils.java
View file
Expand Up @@ -24,13 +24,13 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;
import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.mledger.LedgerOffloaderFactory;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.pulsar.common.nar.NarClassLoader;
import org.apache.pulsar.common.nar.NarClassLoaderBuilder;
import org.apache.pulsar.common.util.ObjectMapperFactory;

/**
Expand All @@ -54,8 +54,11 @@ static Pair<NarClassLoader, LedgerOffloaderFactory> getOffloaderFactory(String n
// need to load offloader NAR to the classloader that also loaded LedgerOffloaderFactory in case
// LedgerOffloaderFactory is loaded by a classloader that is not the default classloader
// as is the case for the pulsar presto plugin
NarClassLoader ncl = NarClassLoader.getFromArchive(new File(narPath), Collections.emptySet(),
LedgerOffloaderFactory.class.getClassLoader(), narExtractionDirectory);
NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(new File(narPath))
.parentClassLoader(LedgerOffloaderFactory.class.getClassLoader())
.extractionDirectory(narExtractionDirectory)
.build();
String configStr = ncl.getServiceDefinition(PULSAR_OFFLOADER_SERVICE_NAME);

OffloaderDefinition conf = ObjectMapperFactory.getThreadLocalYaml()
Expand Down Expand Up @@ -110,8 +113,11 @@ private static void rethrowIOException(Throwable cause)

public static OffloaderDefinition getOffloaderDefinition(String narPath, String narExtractionDirectory)
throws IOException {
try (NarClassLoader ncl = NarClassLoader.getFromArchive(new File(narPath), Collections.emptySet(),
narExtractionDirectory)) {
try (NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(new File(narPath))
.parentClassLoader(LedgerOffloaderFactory.class.getClassLoader())
.extractionDirectory(narExtractionDirectory)
.build()) {
String configStr = ncl.getServiceDefinition(PULSAR_OFFLOADER_SERVICE_NAME);

return ObjectMapperFactory.getThreadLocalYaml().readValue(configStr, OffloaderDefinition.class);
Expand Down
37 changes: 13 additions & 24 deletions managed-ledger/src/test/java/org/apache/bookkeeper/mledger/offload/OffloadersCacheTest.java
View file
Expand Up @@ -18,44 +18,33 @@
*/
package org.apache.bookkeeper.mledger.offload;

import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.testng.IObjectFactory;
import org.testng.annotations.ObjectFactory;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.testng.annotations.Test;

import static org.mockito.ArgumentMatchers.eq;
import static org.testng.Assert.assertSame;

@PrepareForTest({OffloaderUtils.class})
@PowerMockIgnore({"org.apache.logging.log4j.*", "org.apache.pulsar.common.nar.*"})
public class OffloadersCacheTest {

// Necessary to make PowerMockito.mockStatic work with TestNG.
@ObjectFactory
public IObjectFactory getObjectFactory() {
return new org.powermock.modules.testng.PowerMockObjectFactory();
}

@Test
public void testLoadsOnlyOnce() throws Exception {
Offloaders expectedOffloaders = new Offloaders();

PowerMockito.mockStatic(OffloaderUtils.class);
PowerMockito.when(OffloaderUtils.searchForOffloaders(eq("./offloaders"), eq("/tmp")))
.thenReturn(expectedOffloaders);
try (MockedStatic<OffloaderUtils> offloaderUtils = Mockito.mockStatic(OffloaderUtils.class)) {
offloaderUtils.when(() -> OffloaderUtils.searchForOffloaders(eq("./offloaders"), eq("/tmp")))
.thenReturn(expectedOffloaders);

OffloadersCache cache = new OffloadersCache();
OffloadersCache cache = new OffloadersCache();

// Call a first time to load the offloader
Offloaders offloaders1 = cache.getOrLoadOffloaders("./offloaders", "/tmp");
// Call a first time to load the offloader
Offloaders offloaders1 = cache.getOrLoadOffloaders("./offloaders", "/tmp");

assertSame(offloaders1, expectedOffloaders, "The offloaders should be the mocked one.");
assertSame(offloaders1, expectedOffloaders, "The offloaders should be the mocked one.");

// Call a second time to get the stored offlaoder
Offloaders offloaders2 = cache.getOrLoadOffloaders("./offloaders", "/tmp");
// Call a second time to get the stored offlaoder
Offloaders offloaders2 = cache.getOrLoadOffloaders("./offloaders", "/tmp");

assertSame(offloaders2, expectedOffloaders, "The offloaders should be the mocked one.");
assertSame(offloaders2, expectedOffloaders, "The offloaders should be the mocked one.");
}
}
}
12 changes: 12 additions & 0 deletions pom.xml
View file
Expand Up @@ -316,6 +316,12 @@ flexible messaging model and an intuitive client API.</description>
<version>${mockito.version}</version>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>${mockito.version}</version>
</dependency>

<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito2</artifactId>
Expand Down Expand Up @@ -1298,6 +1304,12 @@ flexible messaging model and an intuitive client API.</description>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito2</artifactId>
Expand Down
19 changes: 11 additions & 8 deletions ...mon/src/main/java/org/apache/pulsar/broker/web/plugin/servlet/AdditionalServletUtils.java
View file
Expand Up @@ -25,11 +25,11 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import lombok.experimental.UtilityClass;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.pulsar.common.nar.NarClassLoader;
import org.apache.pulsar.common.nar.NarClassLoaderBuilder;
import org.apache.pulsar.common.util.ObjectMapperFactory;

/**
Expand All @@ -50,9 +50,10 @@ public class AdditionalServletUtils {
*/
public AdditionalServletDefinition getAdditionalServletDefinition(
String narPath, String narExtractionDirectory) throws IOException {

try (NarClassLoader ncl = NarClassLoader.getFromArchive(
new File(narPath), Collections.emptySet(), narExtractionDirectory)) {
try (NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(new File(narPath))
.extractionDirectory(narExtractionDirectory)
.build();) {
return getAdditionalServletDefinition(ncl);
}
}
Expand Down Expand Up @@ -118,10 +119,12 @@ public AdditionalServletDefinitions searchForServlets(String additionalServletDi
public AdditionalServletWithClassLoader load(
AdditionalServletMetadata metadata, String narExtractionDirectory) throws IOException {

NarClassLoader ncl = NarClassLoader.getFromArchive(
metadata.getArchivePath().toAbsolutePath().toFile(),
Collections.emptySet(),
AdditionalServlet.class.getClassLoader(), narExtractionDirectory);
final File narFile = metadata.getArchivePath().toAbsolutePath().toFile();
NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(narFile)
.parentClassLoader(AdditionalServlet.class.getClassLoader())
.extractionDirectory(narExtractionDirectory)
.build();

AdditionalServletDefinition def = getAdditionalServletDefinition(ncl);
if (StringUtils.isBlank(def.getAdditionalServletClass())) {
Expand Down
71 changes: 25 additions & 46 deletions ...src/test/java/org/apache/pulsar/broker/web/plugin/servlet/AdditionalServletUtilsTest.java
View file
Expand Up @@ -18,38 +18,24 @@
*/
package org.apache.pulsar.broker.web.plugin.servlet;

import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.Set;
import org.apache.pulsar.common.nar.NarClassLoader;
import org.apache.pulsar.common.nar.NarClassLoaderBuilder;
import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.testng.IObjectFactory;
import org.testng.annotations.ObjectFactory;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.testng.annotations.Test;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.RETURNS_SELF;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.AssertJUnit.assertSame;
import static org.testng.AssertJUnit.assertTrue;

@PrepareForTest({
NarClassLoader.class
})
@PowerMockIgnore({"org.apache.logging.log4j.*"})
public class AdditionalServletUtilsTest {

// Necessary to make PowerMockito.mockStatic work with TestNG.
@ObjectFactory
public IObjectFactory getObjectFactory() {
return new org.powermock.modules.testng.PowerMockObjectFactory();
}

@Test
public void testLoadEventListener() throws Exception {
AdditionalServletDefinition def = new AdditionalServletDefinition();
Expand All @@ -69,19 +55,16 @@ public void testLoadEventListener() throws Exception {
when(mockLoader.loadClass(eq(MockAdditionalServlet.class.getName())))
.thenReturn(listenerClass);

PowerMockito.mockStatic(NarClassLoader.class);
PowerMockito.when(NarClassLoader.getFromArchive(
any(File.class),
any(Set.class),
any(ClassLoader.class),
any(String.class)
)).thenReturn(mockLoader);

AdditionalServletWithClassLoader returnedPhWithCL = AdditionalServletUtils.load(metadata, "");
AdditionalServlet returnedPh = returnedPhWithCL.getServlet();
final NarClassLoaderBuilder mockedBuilder = mock(NarClassLoaderBuilder.class, RETURNS_SELF);
when(mockedBuilder.build()).thenReturn(mockLoader);
try (MockedStatic<NarClassLoaderBuilder> builder = Mockito.mockStatic(NarClassLoaderBuilder.class)) {
builder.when(() -> NarClassLoaderBuilder.builder()).thenReturn(mockedBuilder);
AdditionalServletWithClassLoader returnedPhWithCL = AdditionalServletUtils.load(metadata, "");
AdditionalServlet returnedPh = returnedPhWithCL.getServlet();

assertSame(mockLoader, returnedPhWithCL.getClassLoader());
assertTrue(returnedPh instanceof MockAdditionalServlet);
assertSame(mockLoader, returnedPhWithCL.getClassLoader());
assertTrue(returnedPh instanceof MockAdditionalServlet);
}
}

@Test(expectedExceptions = IOException.class)
Expand All @@ -102,15 +85,13 @@ public void testLoadEventListenerWithBlankListerClass() throws Exception {
when(mockLoader.loadClass(eq(MockAdditionalServlet.class.getName())))
.thenReturn(listenerClass);

PowerMockito.mockStatic(NarClassLoader.class);
PowerMockito.when(NarClassLoader.getFromArchive(
any(File.class),
any(Set.class),
any(ClassLoader.class),
any(String.class)
)).thenReturn(mockLoader);
final NarClassLoaderBuilder mockedBuilder = mock(NarClassLoaderBuilder.class, RETURNS_SELF);
when(mockedBuilder.build()).thenReturn(mockLoader);
try (MockedStatic<NarClassLoaderBuilder> builder = Mockito.mockStatic(NarClassLoaderBuilder.class)) {
builder.when(() -> NarClassLoaderBuilder.builder()).thenReturn(mockedBuilder);

AdditionalServletUtils.load(metadata, "");
AdditionalServletUtils.load(metadata, "");
}
}

@Test(expectedExceptions = IOException.class)
Expand All @@ -132,14 +113,12 @@ public void testLoadEventListenerWithWrongListerClass() throws Exception {
when(mockLoader.loadClass(eq(Runnable.class.getName())))
.thenReturn(listenerClass);

PowerMockito.mockStatic(NarClassLoader.class);
PowerMockito.when(NarClassLoader.getFromArchive(
any(File.class),
any(Set.class),
any(ClassLoader.class),
any(String.class)
)).thenReturn(mockLoader);
final NarClassLoaderBuilder mockedBuilder = mock(NarClassLoaderBuilder.class, RETURNS_SELF);
when(mockedBuilder.build()).thenReturn(mockLoader);
try (MockedStatic<NarClassLoaderBuilder> builder = Mockito.mockStatic(NarClassLoaderBuilder.class)) {
builder.when(() -> NarClassLoaderBuilder.builder()).thenReturn(mockedBuilder);

AdditionalServletUtils.load(metadata, "");
AdditionalServletUtils.load(metadata, "");
}
}
}
18 changes: 11 additions & 7 deletions pulsar-broker/src/main/java/org/apache/pulsar/broker/intercept/BrokerInterceptorUtils.java
View file
Expand Up @@ -25,11 +25,11 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import lombok.experimental.UtilityClass;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.pulsar.common.nar.NarClassLoader;
import org.apache.pulsar.common.nar.NarClassLoaderBuilder;
import org.apache.pulsar.common.util.ObjectMapperFactory;

/**
Expand All @@ -50,8 +50,10 @@ public class BrokerInterceptorUtils {
*/
public BrokerInterceptorDefinition getBrokerInterceptorDefinition(String narPath, String narExtractionDirectory)
throws IOException {
try (NarClassLoader ncl = NarClassLoader.getFromArchive(new File(narPath), Collections.emptySet(),
narExtractionDirectory)) {
try (NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(new File(narPath))
.extractionDirectory(narExtractionDirectory)
.build()) {
return getBrokerInterceptorDefinition(ncl);
}
}
Expand Down Expand Up @@ -117,10 +119,12 @@ public BrokerInterceptorDefinitions searchForInterceptors(String interceptorsDir
*/
BrokerInterceptorWithClassLoader load(BrokerInterceptorMetadata metadata, String narExtractionDirectory)
throws IOException {
NarClassLoader ncl = NarClassLoader.getFromArchive(
metadata.getArchivePath().toAbsolutePath().toFile(),
Collections.emptySet(),
BrokerInterceptor.class.getClassLoader(), narExtractionDirectory);
final File narFile = metadata.getArchivePath().toAbsolutePath().toFile();
NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(narFile)
.parentClassLoader(BrokerInterceptorUtils.class.getClassLoader())
.extractionDirectory(narExtractionDirectory)
.build();

BrokerInterceptorDefinition def = getBrokerInterceptorDefinition(ncl);
if (StringUtils.isBlank(def.getInterceptorClass())) {
Expand Down
18 changes: 11 additions & 7 deletions pulsar-broker/src/main/java/org/apache/pulsar/broker/protocol/ProtocolHandlerUtils.java
View file
Expand Up @@ -25,11 +25,11 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import lombok.experimental.UtilityClass;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.pulsar.common.nar.NarClassLoader;
import org.apache.pulsar.common.nar.NarClassLoaderBuilder;
import org.apache.pulsar.common.util.ObjectMapperFactory;

/**
Expand All @@ -50,8 +50,10 @@ class ProtocolHandlerUtils {
*/
public static ProtocolHandlerDefinition getProtocolHandlerDefinition(String narPath, String narExtractionDirectory)
throws IOException {
try (NarClassLoader ncl = NarClassLoader.getFromArchive(new File(narPath), Collections.emptySet(),
narExtractionDirectory)) {
try (NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(new File(narPath))
.extractionDirectory(narExtractionDirectory)
.build()) {
return getProtocolHandlerDefinition(ncl);
}
}
Expand Down Expand Up @@ -117,10 +119,12 @@ public static ProtocolHandlerDefinitions searchForHandlers(String handlersDirect
*/
static ProtocolHandlerWithClassLoader load(ProtocolHandlerMetadata metadata,
String narExtractionDirectory) throws IOException {
NarClassLoader ncl = NarClassLoader.getFromArchive(
metadata.getArchivePath().toAbsolutePath().toFile(),
Collections.emptySet(),
ProtocolHandler.class.getClassLoader(), narExtractionDirectory);
final File narFile = metadata.getArchivePath().toAbsolutePath().toFile();
NarClassLoader ncl = NarClassLoaderBuilder.builder()
.narFile(narFile)
.parentClassLoader(ProtocolHandler.class.getClassLoader())
.extractionDirectory(narExtractionDirectory)
.build();

ProtocolHandlerDefinition phDef = getProtocolHandlerDefinition(ncl);
if (StringUtils.isBlank(phDef.getHandlerClass())) {
Expand Down

0 comments on commit ca64c67

Please sign in to comment.