Skip to content

Commit a3371d2

Browse files
swillswill
committed
Merge pull request #1262 from rafaelweingartner/lrg-cs-hackday-015
Removed unnecessary code from getGuestOsType in CitrixResourceBaseConsidering that all mapping of Guest OS Names to their respective hypervisor compatible types is made thorugh accessing a database, we've decided to remove a bit of code in the XcpOssResource class which was doing that same thing for 2 different OS's (both of which ARE in the database). That has led us to a bunch of unused parameters in the getGuestOsType method from its superclass, which we've also decided to remove. Test cases were added for four different possibilities for the platformEmulator String: one for a null String, one for a blank String, one for an empy String and one for a random case with a valid String. * pr/1262: Remove test cases duplicated code. Removed unnecessary code from getGuestOsType in CitrixResourceBase Signed-off-by: Will Stevens <williamstevens@gmail.com>
2 parents a957821 + 8355b58 commit a3371d2

File tree

11 files changed

+132
-62
lines changed

11 files changed

+132
-62
lines changed

‎plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import java.util.UUID;
4545
import java.util.concurrent.TimeoutException;
4646

47-
import javax.ejb.Local;
4847
import javax.naming.ConfigurationException;
4948
import javax.xml.parsers.DocumentBuilderFactory;
5049
import javax.xml.parsers.ParserConfigurationException;
@@ -164,7 +163,6 @@
164163
* before you do any changes in this code here.
165164
*
166165
*/
167-
@Local(value = ServerResource.class)
168166
public abstract class CitrixResourceBase implements ServerResource, HypervisorResource, VirtualRouterDeployer {
169167

170168
public enum SRType {
@@ -1232,7 +1230,7 @@ public VIF createVif(final Connection conn, final String vmName, final VM vm, fi
12321230
}
12331231

12341232
public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmSpec, final Host host) throws XenAPIException, XmlRpcException {
1235-
final String guestOsTypeName = getGuestOsType(vmSpec.getOs(), vmSpec.getPlatformEmulator(), vmSpec.getBootloader() == BootloaderType.CD);
1233+
final String guestOsTypeName = getGuestOsType(vmSpec.getPlatformEmulator());
12361234
final Set<VM> templates = VM.getByNameLabel(conn, guestOsTypeName);
12371235
if (templates == null || templates.isEmpty()) {
12381236
throw new CloudRuntimeException("Cannot find template " + guestOsTypeName + " on XenServer host");
@@ -1337,7 +1335,7 @@ public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmS
13371335
final TemplateObjectTO iso = (TemplateObjectTO) disk.getData();
13381336
final String osType = iso.getGuestOsType();
13391337
if (osType != null) {
1340-
final String isoGuestOsName = getGuestOsType(osType, vmSpec.getPlatformEmulator(), vmSpec.getBootloader() == BootloaderType.CD);
1338+
final String isoGuestOsName = getGuestOsType(vmSpec.getPlatformEmulator());
13411339
if (!isoGuestOsName.equals(guestOsTypeName)) {
13421340
vmSpec.setBootloader(BootloaderType.PyGrub);
13431341
}
@@ -2019,8 +2017,8 @@ public HashMap<String, HashMap<String, VgpuTypesInfo>> getGPUGroupDetails(final
20192017
return null;
20202018
}
20212019

2022-
protected String getGuestOsType(final String stdType, String platformEmulator, final boolean bootFromCD) {
2023-
if (platformEmulator == null) {
2020+
protected String getGuestOsType(String platformEmulator) {
2021+
if (org.apache.commons.lang.StringUtils.isBlank(platformEmulator)) {
20242022
s_logger.debug("no guest OS type, start it as HVM guest");
20252023
platformEmulator = "Other install media";
20262024
}

‎plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpOssResource.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,12 @@
1717

1818
package com.cloud.hypervisor.xenserver.resource;
1919

20-
import javax.ejb.Local;
21-
2220
import org.apache.xmlrpc.XmlRpcException;
2321

24-
import com.cloud.resource.ServerResource;
2522
import com.xensource.xenapi.Connection;
2623
import com.xensource.xenapi.Types.XenAPIException;
2724
import com.xensource.xenapi.VM;
2825

29-
@Local(value = ServerResource.class)
3026
public class XcpOssResource extends CitrixResourceBase {
3127

3228
private static final long mem_32m = 33554432L;
@@ -36,18 +32,6 @@ protected String getPatchFilePath() {
3632
return "scripts/vm/hypervisor/xenserver/xcposs/patch";
3733
}
3834

39-
@Override
40-
protected String getGuestOsType(final String stdType,
41-
final String platformEmulator, final boolean bootFromCD) {
42-
if (stdType.equalsIgnoreCase("Debian GNU/Linux 6(64-bit)")) {
43-
return "Debian Squeeze 6.0 (64-bit)";
44-
} else if (stdType.equalsIgnoreCase("CentOS 5.6 (64-bit)")) {
45-
return "CentOS 5 (64-bit)";
46-
} else {
47-
return super.getGuestOsType(stdType, platformEmulator, bootFromCD);
48-
}
49-
}
50-
5135
@Override
5236
protected void setMemory(final Connection conn, final VM vm, long minMemsize, final long maxMemsize) throws XmlRpcException, XenAPIException {
5337
vm.setMemoryLimits(conn, mem_32m, maxMemsize, minMemsize, maxMemsize);

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,22 @@
1919
import java.util.List;
2020

2121
import org.junit.Assert;
22+
import org.junit.Test;
2223
import org.mockito.Mockito;
2324
import org.powermock.api.mockito.PowerMockito;
2425

2526
import com.cloud.utils.script.Script;
2627

27-
public abstract class CitrixResourceBaseTest {
28+
public class CitrixResourceBaseTest {
2829

29-
public void testGetPathFilesExeption(CitrixResourceBase citrixResourceBase) {
30+
protected CitrixResourceBase citrixResourceBase = new CitrixResourceBase() {
31+
@Override
32+
protected String getPatchFilePath() {
33+
return null;
34+
}
35+
};
36+
37+
public void testGetPathFilesExeption() {
3038
String patch = citrixResourceBase.getPatchFilePath();
3139

3240
PowerMockito.mockStatic(Script.class);
@@ -36,7 +44,7 @@ public void testGetPathFilesExeption(CitrixResourceBase citrixResourceBase) {
3644

3745
}
3846

39-
public void testGetPathFilesListReturned(CitrixResourceBase citrixResourceBase) {
47+
public void testGetPathFilesListReturned() {
4048
String patch = citrixResourceBase.getPatchFilePath();
4149

4250
PowerMockito.mockStatic(Script.class);
@@ -49,4 +57,39 @@ public void testGetPathFilesListReturned(CitrixResourceBase citrixResourceBase)
4957
String receivedPath = files.get(0).getAbsolutePath();
5058
Assert.assertEquals(receivedPath, pathExpected);
5159
}
60+
61+
@Test
62+
public void testGetGuestOsTypeNull() {
63+
String platformEmulator = null;
64+
65+
String expected = "Other install media";
66+
String guestOsType = citrixResourceBase.getGuestOsType(platformEmulator);
67+
Assert.assertEquals(expected, guestOsType);
68+
}
69+
70+
@Test
71+
public void testGetGuestOsTypeEmpty() {
72+
String platformEmulator = "";
73+
74+
String expected = "Other install media";
75+
String guestOsType = citrixResourceBase.getGuestOsType(platformEmulator);
76+
Assert.assertEquals(expected, guestOsType);
77+
}
78+
79+
@Test
80+
public void testGetGuestOsTypeBlank() {
81+
String platformEmulator = " ";
82+
83+
String expected = "Other install media";
84+
String guestOsType = citrixResourceBase.getGuestOsType(platformEmulator);
85+
Assert.assertEquals(expected, guestOsType);
86+
}
87+
88+
@Test
89+
public void testGetGuestOsTypeOther() {
90+
String platformEmulator = "My Own Linux Distribution Y.M (64-bit)";
91+
92+
String guestOsType = citrixResourceBase.getGuestOsType(platformEmulator);
93+
Assert.assertEquals(platformEmulator, guestOsType);
94+
}
5295
}

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpOssResourceTest.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.cloud.hypervisor.xenserver.resource;
1717

1818
import org.junit.Assert;
19+
import org.junit.Before;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -26,11 +27,14 @@
2627
@RunWith(PowerMockRunner.class)
2728
public class XcpOssResourceTest extends CitrixResourceBaseTest{
2829

29-
private XcpOssResource xcpOssResource = new XcpOssResource();
30+
@Before
31+
public void beforeTest() {
32+
super.citrixResourceBase = new XcpOssResource();
33+
}
3034

3135
@Test
3236
public void testPatchFilePath() {
33-
String patchFilePath = xcpOssResource.getPatchFilePath();
37+
String patchFilePath = citrixResourceBase.getPatchFilePath();
3438
String patch = "scripts/vm/hypervisor/xenserver/xcposs/patch";
3539

3640
Assert.assertEquals(patch, patchFilePath);
@@ -39,12 +43,12 @@ public void testPatchFilePath() {
3943
@Test(expected = CloudRuntimeException.class)
4044
@PrepareForTest(Script.class )
4145
public void testGetFiles(){
42-
testGetPathFilesExeption(xcpOssResource);
46+
testGetPathFilesExeption();
4347
}
4448

4549
@Test
4650
@PrepareForTest(Script.class )
4751
public void testGetFilesListReturned(){
48-
testGetPathFilesListReturned(xcpOssResource);
52+
testGetPathFilesListReturned();
4953
}
5054
}

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpServerResourceTest.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.cloud.hypervisor.xenserver.resource;
1717

1818
import org.junit.Assert;
19+
import org.junit.Before;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -27,11 +28,14 @@
2728
@RunWith(PowerMockRunner.class)
2829
public class XcpServerResourceTest extends CitrixResourceBaseTest{
2930

30-
private XcpServerResource xcpServerResource = new XcpServerResource();
31+
@Before
32+
public void beforeTest() {
33+
super.citrixResourceBase = new XcpServerResource();
34+
}
3135

3236
@Test
3337
public void testPatchFilePath() {
34-
String patchFilePath = xcpServerResource.getPatchFilePath();
38+
String patchFilePath = citrixResourceBase.getPatchFilePath();
3539
String patch = "scripts/vm/hypervisor/xenserver/xcpserver/patch";
3640

3741
Assert.assertEquals(patch, patchFilePath);
@@ -40,12 +44,12 @@ public void testPatchFilePath() {
4044
@Test(expected = CloudRuntimeException.class)
4145
@PrepareForTest(Script.class )
4246
public void testGetFilesExeption(){
43-
testGetPathFilesExeption(xcpServerResource);
47+
testGetPathFilesExeption();
4448
}
4549

4650
@Test
4751
@PrepareForTest(Script.class )
4852
public void testGetFilesListReturned(){
49-
testGetPathFilesListReturned(xcpServerResource);
53+
testGetPathFilesListReturned();
5054
}
5155
}

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56FP1ResourceTest.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,40 @@
1616
package com.cloud.hypervisor.xenserver.resource;
1717

1818
import org.junit.Assert;
19+
import org.junit.Before;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.powermock.core.classloader.annotations.PrepareForTest;
2223
import org.powermock.modules.junit4.PowerMockRunner;
2324

2425
import com.cloud.utils.exception.CloudRuntimeException;
2526
import com.cloud.utils.script.Script;
27+
2628
@RunWith(PowerMockRunner.class)
2729
public class XenServer56FP1ResourceTest extends CitrixResourceBaseTest{
2830

29-
private XenServer56FP1Resource xenServer56FP1Resource = new XenServer56FP1Resource();
31+
@Before
32+
public void beforeTest() {
33+
super.citrixResourceBase = new XenServer56FP1Resource();
34+
}
3035

3136
@Test
3237
public void testPatchFilePath() {
33-
String patchFilePath = xenServer56FP1Resource.getPatchFilePath();
38+
String patchFilePath = citrixResourceBase.getPatchFilePath();
3439
String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
3540

3641
Assert.assertEquals(patch, patchFilePath);
3742
}
43+
3844
@Test(expected = CloudRuntimeException.class)
3945
@PrepareForTest(Script.class )
4046
public void testGetFiles(){
41-
testGetPathFilesExeption(xenServer56FP1Resource);
47+
testGetPathFilesExeption();
4248
}
49+
4350
@Test
4451
@PrepareForTest(Script.class )
4552
public void testGetFilesListReturned(){
46-
testGetPathFilesListReturned(xenServer56FP1Resource);
53+
testGetPathFilesListReturned();
4754
}
4855
}

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56ResourceTest.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.cloud.hypervisor.xenserver.resource;
1717

1818
import org.junit.Assert;
19+
import org.junit.Before;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -26,11 +27,14 @@
2627
@RunWith(PowerMockRunner.class)
2728
public class XenServer56ResourceTest extends CitrixResourceBaseTest {
2829

29-
private XenServer56Resource xenServer56Resource = new XenServer56Resource();
30+
@Before
31+
public void beforeTest() {
32+
super.citrixResourceBase = new XenServer56Resource();
33+
}
3034

3135
@Test
3236
public void testPatchFilePath() {
33-
String patchFilePath = xenServer56Resource.getPatchFilePath();
37+
String patchFilePath = citrixResourceBase.getPatchFilePath();
3438
String patch = "scripts/vm/hypervisor/xenserver/xenserver56/patch";
3539

3640
Assert.assertEquals(patch, patchFilePath);
@@ -39,11 +43,12 @@ public void testPatchFilePath() {
3943
@Test(expected = CloudRuntimeException.class)
4044
@PrepareForTest(Script.class )
4145
public void testGetFiles(){
42-
testGetPathFilesExeption(xenServer56Resource);
46+
testGetPathFilesExeption();
4347
}
48+
4449
@Test
4550
@PrepareForTest(Script.class )
4651
public void testGetFilesListReturned(){
47-
testGetPathFilesListReturned(xenServer56Resource);
52+
testGetPathFilesListReturned();
4853
}
4954
}

‎plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56SP2ResourceTest.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.cloud.hypervisor.xenserver.resource;
1717

1818
import org.junit.Assert;
19+
import org.junit.Before;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -26,23 +27,28 @@
2627
@RunWith(PowerMockRunner.class)
2728
public class XenServer56SP2ResourceTest extends CitrixResourceBaseTest{
2829

29-
private XenServer56SP2Resource xenServer56SP2Resource = new XenServer56SP2Resource();
30+
@Before
31+
public void beforeTest() {
32+
super.citrixResourceBase = new XenServer56SP2Resource();
33+
}
3034

3135
@Test
3236
public void testPatchFilePath() {
33-
String patchFilePath = xenServer56SP2Resource.getPatchFilePath();
37+
String patchFilePath = citrixResourceBase.getPatchFilePath();
3438
String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
3539

3640
Assert.assertEquals(patch, patchFilePath);
3741
}
42+
3843
@Test(expected = CloudRuntimeException.class)
3944
@PrepareForTest(Script.class )
4045
public void testGetFiles(){
41-
testGetPathFilesExeption(xenServer56SP2Resource);
46+
testGetPathFilesExeption();
4247
}
48+
4349
@Test
4450
@PrepareForTest(Script.class )
4551
public void testGetFilesListReturned(){
46-
testGetPathFilesListReturned(xenServer56SP2Resource);
52+
testGetPathFilesListReturned();
4753
}
4854
}

0 commit comments

Comments
 (0)