Martin Mucha has posted comments on this change. Change subject: core: tests for NetworkAttachmentValidator ......................................................................
Patch Set 40: (5 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 77: implementation which returns failing ValidationResult on NetworkValidator#networkIsSet. Finally it's tested, whether Line 78: this ValidationResult was propagated correctly. Line 79: */ Line 80: @Test Line 81: public void testNetworkExists() throws Exception { > Please add test for the valid use case. Done Line 82: NetworkAttachmentValidator networkAttachmentValidatorSpy = Mockito.spy( Line 83: new NetworkAttachmentValidator(new NetworkAttachment(), new VDS(), managementNetworkUtil)); Line 84: Line 85: NetworkValidator networkValidatorSpy = Mockito.spy(new NetworkValidator(new Network())); Line 81: public void testNetworkExists() throws Exception { Line 82: NetworkAttachmentValidator networkAttachmentValidatorSpy = Mockito.spy( Line 83: new NetworkAttachmentValidator(new NetworkAttachment(), new VDS(), managementNetworkUtil)); Line 84: Line 85: NetworkValidator networkValidatorSpy = Mockito.spy(new NetworkValidator(new Network())); > The NetworkValidator can be a mock. Done Line 86: doReturn(networkValidatorSpy).when(networkAttachmentValidatorSpy).createNetworkValidator(); Line 87: Line 88: ValidationResult propagatedResult = new ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS, "a"); Line 89: doReturn(propagatedResult).when(networkValidatorSpy).networkIsSet(); Line 119: public void testNotRemovingManagementNetwork() throws Exception { Line 120: NetworkAttachmentValidator networkAttachmentValidatorSpy = Mockito.spy( Line 121: new NetworkAttachmentValidator(new NetworkAttachment(), new VDS(), managementNetworkUtil)); Line 122: Line 123: NetworkValidator networkValidatorSpy = Mockito.spy(new NetworkValidator(new Network())); > the networkValidator can be a mock. Done Line 124: doReturn(networkValidatorSpy).when(networkAttachmentValidatorSpy).createNetworkValidator(); Line 125: Line 126: ValidationResult propagatedResult = Line 127: new ValidationResult(VdcBllMessages.NETWORK_CANNOT_REMOVE_MANAGEMENT_NETWORK, "a"); Line 177: assertThat(validator.ipConfiguredForStaticBootProtocol(), isValid()); Line 178: } Line 179: Line 180: @Test Line 181: public void testIpConfiguredForStaticBootProtocolWhenIpConfigurationIsNotNullAndBootProtocolIsNotStatic() throws Exception { > Please separate to two tests. I probably did that to cover all enum values in one test, but that does not make sense unless iteration over values is used. But you're right, we cannot make assumptions that 'everything except static' is correct, while we do not(can't) know, what values can be added in future. So iteration is clearly wrong and thus there's not reason to specify all tests in one method. Changed to two positive enum values tests: when it's this value, it's has to be true. Done. Line 182: Line 183: assertThat( Line 184: new NetworkAttachmentValidator( Line 185: createNetworkAttachmentWithIpConfiguration(NetworkBootProtocol.DHCP, null, null), Line 336: isValid()); Line 337: } Line 338: Line 339: @Test Line 340: public void testNetworkIpAddressWasSameAsHostnameAndChangedWhenIpConfigurationIsNotStatic() throws Exception { based on argumentation in testIpConfiguredForStaticBootProtocolWhenIpConfigurationIsNotNullAndBootProtocolIsNotStatic I rewrote this into 2 tests. Line 341: Line 342: for (NetworkBootProtocol networkBootProtocol : NetworkBootProtocol.values()) { Line 343: if (networkBootProtocol == NetworkBootProtocol.STATIC_IP) { Line 344: continue; -- 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
