Mike Kolesnik has posted comments on this change.
Change subject: engine: Refactored code into NetworkValidator class
......................................................................
Patch Set 6: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java
Line 15: import org.ovirt.engine.core.dal.VdcBllMessages;
Line 16:
Line 17: @SuppressWarnings("serial")
Line 18: public class UpdateNetworkCommand<T extends
AddNetworkStoragePoolParameters> extends NetworkCommon<T> {
Line 19: private List<NetworkCluster> clusterAttachments;
I think it was being used in the can do action also, but maybe it was a mistake
to begin with..
Line 20:
Line 21: public UpdateNetworkCommand(T parameters) {
Line 22: super(parameters);
Line 23: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java
Line 138:
Line 139: /**
Line 140: * @return An error iff the network is in fact the data center's
management network.
Line 141: */
Line 142: public ValidationResult notManagementNetwork() {
Maybe, but not with this fixed error message..
Line 143: String managementNetwork = Config.<String>
GetValue(ConfigValues.ManagementNetwork);
Line 144: return managementNetwork.equalsIgnoreCase(network.getName())
Line 145: ? new
ValidationResult(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK)
Line 146: : ValidationResult.VALID;
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkValidatorTest.java
Line 86: public void dataCenterExists() throws Exception {
Line 87: assertEquals(ValidationResult.VALID,
validator.dataCenterExists());
Line 88: }
Line 89:
Line 90: private void vmNetworkSetupTest(ValidationResult expected, boolean
isVmNetwork, boolean isFeatureSupported) {
JavaBean naming convention.
You can look at section 8.3.2 here:
https://docs.google.com/viewer?a=v&q=cache:lJ3hOUA7OiIJ:download.oracle.com/otn-pub/jcp/7224-javabeans-1.01-fr-spec-oth-JSpec/beans.101.pdf+&hl=en&pid=bl&srcid=ADGEESitSi0XubJYzFMBMZRUPezhm-4Dr9xZ7C2wiSz_gWNoldd4j2M9tdPwT50wOVh8c_bitSCnf_I-UyJgczC6O-q9O7euX-Ryn1ELgh04Uy0KkoSMx3oLv-jONbw1bd7bjm1xq6LA&sig=AHIEtbSW5qnUls1krWchDWhtbsE8ANtrNg
Line 91:
mockConfigRule.mockConfigValue(ConfigValues.NonVmNetworkSupported, null,
isFeatureSupported);
Line 92: when(network.isVmNetwork()).thenReturn(isVmNetwork);
Line 93:
Line 94: assertEquals(expected.getMessage(),
validator.vmNetworkSetCorrectly().getMessage());
Line 299:
Line 300: networkNameAvailableTest(ValidationResult.VALID,
Collections.singletonList(otherNetwork));
Line 301: }
Line 302:
Line 303: private void networkNotUsedTest(ValidationResult expected,
List<Nameable> nameables) {
What is the "testing hole" that you're referring to?
Also the test is not supposed to "know" what internal logic is used, be it
private or protected methods, it should test the contract.
The only exception we ever do for that rule is external dependencies which
currently are not being injected into the objects, but if they were then we
wouldn't even need to mock those methods and could truly perform "black box"
testing.
Also the one of the main reasons for the tests is to ward against future
breakage, so I would say it's a very good reason.
Line 304: assertEquals(expected, validator.networkNotUsed(nameables,
VdcBllMessages.VAR__ENTITIES__VMS));
Line 305: }
Line 306:
Line 307: @Test
--
To view, visit http://gerrit.ovirt.org/10940
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf40d81f0481c6b6dec141a71363888dc9e9a941
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
