Juan Hernandez has posted comments on this change. Change subject: restapi: return 404 when there are inexisting resources on path ......................................................................
Patch Set 26: (3 comments) https://gerrit.ovirt.org/#/c/39499/26/backend/manager/modules/dal/src/main/jdbc-resources/engine-daos.properties File backend/manager/modules/dal/src/main/jdbc-resources/engine-daos.properties: Line 89: ExternalVariableDao=org.ovirt.engine.core.dao.ExternalVariableDaoDbFacadeImpl Line 90: VdsKdumpStatusDao=org.ovirt.engine.core.dao.VdsKdumpStatusDaoDbFacadeImpl Line 91: DiskProfileDao=org.ovirt.engine.core.dao.profiles.DiskProfileDaoDbFacadeImpl Line 92: CpuProfileDao=org.ovirt.engine.core.dao.profiles.CpuProfileDaoDbFacadeImpl Line 93: NetworkAttachmentDao=org.ovirt.engine.core.dao.network.NetworkAttachmentDaoDbFacadeImpl This file was probably re-added by accident, remember to remove it before merging. https://gerrit.ovirt.org/#/c/39499/26/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostNetworkAttachmentsResourceTest.java File backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostNetworkAttachmentsResourceTest.java: Line 10: @Override Line 11: protected void verifyIfHostExistsToHandle404StatusCode() { Line 12: //very disgusting hack. Test dictates, that 400 is returned in case of wrong parameters. But 404 Line 13: // has precedence to that, so production code should be able to return 404 when necessary, but I Line 14: // have no idea how to convince rests test to be ok with that. Any help welcomed. The reason for the failure of this test is not that the test dictates 400, but that the verification of the host performs an unexpected (by the test) query. The missing expectation means that the mocked backend will return null instead of a valid response, and that triggers a NPE. The way to solve that is to prepare an expectation for that query: https://gerrit.ovirt.org/40373 Also take into account that this additional query to verify the host may have negative performance impact. Consider if it is really necessary to do it. Line 15: } Line 16: }, VdcQueryType.GetNetworkAttachmentsByHostId); Line 17: } https://gerrit.ovirt.org/#/c/39499/26/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostNicNetworkAttachmentsResourceTest.java File backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostNicNetworkAttachmentsResourceTest.java: Line 14: @Override Line 15: protected void verifyHostAndNicExistence() { Line 16: //very disgusting hack. Test dictates, that 400 is returned in case of wrong parameters. But 404 Line 17: // has precedence to that, so production code should be able to return 404 when necessary, but I Line 18: // have no idea how to convince rests test to be ok with that. Any help welcomed. See the comments here (in this same patch): https://gerrit.ovirt.org/#/c/39499/26/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BckendHostNetworkAttachmentsResourceTest.java The reason for this failure isn't probably that the test dictates 400, but that you need to prepare expectations for the additional queries used to verify the existence of hosts or NICs. Line 19: } Line 20: Line 21: protected void verifyIfHostExistsToHandle404StatusCode() { Line 22: //very disgusting hack. Test dictates, that 400 is returned in case of wrong parameters. But 404 -- To view, visit https://gerrit.ovirt.org/39499 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3e96ebfee8bbf2c6c1714d9d652fa5cc5e46886 Gerrit-PatchSet: 26 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@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
