Martin Mucha has posted comments on this change. Change subject: core: tests for NetworkAttachmentValidator ......................................................................
Patch Set 40: (8 comments) https://gerrit.ovirt.org/#/c/36570/40/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkAttachmentValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkAttachmentValidatorTest.java: Line 97: public void testNotExternalNetworkWhenExternalNetworkIsProvided() throws Exception { Line 98: Network externalNetwork = new Network(); Line 99: externalNetwork.setProvidedBy(new ProviderNetwork(Guid.newGuid(), "")); Line 100: Line 101: when(networkDaoMock.get(any(Guid.class))).thenReturn(externalNetwork); > Please be more explicit- setId on the network, pass to the get this id, set Done Line 102: Line 103: assertThat(new NetworkAttachmentValidator(new NetworkAttachment(), new VDS(), managementNetworkUtil).notExternalNetwork(), Line 104: failsWith(VdcBllMessages.EXTERNAL_NETWORK_CANNOT_BE_PROVISIONED)); Line 105: } Line 104: failsWith(VdcBllMessages.EXTERNAL_NETWORK_CANNOT_BE_PROVISIONED)); Line 105: } Line 106: Line 107: @Test Line 108: public void testNotExternalNetwork() throws Exception { > same comment as the previous. Done Line 109: Network notExternalNetwork = new Network(); Line 110: notExternalNetwork.setProvidedBy(null); Line 111: Line 112: when(networkDaoMock.get(any(Guid.class))).thenReturn(notExternalNetwork); Line 143: NetworkAttachment attachment = new NetworkAttachment(); Line 144: attachment.setNetworkId(network.getId()); Line 145: Line 146: NetworkClusterId networkClusterId = new NetworkClusterId(host.getVdsGroupId(), attachment.getNetworkId()); Line 147: when(networkClusterDaoMock.get(eq(networkClusterId))).thenReturn(new NetworkCluster()); > The comment should have been placed in NetworkAttachmentValidator.getNetwor do I understand correctly, that you're complaining about method org.ovirt.engine.core.bll.validator.NetworkAttachmentValidator#getNetworkCluster and replacing there: getNetwork().getId() with attachment.getNetworkId() ??? If so, it should work alright; I don't know why there's getNetwork().getId() and I cannot see reason why it shouldn't be expressed like attachment.getNetworkId(), so we can (probably) alter it. Line 148: when(networkDaoMock.get(Mockito.eq(network.getId()))).thenReturn(network); Line 149: Line 150: assertThat(new NetworkAttachmentValidator(attachment, host, managementNetworkUtil).networkAttachedToCluster(), isValid()); Line 151: } Line 259: NetworkClusterId networkClusterId = new NetworkClusterId(host.getVdsGroupId(), attachment.getNetworkId()); Line 260: when(networkClusterDaoMock.get(eq(networkClusterId))).thenReturn(networkCluster); Line 261: when(networkDaoMock.get(Mockito.eq(network.getId()))).thenReturn(network); Line 262: Line 263: assertThat(new NetworkAttachmentValidator(attachment, host, managementNetworkUtil).bootProtocolSetForDisplayNetwork(), isValid()); > The comment is relevant to the validator itself and not the tests- shouldn' from db table definition & validation annotations I'd guess it's allowed both to be unspecified. But we should probably have that confirmed by moti when he's back from pto. Line 264: } Line 265: Line 266: @Test Line 267: public void testBootProtocolSetForDisplayNetworkWhenIpConfigurationIsNull() throws Exception { Line 423: Network network = new Network(); Line 424: network.setId(Guid.newGuid()); Line 425: network.setName("networkName"); Line 426: Line 427: IConfigUtilsInterface iConfigUtilsInterfaceMock = mock(IConfigUtilsInterface.class); > Please use MockConfigRule- Done Line 428: Config.setConfigUtils(iConfigUtilsInterfaceMock); Line 429: Line 430: when(iConfigUtilsInterfaceMock.getValue(eq(ConfigValues.MultipleGatewaysSupported), anyString())).thenReturn(false); Line 431: when(networkDaoMock.get(any(Guid.class))).thenReturn(network); Line 427: IConfigUtilsInterface iConfigUtilsInterfaceMock = mock(IConfigUtilsInterface.class); Line 428: Config.setConfigUtils(iConfigUtilsInterfaceMock); Line 429: Line 430: when(iConfigUtilsInterfaceMock.getValue(eq(ConfigValues.MultipleGatewaysSupported), anyString())).thenReturn(false); Line 431: when(networkDaoMock.get(any(Guid.class))).thenReturn(network); > Please be explicit, pass the actual id. Done Line 432: when(managementNetworkUtil.isManagementNetwork(network.getId())).thenReturn(false); Line 433: Line 434: VDS host = new VDS(); Line 435: host.setVdsGroupCompatibilityVersion(Version.getLast()); Line 440: Line 441: @Test Line 442: public void testNetworkNotAttachedToHost() throws Exception { Line 443: //no vds for network id. Line 444: when(vdsDaoMock.getAllForNetwork(any(Guid.class))).thenReturn(Collections.<VDS>emptyList()); > Please be explicit, set the networkId. Done Line 445: Line 446: assertThat(new NetworkAttachmentValidator(new NetworkAttachment(), null, managementNetworkUtil).networkNotAttachedToHost(), isValid()); Line 447: } Line 448: Line 451: VDS host = new VDS(); Line 452: host.setId(Guid.newGuid()); Line 453: Line 454: //no vds for network id. Line 455: when(vdsDaoMock.getAllForNetwork(any(Guid.class))).thenReturn(Collections.singletonList(host)); > Please be explicit, set the networkId. Done Line 456: Line 457: assertThat(new NetworkAttachmentValidator(new NetworkAttachment(), host, managementNetworkUtil).networkNotAttachedToHost(), Line 458: failsWith(VdcBllMessages.NETWORK_ALREADY_ATTACHED_TO_HOST)); Line 459: } -- To view, visit https://gerrit.ovirt.org/36570 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia4ba4327b93f8628e234e0e2b23234d8b89902b5 Gerrit-PatchSet: 40 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
