RodrigoDLopez commented on a change in pull request #4409:
URL: https://github.com/apache/cloudstack/pull/4409#discussion_r506623065
##########
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -3273,8 +3280,17 @@ protected void updateDiskOfferingTagsIfIsNotNull(String
tags, DiskOfferingVO dis
* Check if it needs to update any parameter when updateDiskoffering is
called
* Verify if name or displayText are not blank, tags is not null, sortkey
and displayDiskOffering is not null
*/
- protected boolean shouldUpdateDiskOffering(String name, String
displayText, Integer sortKey, Boolean displayDiskOffering, String tags) {
- return StringUtils.isNotBlank(name) ||
StringUtils.isNotBlank(displayText) || tags != null || sortKey != null ||
displayDiskOffering != null;
+ protected boolean shouldUpdateDiskOffering(String name, String
displayText, Integer sortKey, Boolean displayDiskOffering, String tags, String
cacheMode) {
Review comment:
those change will not impact into edit diskoffering tags
##########
File path:
server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
##########
@@ -946,17 +946,32 @@ public void
validateMaximumIopsAndBytesLengthTestDefaultLengthConfigs() {
@Test
public void shouldUpdateDiskOfferingTests(){
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(),
Mockito.anyString(), Mockito.anyInt(), Mockito.anyBoolean(),
Mockito.anyString()));
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(),
nullable(String.class), nullable(Integer.class), nullable(Boolean.class),
nullable(String.class)));
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
Mockito.anyString(), nullable(Integer.class), nullable(Boolean.class),
nullable(String.class)));
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), Mockito.anyInt(), nullable(Boolean.class),
nullable(String.class)));
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), nullable(int.class), Mockito.anyBoolean(),
nullable(String.class)));
-
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), nullable(int.class), nullable(Boolean.class),
Mockito.anyString()));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(),
Mockito.anyString(), Mockito.anyInt(), Mockito.anyBoolean(),
Mockito.anyString(), Mockito.anyString()));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(),
nullable(String.class), nullable(Integer.class), nullable(Boolean.class),
nullable(String.class), nullable(String.class)));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
Mockito.anyString(), nullable(Integer.class), nullable(Boolean.class),
nullable(String.class), nullable(String.class)));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), Mockito.anyInt(), nullable(Boolean.class),
nullable(String.class), nullable(String.class)));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), nullable(int.class), Mockito.anyBoolean(),
nullable(String.class), nullable(String.class)));
+
Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class),
nullable(String.class), nullable(int.class), nullable(Boolean.class),
Mockito.anyString(), Mockito.anyString()));
}
@Test
public void shouldUpdateDiskOfferingTestFalse(){
- Assert.assertFalse(configurationMgr.shouldUpdateDiskOffering(null,
null, null, null, null));
+ Assert.assertFalse(configurationMgr.shouldUpdateDiskOffering(null,
null, null, null, null, null));
+ }
+
+ @Test
+ public void shouldUpdateIopsRateParametersTestFalse() {
+ configurationMgr.shouldUpdateIopsRateParameters(null, null, null,
null, null, null);
Review comment:
I believe your test is incorrect.
You need an assertion here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
