Lior Vernia has posted comments on this change. Change subject: engine,webadmin: Replace references to NetworkQoS with HostNetworkQos ......................................................................
Patch Set 9: (6 comments) http://gerrit.ovirt.org/#/c/34131/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java: Line 256: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_NOT_SUPPORTED, iface.getNetworkName()); Line 257: } Line 258: Line 259: HostNetworkQosValidator qosValidator = new HostNetworkQosValidator(iface.getQos()); Line 260: if (qosValidator.requiredValuesPresent() != ValidationResult.VALID) { > Please add tests for these two validations. They're validated as part of HostNetworkQosValidatorTest... What validation is missing? That the right error is returned here? Line 261: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES, Line 262: iface.getNetworkName()); Line 263: } Line 264: if (qosValidator.valuesConsistent() != ValidationResult.VALID) { http://gerrit.ovirt.org/#/c/34131/9/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java: Line 239 Line 240 Line 241 Line 242 Line 243 > Those methods are stiil part of the validator. Why not testing them? Done > I don't understand why did you have to change this file at all. Can't remember, maybe an error. Line 1: package org.ovirt.engine.core.bll.validator; Line 2: Line 3: import static org.junit.Assert.assertThat; Line 4: import static org.mockito.Mockito.doReturn; Line 20: public class NetworkQosValidatorTest { Line 21: Line 22: private static final Guid DEFAULT_GUID = Guid.newGuid(); Line 23: private static final Guid OTHER_GUID = Guid.newGuid(); Line 24: private static final Integer BANDWIDTH = 100; > What is the purpose of this change? Same. Line 25: Line 26: private NetworkQoS qos; Line 27: private NetworkQoS oldQos; Line 28: private List<NetworkQoS> allQos; http://gerrit.ovirt.org/#/c/34131/9/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/qos/DataCenterHostNetworkQosListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/qos/DataCenterHostNetworkQosListModel.java: Line 38: } Line 39: Line 40: @Override Line 41: protected RemoveQosModel<HostNetworkQos> getRemoveQosModel() { Line 42: return new RemoveHostNetworkQosModel(this); > How is it related to this patch and how did it compile previously? Rebase artifact, fixed in earlier patch. Line 43: } Line 44: Line 45: @Override Line 46: protected String getListName() { http://gerrit.ovirt.org/#/c/34131/9/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostInterfaceModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostInterfaceModel.java: Line 349: Line 350: if (ev.matchesDefinition(ListModel.selectedItemChangedEventDefinition) && sender == getNetwork()) { Line 351: network_SelectedItemChanged(null); Line 352: } else if (sender == getQosOverridden()) { Line 353: getQosModel().setIsChangable(getQosOverridden().getEntity()); > You should also consider the changeabilty state of the qosOverriden. (Exapl Done Line 354: } Line 355: } Line 356: Line 357: private void network_SelectedItemChanged(EventArgs e) -- To view, visit http://gerrit.ovirt.org/34131 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6a625d8c5b554cb525efc41d31013df0de2a146 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@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
