Moti Asayag has posted comments on this change.

Change subject: core: tests for 
HostSetupNetworksValidator#validateCustomProperties()
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/36998/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java:

Line 448:     private boolean validateCustomProperties() {
Line 449:         String version = 
host.getVdsGroupCompatibilityVersion().getValue();
Line 450:         SimpleCustomPropertiesUtil util = 
SimpleCustomPropertiesUtil.getInstance();
Line 451: 
Line 452:         Map<String, String> validPropertiesForVm =
the name misleads- i think it worth mentioning the entire context to reduce 
confusion:

 validPropertiesForVm/validPropertiesForVmNetowrk
Line 453:                 util.convertProperties(Config.<String> 
getValue(ConfigValues.PreDefinedNetworkCustomProperties, version));
Line 454:         
validPropertiesForVm.putAll(util.convertProperties(Config.<String> 
getValue(ConfigValues.UserDefinedNetworkCustomProperties,
Line 455:                 version)));
Line 456: 


http://gerrit.ovirt.org/#/c/36998/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java:

Line 58:         host.setVdsGroupCompatibilityVersion(Version.v3_5);
Line 59:     }
Line 60: 
Line 61:     @Test
Line 62:     public void testNetworksOnNicMatchMtu_whenNoNetworksAreProvided() 
throws Exception {
please maintain java naming convention and remove underscore from method's name 
across the file.
Line 63: 
Line 64:         Map<String, List<Network>> networksOnNics =
Line 65:                 Collections.singletonMap("nicName", 
Collections.<Network> emptyList());
Line 66: 


Line 92:     }
Line 93: 
Line 94:     /**
Line 95:      * this is probably not a valid scenario, but validation method 
allows this.
Line 96:      */
need to see if from some reason this was allowed in previous api. 

it might be that a case of:
  eth0 (mtu 10) ---- red (mtu 10)
                |
                ---- blue (mtu 5)
Line 97:     @Test
Line 98:     public void 
testNetworksOnNicMatchMtu_ignoreMtuDifferenceWhenBothNetworksAreVmNetworks() 
throws Exception {
Line 99:         List<Network> networks = Arrays.asList(createNetwork(1, true),
Line 100:                 createNetwork(2, true));


-- 
To view, visit http://gerrit.ovirt.org/36998
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I24c3458f39f5023d40e09f530847c1ccf36041e5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to