)]}'
{"tempest/scenario/test_security_groups_basic_ops.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"82a7f1c06e070af8fc30cd855acc25f1638c942e","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_605d0046","line":714,"updated":"2020-11-12 17:11:10.000000000","message":"I\u0027m not sure what this adds as this is already being tested in the api test:\n\ntempest/api/network/test_security_groups_negative.py:test_delete_non_existent_security_group()\n\nI think in the end it will be the same neutron code exercised, right?","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7eed4ae2e19ce5b9bbaeebd38647109145a42c5d","unresolved":true,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"17402e3e_661f52d0","line":714,"in_reply_to":"1ba6ad4d_cf9ba03e","updated":"2021-01-05 19:16:26.000000000","message":"Yeah these are little different when come to point of VM operation.\n\n\u003e1) The SG ID it tries to delete was actually EXISTING in the past. \nThis is almost same as test_delete_not_existing_security_group, neutron or nova proxy APIs operation or code path does not make any difference on trying to delete \u0027never existing\u0027 or \u0027currently not existing\u0027 sg. both are same case and they 404 if they do not find the requested sg id in DB. \n\nWhether previously existing sg is deleted properly or not that is \u0027deletion of sg\u0027 test which we verify in many test like - https://github.com/openstack/tempest/blob/fec2c93cdcc14ad08d0a35136ee287525e7a4879/tempest/api/compute/security_groups/test_security_groups.py#L85\n\n\u003e2) As for VM part of the test, we don\u0027t have it in the existing API scenario. \nThese operation negative testing we do not have currently in Tempest. In past we used to accept these type if negative testing but with current (I think 2-3 years old now) Tempest policy [1], Tempest is not right place to test these negative operation which are easily testable in project side unit test or functional test. And this test case already existing in nova unit test -https://github.com/openstack/nova/blob/34c5df7b2b8c7eebbfc8aa0bc3edba501dddcb61/nova/tests/unit/api/openstack/compute/test_security_groups.py#L710\n\n[1] https://docs.openstack.org/tempest/latest/HACKING.html#negative-tests","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"29946c77ba10fcac12039724b8a57728e91a58c4","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"c479a02e_8fff514c","line":714,"in_reply_to":"1f621f24_605d0046","updated":"2020-12-08 14:53:46.000000000","message":"I agree with you. Maybe, one difference is creating an instance. But I\u0027m not sure what is the expectation or difference from the api negative test.","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"2f99240e2ff02ee6295997c4b47bc75392b3f3e6","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"20758202_b0ea6935","line":714,"in_reply_to":"1f621f24_605d0046","updated":"2020-12-08 14:52:17.000000000","message":"yeah, this is negative tests which already exist as mentioned by Brian above. If we have new scenario on sec grps other than the already covered in this file then it will be good to cover those.","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"c1a3d4f86a8e194c71d83deb4b604b695f19e29a","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"e76f7e3f_0520602e","line":714,"in_reply_to":"c479a02e_8fff514c","updated":"2020-12-09 08:51:04.000000000","message":"The only scenario covered in the existing test, is just a single test case that tries to delete SG using random generated ID for that purpose, that it.\n\nThis patch is more than that, as it tries to use a VALID SG ID!\n1) The SG ID it tries to delete was actually EXISTING in the past. Test tries to use a VALID SG ID that shouldn\u0027t be existing anymore. What if SG ID is still \"alive\" and wasn\u0027t actually removed from the system? \n\n2) As for VM part of the test, we don\u0027t have it in the existing API scenario. \n# Test tries to remove \"not existing, but VALID ID that was actually existing in the past\", from the VM.\n# Test tries to remove not associated, but existing security group from VM.\nBoth cases are trying to \"remove SG from ID\" (unassociate) and not to simply delete the SG like and existing API test does, that\u0027s the reason for having VM created.","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"63a781a941c0fbe3812028c6810349af8925eaed","unresolved":true,"context_lines":[{"line_number":711,"context_line":"            lib_exc.NotFound,"},{"line_number":712,"context_line":"            self.os_primary.servers_client.remove_security_group,"},{"line_number":713,"context_line":"            server_id\u003dserver[\u0027id\u0027],"},{"line_number":714,"context_line":"            name\u003dnot_existing_sg[\u0027security_group\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ba6ad4d_cf9ba03e","line":714,"in_reply_to":"e76f7e3f_0520602e","updated":"2021-01-05 15:20:50.000000000","message":"I don\u0027t think it is the same as test_delete_not_existing_security_group test. This one is trying to detach not existing SG from the VM (port) and the one mentioned by Brian is simply calling Neutron API to try to remove SG from the Neutron DB.","commit_id":"c86800cb754afd0d6c7cce11122bf63805203c19"}]}