Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class PurgeExpungedResourcesCmdTest {

@Spy
@InjectMocks
PurgeExpungedResourcesCmd spyCmd = Mockito.spy(new PurgeExpungedResourcesCmd());
PurgeExpungedResourcesCmd spyCmd;

@Test
public void testGetResourceType() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@
import com.cloud.utils.Pair;
import org.apache.cloudstack.api.response.ExtractResponse;
import org.apache.cloudstack.storage.browser.StorageBrowser;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.test.util.ReflectionTestUtils;
Expand All @@ -48,18 +45,6 @@ public class DownloadImageStoreObjectCmdTest {
@Spy
private DownloadImageStoreObjectCmd cmd;

private AutoCloseable closeable;

@Before
public void setUp() {
closeable = MockitoAnnotations.openMocks(this);
}

@After
public void tearDown() throws Exception {
closeable.close();
}

@Test
public void testExecute() throws Exception {
ReflectionTestUtils.setField(cmd, "storeId", 1L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ public class MigrateVirtualMachineWithVolumeCmdTest {
@Mock
Host hostMock;

@Mock
private Object job;

@Mock
private Object _responseObject;

@Spy
@InjectMocks
MigrateVirtualMachineWithVolumeCmd cmdSpy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.cloud.network.vpc.VpcService;
import com.cloud.user.AccountService;
import com.cloud.utils.db.EntityManager;
import junit.framework.TestCase;
import org.apache.cloudstack.api.ResponseGenerator;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -34,7 +33,7 @@
import java.util.List;

@RunWith(MockitoJUnitRunner.class)
public class CreateVPCCmdByAdminTest extends TestCase {
public class CreateVPCCmdByAdminTest {

@Mock
public VpcService _vpcService;
Expand All @@ -43,8 +42,10 @@ public class CreateVPCCmdByAdminTest extends TestCase {
@Mock
public AccountService _accountService;
private ResponseGenerator responseGenerator;
@Mock
public Object job;
@InjectMocks
CreateVPCCmdByAdmin cmd = new CreateVPCCmdByAdmin();
CreateVPCCmdByAdmin cmd;

@Test
public void testBgpPeerIds() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ public class UpdateNetworkCmdTest {
NetworkService networkService;
@Mock
public EntityManager _entityMgr;
private ResponseGenerator responseGenerator;
@Mock
private ResponseGenerator _responseGenerator;
@Mock
private Object job;
@InjectMocks
UpdateNetworkCmd cmd = new UpdateNetworkCmd();

Expand Down Expand Up @@ -176,15 +179,13 @@ public void testExecute() throws InsufficientCapacityException {
ReflectionTestUtils.setField(cmd, "id", networkId);
ReflectionTestUtils.setField(cmd, "publicMtu", publicmtu);
Network network = Mockito.mock(Network.class);
responseGenerator = Mockito.mock(ResponseGenerator.class);
NetworkResponse response = Mockito.mock(NetworkResponse.class);
response.setPublicMtu(publicmtu);
Mockito.when(networkService.getNetwork(networkId)).thenReturn(network);
Mockito.when(networkService.updateGuestNetwork(cmd)).thenReturn(network);
cmd._responseGenerator = responseGenerator;
Mockito.when(responseGenerator.createNetworkResponse(ResponseObject.ResponseView.Restricted, network)).thenReturn(response);
Mockito.when(_responseGenerator.createNetworkResponse(ResponseObject.ResponseView.Restricted, network)).thenReturn(response);
cmd.execute();
Mockito.verify(responseGenerator).createNetworkResponse(Mockito.any(ResponseObject.ResponseView.class), Mockito.any(Network.class));
Mockito.verify(_responseGenerator).createNetworkResponse(Mockito.any(ResponseObject.ResponseView.class), Mockito.any(Network.class));
NetworkResponse actualResponse = (NetworkResponse) cmd.getResponseObject();

Assert.assertEquals(response, actualResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.cloud.network.vpc.VpcService;
import com.cloud.user.AccountService;
import com.cloud.utils.db.EntityManager;
import junit.framework.TestCase;
import org.apache.cloudstack.api.ResponseGenerator;
import org.apache.cloudstack.api.ResponseObject;
import org.apache.cloudstack.api.response.VpcResponse;
Expand All @@ -39,17 +38,21 @@
import org.springframework.test.util.ReflectionTestUtils;

@RunWith(MockitoJUnitRunner.class)
public class CreateVPCCmdTest extends TestCase {
public class CreateVPCCmdTest {

@Mock
public VpcService _vpcService;
@Mock
public EntityManager _entityMgr;
@Mock
public AccountService _accountService;
private ResponseGenerator responseGenerator;
@Mock
private ResponseGenerator _responseGenerator;
@Mock
private Object job;

@InjectMocks
CreateVPCCmd cmd = new CreateVPCCmd();
CreateVPCCmd cmd;

@Test
public void testGetAccountName() {
Expand Down Expand Up @@ -185,11 +188,9 @@ public void testExecute() throws ResourceUnavailableException, InsufficientCapac
VpcResponse response = Mockito.mock(VpcResponse.class);

ReflectionTestUtils.setField(cmd, "id", 1L);
responseGenerator = Mockito.mock(ResponseGenerator.class);
Mockito.doNothing().when(_vpcService).startVpc(cmd);
Mockito.when(_entityMgr.findById(Mockito.eq(Vpc.class), Mockito.any(Long.class))).thenReturn(vpc);
cmd._responseGenerator = responseGenerator;
Mockito.when(responseGenerator.createVpcResponse(ResponseObject.ResponseView.Restricted, vpc)).thenReturn(response);
Mockito.when(_responseGenerator.createVpcResponse(ResponseObject.ResponseView.Restricted, vpc)).thenReturn(response);
cmd.execute();
Mockito.verify(_vpcService, Mockito.times(1)).startVpc(cmd);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ private void configureAndVerifyTestSearchDiskDefOnMigrateDiskInfoList(String ser

@Test
public void deleteOrDisconnectDisksOnSourcePoolTest() {
LibvirtMigrateCommandWrapper spyLibvirtMigrateCmdWrapper = Mockito.spy(libvirtMigrateCmdWrapper);
LibvirtMigrateCommandWrapper spyLibvirtMigrateCmdWrapper = libvirtMigrateCmdWrapper;
Mockito.doNothing().when(spyLibvirtMigrateCmdWrapper).deleteLocalVolume("volPath");
Comment on lines +707 to 708
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The redundant spy wrapping is unnecessary since libvirtMigrateCmdWrapper is already a spy. Removing this redundant wrapping can simplify the test code.

Suggested change
LibvirtMigrateCommandWrapper spyLibvirtMigrateCmdWrapper = libvirtMigrateCmdWrapper;
Mockito.doNothing().when(spyLibvirtMigrateCmdWrapper).deleteLocalVolume("volPath");
Mockito.doNothing().when(libvirtMigrateCmdWrapper).deleteLocalVolume("volPath");

Copilot uses AI. Check for mistakes.

List<MigrateDiskInfo> migrateDiskInfoList = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;

import com.cloud.agent.api.to.DataStoreTO;
Expand All @@ -42,7 +43,8 @@
@RunWith(MockitoJUnitRunner.class)
public class LibvirtRevertSnapshotCommandWrapperTest {

LibvirtRevertSnapshotCommandWrapper libvirtRevertSnapshotCommandWrapperSpy = Mockito.spy(LibvirtRevertSnapshotCommandWrapper.class);
@Spy
LibvirtRevertSnapshotCommandWrapper libvirtRevertSnapshotCommandWrapperSpy;

@Mock
KVMStoragePool kvmStoragePoolPrimaryMock;
Expand Down Expand Up @@ -107,7 +109,7 @@ public void validateGetSnapshotPathExistsOnPrimaryStorage() {
Mockito.doReturn(snapshotPath).when(snapshotObjectToPrimaryMock).getPath();

try (MockedStatic<Files> ignored = Mockito.mockStatic(Files.class)) {
Mockito.when(Files.exists(Mockito.any(Path.class), Mockito.any())).thenReturn(true);
Mockito.when(Files.exists(Mockito.any(Path.class))).thenReturn(true);

Pair<String, SnapshotObjectTO> result = libvirtRevertSnapshotCommandWrapperSpy.getSnapshot(
snapshotObjectToPrimaryMock, snapshotObjectToSecondaryMock,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public class VmwareManagerImplTest {
private Map<String, String> clusterDetails;
@Mock
private Map<String, String> hostDetails;
@Mock
private Map<String, Object> _configParams;

@Before
public void beforeTest() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import java.util.List;
import java.util.Map;

import com.cloud.agent.api.Command;
import com.cloud.agent.api.ScaleVmAnswer;
import com.cloud.hypervisor.vmware.mo.DatastoreMO;
import com.cloud.hypervisor.vmware.mo.HostDatastoreBrowserMO;
import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
Expand All @@ -52,7 +54,6 @@
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsAnswer;
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsCommand;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -62,17 +63,14 @@
import org.mockito.MockedConstruction;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;

import com.cloud.agent.api.Answer;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.CheckGuestOsMappingAnswer;
import com.cloud.agent.api.CheckGuestOsMappingCommand;
import com.cloud.agent.api.GetHypervisorGuestOsNamesAnswer;
import com.cloud.agent.api.GetHypervisorGuestOsNamesCommand;
import com.cloud.agent.api.ScaleVmAnswer;
import com.cloud.agent.api.ScaleVmCommand;
import com.cloud.agent.api.routing.GetAutoScaleMetricsAnswer;
import com.cloud.agent.api.routing.GetAutoScaleMetricsCommand;
Expand Down Expand Up @@ -185,6 +183,8 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {
VimPortType vimService;
@Mock
HostCapability hostCapability;
@Mock
ManagedObjectReference _morHyperHost;

CopyCommand storageCmd;
EnumMap<VmwareStorageProcessorConfigurableFields, Object> params = new EnumMap<VmwareStorageProcessorConfigurableFields,Object>(VmwareStorageProcessorConfigurableFields.class);
Expand All @@ -201,11 +201,9 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {

private Map<String,String> specsArray = new HashMap<String,String>();

AutoCloseable closeable;

@Before
public void setup() throws Exception {
closeable = MockitoAnnotations.openMocks(this);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The class is annotated with @RunWith(MockitoJUnitRunner.class) which is calling MockitoAnnotations.openMocks(...); before every test. Calling openMocks a second time can have undesirable side effects.

I removed the line as it was unnecessary and could issues in some circumstances.

storageCmd = mock(CopyCommand.class);
doReturn(context).when(_resource).getServiceContext(null);
when(cmd.getVirtualMachine()).thenReturn(vmSpec);
Expand Down Expand Up @@ -234,11 +232,6 @@ public void setup() throws Exception {
when(hostCapability.isNestedHVSupported()).thenReturn(true);
}

@After
public void tearDown() throws Exception {
closeable.close();
}

//Test successful scaling up the vm
@Test
public void testScaleVMF1() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void prepareForShutdownCmdOtherMsHostsInPreparingState() {
ManagementServerHostVO msHost2 = mock(ManagementServerHostVO.class);
List<ManagementServerHostVO> msHostList = new ArrayList<>();
msHostList.add(msHost2);
Mockito.when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.lenient().when(msHostDao.listBy(any())).thenReturn(msHostList);
Copy link
Member

@vishesh92 vishesh92 Apr 10, 2025

Choose a reason for hiding this comment

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

iirc mockito was throwing errors earlier when an unnecessary mocks was setup. Why is lenient() required now? Can't we just remove the mock if it's not required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be removed. I choose not to remove it because I did not want to significantly alter any logic of the tests as part of the Mockito version upgrade.

It is unclear to me if the mock is not being used by accident and it should be used. I feel the discussion about the test intention should happen independently of the Mockito upgrade.

Mockito.when(msHostDao.findById(1L)).thenReturn(msHost1);
PrepareForShutdownCmd cmd = mock(PrepareForShutdownCmd.class);
Mockito.when(cmd.getManagementServerId()).thenReturn(1L);
Expand All @@ -169,7 +169,7 @@ public void prepareForShutdownCmdNullResponseFromClusterManager() {
ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Up);
List<ManagementServerHostVO> msHostList = new ArrayList<>();
Mockito.when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.lenient().when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.when(msHostDao.findById(1L)).thenReturn(msHost);
PrepareForShutdownCmd cmd = mock(PrepareForShutdownCmd.class);
Mockito.when(cmd.getManagementServerId()).thenReturn(1L);
Expand All @@ -185,7 +185,7 @@ public void prepareForShutdownCmdFailedResponseFromClusterManager() {
ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Up);
List<ManagementServerHostVO> msHostList = new ArrayList<>();
Mockito.when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.lenient().when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.when(msHostDao.findById(1L)).thenReturn(msHost);
PrepareForShutdownCmd cmd = mock(PrepareForShutdownCmd.class);
Mockito.when(cmd.getManagementServerId()).thenReturn(1L);
Expand All @@ -200,7 +200,7 @@ public void prepareForShutdownCmdFailedResponseFromClusterManager() {
public void prepareForShutdownCmdSuccessResponseFromClusterManager() {
ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Up);
Mockito.when(msHostDao.listBy(any())).thenReturn(new ArrayList<>());
Mockito.lenient().when(msHostDao.listBy(any())).thenReturn(new ArrayList<>());
Mockito.when(msHostDao.findById(1L)).thenReturn(msHost);
Mockito.when(hostDao.listByMs(anyLong())).thenReturn(new ArrayList<>());
PrepareForShutdownCmd cmd = mock(PrepareForShutdownCmd.class);
Expand Down Expand Up @@ -279,7 +279,7 @@ public void triggerShutdownCmdMsInUpStateAndOtherMsHostsInPreparingState() {
ManagementServerHostVO msHost2 = mock(ManagementServerHostVO.class);
List<ManagementServerHostVO> msHostList = new ArrayList<>();
msHostList.add(msHost2);
Mockito.when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.lenient().when(msHostDao.listBy(any())).thenReturn(msHostList);
Mockito.when(msHostDao.findById(1L)).thenReturn(msHost1);
TriggerShutdownCmd cmd = mock(TriggerShutdownCmd.class);
Mockito.when(cmd.getManagementServerId()).thenReturn(1L);
Expand Down
2 changes: 1 addition & 1 deletion plugins/storage/volume/storpool/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<artifactId>mockito-core</artifactId>
<version>${cs.mockito.version}</version>
</dependency>
<dependency>
Expand Down
4 changes: 2 additions & 2 deletions plugins/user-authenticators/ldap/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<artifactId>mockito-core</artifactId>
<version>${cs.mockito.version}</version>
<scope>compile</scope>
<exclusions>
Expand All @@ -147,7 +147,7 @@
<dependency>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
<version>1.14.5</version>
<version>1.15.11</version>
</dependency>
<dependency>
<groupId>junit</groupId>
Expand Down
6 changes: 6 additions & 0 deletions plugins/user-authenticators/saml2/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@
<artifactId>assertj-core</artifactId>
<version>${cs.assertj.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
@Mock
HttpServletRequest req;

@Mock
Object _responseObject;

@Spy
@InjectMocks
private SAML2LoginAPIAuthenticatorCmd cmdSpy;
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
<cs.junit.dataprovider.version>1.13.1</cs.junit.dataprovider.version>
<cs.junit.jupiter.version>5.9.1</cs.junit.jupiter.version>
<cs.guava-testlib.version>18.0</cs.guava-testlib.version>
<cs.mockito.version>4.11.0</cs.mockito.version>
<cs.mockito.version>5.16.1</cs.mockito.version>
<cs.selenium.server.version>1.0-20081010.060147</cs.selenium.server.version>
<cs.selenium-java-client-driver.version>1.0.1</cs.selenium-java-client-driver.version>
<cs.testng.version>7.1.0</cs.testng.version>
Expand Down Expand Up @@ -750,7 +750,7 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<artifactId>mockito-core</artifactId>
<version>${cs.mockito.version}</version>
<scope>test</scope>
<exclusions>
Expand Down
Loading