Copilot commented on code in PR #13870:
URL: https://github.com/apache/cloudstack/pull/13870#discussion_r3773703690
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -545,6 +545,80 @@ public void
getConsoleConnectionDetailsReturnsDetailsForKVMHypervisor() {
Assert.assertEquals(port, result.getPort());
}
+ @Test
+ public void
getConsoleConnectionDetailsRequiresNewViewerForKVMUserVmWhenMultipleViewersEnabled()
{
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ String hostAddress = "192.168.1.100";
+ int port = 5900;
+ Pair<String, Integer> hostPortInfo = new Pair<>(hostAddress, port);
+
+ Mockito.when(vm.getUuid()).thenReturn("vm-uuid");
+ Mockito.when(vm.getHostName()).thenReturn("vm-hostname");
+ Mockito.when(vm.getVncPassword()).thenReturn("vnc-password");
+ Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User);
+
Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+
Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(),
Mockito.anyList())).thenReturn(Map.of());
+ Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo);
+ Mockito.doReturn(new Ternary<>(hostAddress, null, null))
+ .when(consoleAccessManager).parseHostInfo(Mockito.anyString());
+
Mockito.doReturn(true).when(consoleAccessManager).isKvmMultipleConsoleViewersEnabled();
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetails(vm, host);
+
+ Assert.assertNotNull(result);
+ Assert.assertTrue(result.isSessionRequiresNewViewer());
+ }
+
+ @Test
+ public void
getConsoleConnectionDetailsDoesNotRequireNewViewerForKVMUserVmWhenMultipleViewersDisabled()
{
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ String hostAddress = "192.168.1.100";
+ int port = 5900;
+ Pair<String, Integer> hostPortInfo = new Pair<>(hostAddress, port);
+
+ Mockito.when(vm.getUuid()).thenReturn("vm-uuid");
+ Mockito.when(vm.getHostName()).thenReturn("vm-hostname");
+ Mockito.when(vm.getVncPassword()).thenReturn("vnc-password");
+ Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User);
+
Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+
Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(),
Mockito.anyList())).thenReturn(Map.of());
+ Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo);
+ Mockito.doReturn(new Ternary<>(hostAddress, null, null))
+ .when(consoleAccessManager).parseHostInfo(Mockito.anyString());
+
Mockito.doReturn(false).when(consoleAccessManager).isKvmMultipleConsoleViewersEnabled();
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetails(vm, host);
+
+ Assert.assertNotNull(result);
+ Assert.assertFalse(result.isSessionRequiresNewViewer());
+ }
+
+ @Test
+ public void
getConsoleConnectionDetailsDoesNotRequireNewViewerForKVMSystemVmWhenMultipleViewersEnabled()
{
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ String hostAddress = "192.168.1.100";
+ int port = 5900;
+ Pair<String, Integer> hostPortInfo = new Pair<>(hostAddress, port);
+
+ Mockito.when(vm.getUuid()).thenReturn("vm-uuid");
+ Mockito.when(vm.getHostName()).thenReturn("vm-hostname");
+ Mockito.when(vm.getVncPassword()).thenReturn("vnc-password");
+
Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
+
Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+
Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(),
Mockito.anyList())).thenReturn(Map.of());
+ Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo);
+ Mockito.doReturn(new Ternary<>(hostAddress, null, null))
+ .when(consoleAccessManager).parseHostInfo(Mockito.anyString());
Review Comment:
This test is named "...WhenMultipleViewersEnabled" but it never enables the
setting (it doesn’t stub `isKvmMultipleConsoleViewersEnabled()` to return
`true`). As written, it will pass even if the enabled-path is broken, so it
doesn’t actually validate the intended behavior for system VMs when the setting
is on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
