)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e836ae8d7eebbeb51fd5f2c3cf4a953d64dc47e9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix import line location"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When using fix option in order to add UUIDs to those"},{"line_number":10,"context_line":"tests that have no UUIDs, the decorators import line"},{"line_number":11,"context_line":"(\"from tempest.lib import decorators\") is added at the"},{"line_number":12,"context_line":"end of the file."},{"line_number":13,"context_line":"This behavior happens when excuting check_uuid script"},{"line_number":14,"context_line":"against some tempest plugin project (e.g. manila-tempest-plugin)."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"9f560f44_a914924f","line":12,"range":{"start_line":10,"start_character":25,"end_line":12,"end_character":16},"updated":"2020-07-30 16:58:54.000000000","message":"this is not happening in all cases. i tried for Tempest and cinder-tempest-plugin and both are modified with correct import line. But for manila tempest plugin it produces the error you mentioned here ? any idea what specific case it give error.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2281910a09f9d57ae73f326be3fb05a2cda74e85","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix import line location"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When using fix option in order to add UUIDs to those"},{"line_number":10,"context_line":"tests that have no UUIDs, the decorators import line"},{"line_number":11,"context_line":"(\"from tempest.lib import decorators\") is added at the"},{"line_number":12,"context_line":"end of the file."},{"line_number":13,"context_line":"This behavior happens when excuting check_uuid script"},{"line_number":14,"context_line":"against some tempest plugin project (e.g. manila-tempest-plugin)."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"9f560f44_70fb5646","line":12,"range":{"start_line":10,"start_character":25,"end_line":12,"end_character":16},"in_reply_to":"9f560f44_615baef6","updated":"2020-08-02 11:29:06.000000000","message":"Thanks for you comments.\nDone","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix import line location"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When using fix option in order to add UUIDs to those"},{"line_number":10,"context_line":"tests that have no UUIDs, the decorators import line"},{"line_number":11,"context_line":"(\"from tempest.lib import decorators\") is added at the"},{"line_number":12,"context_line":"end of the file."},{"line_number":13,"context_line":"This behavior happens when excuting check_uuid script"},{"line_number":14,"context_line":"against some tempest plugin project (e.g. manila-tempest-plugin)."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"9f560f44_615baef6","line":12,"range":{"start_line":10,"start_character":25,"end_line":12,"end_character":16},"in_reply_to":"9f560f44_a914924f","updated":"2020-07-30 20:33:59.000000000","message":"Goutham point it that this is case when there is no tempest import in the file\n- https://github.com/openstack/tempest/blob/f2727fa6b4eb9ed3a8b48f6b8a6b0d34d72307fb/tempest/lib/cmd/check_uuid.py#L186-L191\n\nPlease add that case in cmt msg.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"}],"tempest/lib/cmd/check_uuid.py":[{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"34c1f0006a8f1c1bdf829f463ae823bac40b094f","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            patch +\u003d \u0027\\n\u0027"},{"line_number":68,"context_line":"        self.patches[patch_id] \u003d self._quote(patch)"},{"line_number":69,"context_line":"        lines \u003d self.source_files[filename].split(self._quote(\u0027\\n\u0027))"},{"line_number":70,"context_line":"        lines[line_no - 1] \u003d \u0027\u0027.join((\u0027{%s:s}\u0027 % patch_id, lines[line_no - 1]))"},{"line_number":71,"context_line":"        self.source_files[filename] \u003d self._quote(\u0027\\n\u0027).join(lines)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_c8928458","line":70,"range":{"start_line":70,"start_character":8,"end_line":70,"end_character":79},"updated":"2020-01-08 12:46:28.000000000","message":"if line_no is 0, then the patch is added to lines[-1] which is the end of the file","commit_id":"14c78eb7f0fe19bc97cea523390e2fabfc861a11"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"496b71fb0a8b2a31c1950af61d2c86842a5f92be","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        tempest_imports \u003d [node for node in src_parsed.body"},{"line_number":188,"context_line":"                           if self._import_name(node) and"},{"line_number":189,"context_line":"                           \u0027tempest\u0027 in self._import_name(node)]"},{"line_number":190,"context_line":"        if not tempest_imports:"},{"line_number":191,"context_line":"            import_snippet \u003d \u0027\\n\u0027.join((\u0027\u0027, IMPORT_LINE, \u0027\u0027))"},{"line_number":192,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8db1c948","line":189,"range":{"start_line":188,"start_character":53,"end_line":189,"end_character":63},"updated":"2020-01-12 12:54:01.000000000","message":"Hi Martin,\nThanks for your comments,\nTo be honest I don\u0027t understand what is the purpose to differentiate between tempest import and non tempest import, we just want to know where to place the import line. \nAfter all, the uuid decorator is added to all tests.\nI think this condition is unnecessary, why not compare with all imports.","commit_id":"14c78eb7f0fe19bc97cea523390e2fabfc861a11"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"34c1f0006a8f1c1bdf829f463ae823bac40b094f","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        with open(source_path) as f:"},{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        tempest_imports \u003d [node for node in src_parsed.body"},{"line_number":188,"context_line":"                           if self._import_name(node) and"},{"line_number":189,"context_line":"                           \u0027tempest\u0027 in self._import_name(node)]"},{"line_number":190,"context_line":"        if not tempest_imports:"},{"line_number":191,"context_line":"            import_snippet \u003d \u0027\\n\u0027.join((\u0027\u0027, IMPORT_LINE, \u0027\u0027))"},{"line_number":192,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_c8f604d9","line":189,"range":{"start_line":187,"start_character":8,"end_line":189,"end_character":64},"updated":"2020-01-08 12:46:28.000000000","message":"I think it\u0027s supposed to find only tempest imports here (from tempest module) therefore there is the dot, by removing it, it will look for any imports in which \u0027tempest\u0027 is a substring of the import. It seems it intentionally seeks tempest imports so that the new import line (from tempest.lib import decorators) is put among tempest imports (as that\u0027s tempest import as well) - by removing the dot, this wouldn\u0027t work anymore.","commit_id":"14c78eb7f0fe19bc97cea523390e2fabfc861a11"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"03a1ef6932a6585912daf713e1493a1e0fc14a02","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        tempest_imports \u003d [node for node in src_parsed.body"},{"line_number":188,"context_line":"                           if self._import_name(node) and"},{"line_number":189,"context_line":"                           \u0027tempest\u0027 in self._import_name(node)]"},{"line_number":190,"context_line":"        if not tempest_imports:"},{"line_number":191,"context_line":"            import_snippet \u003d \u0027\\n\u0027.join((\u0027\u0027, IMPORT_LINE, \u0027\u0027))"},{"line_number":192,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_599aaf75","line":189,"range":{"start_line":188,"start_character":53,"end_line":189,"end_character":63},"in_reply_to":"3fa7e38b_8db1c948","updated":"2020-01-13 09:03:19.000000000","message":"Hi Liron,\nhere is my theory:\nthe code needs to find out on which line the new import (from tempest.lib import decorators) should be added. Yes, it needs to be added among other imports, so technically, it would be enough to look for any import as you suggested, however, based on pep8 recommendations (at least I guess it\u0027s pep8) the imports from the same module (in our case tempest for example) should be gathered together, therefore it\u0027s needed to differentiate between tempest and not tempest imports as the new import which is about to be added is a tempest one.\n\nExample:\n\n# wrong result\nfrom tempest import tempest.api.bla\nfrom tempest import tempest.cmd\n\nimport os\nimport sys\n\nfrom tempest.lib import decorators\n\n----------------------------------------\n# expected result\nfrom tempest import tempest.api.bla\nfrom tempest import tempest.cmd\nfrom tempest.lib import decorators\n\nimport os\nimport sys","commit_id":"14c78eb7f0fe19bc97cea523390e2fabfc861a11"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"34c1f0006a8f1c1bdf829f463ae823bac40b094f","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        tempest_imports \u003d [node for node in src_parsed.body"},{"line_number":188,"context_line":"                           if self._import_name(node) and"},{"line_number":189,"context_line":"                           \u0027tempest\u0027 in self._import_name(node)]"},{"line_number":190,"context_line":"        if not tempest_imports:"},{"line_number":191,"context_line":"            import_snippet \u003d \u0027\\n\u0027.join((\u0027\u0027, IMPORT_LINE, \u0027\u0027))"},{"line_number":192,"context_line":"        else:"},{"line_number":193,"context_line":"            for node in tempest_imports:"},{"line_number":194,"context_line":"                if self._import_name(node) \u003c DECORATOR_IMPORT:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_08a5fcb3","line":191,"range":{"start_line":190,"start_character":0,"end_line":191,"end_character":61},"updated":"2020-01-08 12:46:28.000000000","message":"the real problem here is that if the source file doesn\u0027t contain any tempest imports, import_snippet in this condition will be set to the IMPORT_LINE (that\u0027s fine) but notice, that line_no is not changed - it\u0027s still set to 0 which means that the IMPORT_LINE is gonna be added at the very beginning of the source file, however, then the add_patch method is called (line 60 in this file) where -1 is subtracted from line_no, see the other comment\n\nI would suggest to modify the line_no in case there are no tempest imports to avoid line_no \u003d\u003d 0, the question is how to do that, maybe find lines with \u0027import\u0027 or \u0027from\u0027 keywords? It would probably required a regex (^import |^from.*import or something similar) so that we don\u0027t end up with grepping all \u0027import\u0027 and \u0027from\u0027 words in comments, names etc... or something else, I don\u0027t know, I\u0027ll leave it up to you :)","commit_id":"14c78eb7f0fe19bc97cea523390e2fabfc861a11"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"0d9b0f3c2ffde26189ae6e3db87f17fc3b0cc226","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        with open(source_path) as f:"},{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        import_list \u003d src_parsed.body[:]"},{"line_number":188,"context_line":"        tempest_imports \u003d [import_list.pop(idx) for idx, node in"},{"line_number":189,"context_line":"                           enumerate(import_list) if self._import_name(node)"},{"line_number":190,"context_line":"                           and \u0027tempest.\u0027 in self._import_name(node)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_8af8bd18","line":187,"range":{"start_line":187,"start_character":37,"end_line":187,"end_character":40},"updated":"2020-01-14 08:25:48.000000000","message":"is it necessary?","commit_id":"54997d286af002743c3ce3b5c1fcb6f755fbf69c"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2ea5d62a99842bc07c00bbeb647359a2f3a2f450","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        with open(source_path) as f:"},{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        import_list \u003d src_parsed.body[:]"},{"line_number":188,"context_line":"        tempest_imports \u003d [import_list.pop(idx) for idx, node in"},{"line_number":189,"context_line":"                           enumerate(import_list) if self._import_name(node)"},{"line_number":190,"context_line":"                           and \u0027tempest.\u0027 in self._import_name(node)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_e0262dba","line":187,"range":{"start_line":187,"start_character":37,"end_line":187,"end_character":40},"in_reply_to":"3fa7e38b_8af8bd18","updated":"2020-01-14 09:09:08.000000000","message":"Martin,\nYou absolutely right.\nThat\u0027s what happens when not thinking simply :)\nI\u0027m fixing it now.","commit_id":"54997d286af002743c3ce3b5c1fcb6f755fbf69c"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"0d9b0f3c2ffde26189ae6e3db87f17fc3b0cc226","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        import_list \u003d src_parsed.body[:]"},{"line_number":188,"context_line":"        tempest_imports \u003d [import_list.pop(idx) for idx, node in"},{"line_number":189,"context_line":"                           enumerate(import_list) if self._import_name(node)"},{"line_number":190,"context_line":"                           and \u0027tempest.\u0027 in self._import_name(node)]"},{"line_number":191,"context_line":"        if tempest_imports:"},{"line_number":192,"context_line":"            import_list \u003d tempest_imports"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_5d8e0a7b","line":189,"range":{"start_line":188,"start_character":26,"end_line":189,"end_character":76},"updated":"2020-01-14 08:25:48.000000000","message":"correct me if I\u0027m wrong, but I think it can be written more efficiently - in case, there is at least one tempest import, in the condition below import_list will be overridden by tempest_imports, so, maybe popping items from import_list in this comprehension is not necessary and then you also wouldn\u0027t need to use enumarate function for getting an index of an item.","commit_id":"54997d286af002743c3ce3b5c1fcb6f755fbf69c"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"96abb1427543da222d6b361447ca4a61aee06ba2","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            src_lines \u003d f.read().split(\u0027\\n\u0027)"},{"line_number":186,"context_line":"        line_no \u003d 0"},{"line_number":187,"context_line":"        import_list \u003d src_parsed.body"},{"line_number":188,"context_line":"        tempest_imports \u003d [node for node in src_parsed.body"},{"line_number":189,"context_line":"                           if self._import_name(node) and"},{"line_number":190,"context_line":"                           \u0027tempest.\u0027 in self._import_name(node)]"},{"line_number":191,"context_line":"        if tempest_imports:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_31d495bc","line":188,"range":{"start_line":188,"start_character":44,"end_line":188,"end_character":59},"updated":"2020-01-14 12:27:58.000000000","message":"this could be replaced by import_list, but it\u0027s also fine as is","commit_id":"50afefa14dbec5150609d3c396baecd9f24bd914"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no import line"},{"line_number":204,"context_line":"            elif not import_list:"},{"line_number":205,"context_line":"                line_no \u003d 1"},{"line_number":206,"context_line":"                import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\\n\u0027"},{"line_number":207,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":208,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_04a76551","line":205,"range":{"start_line":205,"start_character":16,"end_line":205,"end_character":27},"updated":"2020-07-30 20:33:59.000000000","message":"this will go into licence paragraph[1]. we can add it as 15 so that in most of the cases it will fall after licence paragraph also add note for the same to know reason of 15.\n\n[1] https://github.com/openstack/manila-tempest-plugin/blob/master/manila_tempest_tests/tests/api/test_access_rules_metadata.py#L14","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2281910a09f9d57ae73f326be3fb05a2cda74e85","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no import line"},{"line_number":204,"context_line":"            elif not import_list:"},{"line_number":205,"context_line":"                line_no \u003d 1"},{"line_number":206,"context_line":"                import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\\n\u0027"},{"line_number":207,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":208,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_30055e47","line":205,"range":{"start_line":205,"start_character":16,"end_line":205,"end_character":27},"in_reply_to":"9f560f44_04a76551","updated":"2020-08-02 11:29:06.000000000","message":"This means there is no import line in the module at all.\nSuch a case can not be, because the class in the module must be a subclass \nof unittest.TestCase(means there is at least one import line), otherwise, \"_is_test_case\" method in L160 will not discover the methods with prefix \"test_\" in the class as tests.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0948a2a1883e0e12f26c5ea0e766b00e6492d69f","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no import line"},{"line_number":204,"context_line":"            elif not import_list:"},{"line_number":205,"context_line":"                line_no \u003d 1"},{"line_number":206,"context_line":"                import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\\n\u0027"},{"line_number":207,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":208,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_1b931b5d","line":205,"range":{"start_line":205,"start_character":16,"end_line":205,"end_character":27},"in_reply_to":"9f560f44_30055e47","updated":"2020-08-02 17:43:21.000000000","message":"yeah, that is not possible case. so let\u0027s print msg of this invalid case and return without adding the patch","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            elif not import_list:"},{"line_number":205,"context_line":"                line_no \u003d 1"},{"line_number":206,"context_line":"                import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\\n\u0027"},{"line_number":207,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":208,"context_line":"            else:"},{"line_number":209,"context_line":"                line_no \u003d import_list[-1].lineno + 1"},{"line_number":210,"context_line":"                import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE + \u0027\\n\u0027"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        patcher.add_patch(source_path, import_snippet, line_no)"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_84b535d0","line":210,"range":{"start_line":207,"start_character":0,"end_line":210,"end_character":58},"updated":"2020-07-30 20:33:59.000000000","message":"I am ok with this. This is best we can do in simple way. otherwise, tempest import needs to be added in between of std lib and arg.package import.\n\nWe do not do hacking check for import order anymore so fixing those manually is fine.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2281910a09f9d57ae73f326be3fb05a2cda74e85","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            elif not import_list:"},{"line_number":205,"context_line":"                line_no \u003d 1"},{"line_number":206,"context_line":"                import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\\n\u0027"},{"line_number":207,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":208,"context_line":"            else:"},{"line_number":209,"context_line":"                line_no \u003d import_list[-1].lineno + 1"},{"line_number":210,"context_line":"                import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE + \u0027\\n\u0027"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        patcher.add_patch(source_path, import_snippet, line_no)"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_f0e766ab","line":210,"range":{"start_line":207,"start_character":0,"end_line":210,"end_character":58},"in_reply_to":"9f560f44_84b535d0","updated":"2020-08-02 11:29:06.000000000","message":"I fixed and tested it.\nNow it should work when there is no tempest import in the module, like in some modules in manila-tempest-plugin.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0948a2a1883e0e12f26c5ea0e766b00e6492d69f","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":204,"context_line":"            else:"},{"line_number":205,"context_line":"                for import_line in import_list:"},{"line_number":206,"context_line":"                    if self._import_name(import_line) \u003c DECORATOR_IMPORT:"},{"line_number":207,"context_line":"                        continue"},{"line_number":208,"context_line":"                    else:"},{"line_number":209,"context_line":"                        line_no \u003d import_line.lineno"},{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_fb8107fd","line":211,"range":{"start_line":205,"start_character":0,"end_line":211,"end_character":29},"updated":"2020-08-02 17:43:21.000000000","message":"I think this can end up adding tempest import in between of project import. Previous logic was fine which can be edited manually if it needs to move up.\n\n\n            ...\n            # There is no tempest import \u003d\u003e create new block of tempest imports\n            elif import_list:\n                line_no \u003d import_list[-1].lineno + 1\n                import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE + \u0027\\n\u0027\n            else:\n                print(\"This is case where no import is present inthe test file \"\n                      \"which is not a valid case so not applying test id.\")\n                return","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d7e6a23c82d7a9dd95feb58715f2aad92e405839","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":204,"context_line":"            else:"},{"line_number":205,"context_line":"                for import_line in import_list:"},{"line_number":206,"context_line":"                    if self._import_name(import_line) \u003c DECORATOR_IMPORT:"},{"line_number":207,"context_line":"                        continue"},{"line_number":208,"context_line":"                    else:"},{"line_number":209,"context_line":"                        line_no \u003d import_line.lineno"},{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_0e3652b5","line":211,"range":{"start_line":205,"start_character":0,"end_line":211,"end_character":29},"in_reply_to":"9f560f44_1b5e7b43","updated":"2020-09-03 21:25:07.000000000","message":"agree this case we can handle when there are no tempest import but other non-project import like from testtools....\n\nBut the case it break is when there is no third party or non-project import then this logic end up adding tempest import in between of project imports.","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"9773d699a775e80c1ef4cf86bed5c0b1f221bdac","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":204,"context_line":"            else:"},{"line_number":205,"context_line":"                for import_line in import_list:"},{"line_number":206,"context_line":"                    if self._import_name(import_line) \u003c DECORATOR_IMPORT:"},{"line_number":207,"context_line":"                        continue"},{"line_number":208,"context_line":"                    else:"},{"line_number":209,"context_line":"                        line_no \u003d import_line.lineno"},{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_5b8d93d1","line":211,"range":{"start_line":205,"start_character":0,"end_line":211,"end_character":29},"in_reply_to":"9f560f44_5b43d3af","updated":"2020-08-02 19:17:03.000000000","message":"what all cases? in case of no tempest import we need to add the new block for tempest import in the end right?\n\nIn PS10, it will end up adding in between of non tempest import. In PS9, it correctly add after the non tempest import.\n\nI am showing the test case to verify this case failing with PS10 and will pass with PS9 changes - https://review.opendev.org/#/c/744048/3/tempest/tests/lib/cmd/test_check_uuid.py","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"9f3d59dd12e2ca4af6300fe9ce119cb3858c1cc6","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":204,"context_line":"            else:"},{"line_number":205,"context_line":"                for import_line in import_list:"},{"line_number":206,"context_line":"                    if self._import_name(import_line) \u003c DECORATOR_IMPORT:"},{"line_number":207,"context_line":"                        continue"},{"line_number":208,"context_line":"                    else:"},{"line_number":209,"context_line":"                        line_no \u003d import_line.lineno"},{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_1b5e7b43","line":211,"range":{"start_line":205,"start_character":0,"end_line":211,"end_character":29},"in_reply_to":"9f560f44_5b8d93d1","updated":"2020-08-02 19:30:11.000000000","message":"In manila-tempest-plugin there are some modules that contain the following import:\n\"from testtools import testcase as tc\"\n\nIn that case \"from tempest.lib import decorators\" should be above this import line, like:\n\nfrom tempest.lib import decorators\n\nfrom testtools import testcase as tc","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"5f6b59c4dc235ab4d0aeee6316561b3c970c3273","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                import_snippet \u003d IMPORT_LINE"},{"line_number":203,"context_line":"            # There is no tempest import \u003d\u003e create new block of tempest imports"},{"line_number":204,"context_line":"            else:"},{"line_number":205,"context_line":"                for import_line in import_list:"},{"line_number":206,"context_line":"                    if self._import_name(import_line) \u003c DECORATOR_IMPORT:"},{"line_number":207,"context_line":"                        continue"},{"line_number":208,"context_line":"                    else:"},{"line_number":209,"context_line":"                        line_no \u003d import_line.lineno"},{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_5b43d3af","line":211,"range":{"start_line":205,"start_character":0,"end_line":211,"end_character":29},"in_reply_to":"9f560f44_fb8107fd","updated":"2020-08-02 18:20:01.000000000","message":"But the import order will be wrong.\nWhy not cover all cases?\nThis happens in a lot of modules in manila-tempest-plugin.","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0948a2a1883e0e12f26c5ea0e766b00e6492d69f","unresolved":false,"context_lines":[{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        patcher.add_patch(source_path, import_snippet, line_no)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_5b991377","line":214,"range":{"start_line":213,"start_character":0,"end_line":214,"end_character":55},"updated":"2020-08-02 17:43:21.000000000","message":"here:\n\n                print(\"This is case where no import is present inthe test file \"\n                      \"which is not a valid case so not applying test id.\")\n                return","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"5f6b59c4dc235ab4d0aeee6316561b3c970c3273","unresolved":false,"context_lines":[{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        patcher.add_patch(source_path, import_snippet, line_no)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_bb15cf9e","line":214,"range":{"start_line":213,"start_character":0,"end_line":214,"end_character":55},"in_reply_to":"9f560f44_5b991377","updated":"2020-08-02 18:20:01.000000000","message":"This condition will never happen.\nAs I mentioned before \"tests \u003d checker.get_tests()\" in L353\nwill not discover module without one import line.\nIf we will take a module without any import line, checker.get_tests()\nwill not include this module.","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"9773d699a775e80c1ef4cf86bed5c0b1f221bdac","unresolved":false,"context_lines":[{"line_number":210,"context_line":"                        import_snippet \u003d IMPORT_LINE + \u0027\\n\\n\u0027"},{"line_number":211,"context_line":"                        break"},{"line_number":212,"context_line":"                else:"},{"line_number":213,"context_line":"                    line_no \u003d import_list[-1].lineno + 1"},{"line_number":214,"context_line":"                    import_snippet \u003d \u0027\\n\u0027 + IMPORT_LINE"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        patcher.add_patch(source_path, import_snippet, line_no)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_fbb5877f","line":214,"range":{"start_line":213,"start_character":0,"end_line":214,"end_character":55},"in_reply_to":"9f560f44_bb15cf9e","updated":"2020-08-02 19:17:03.000000000","message":"yeah, in that case I suggest we do not call add_patch and print warning if any other method start using that for no import cases also.","commit_id":"ad080bc78d703a6babedc31665f4d8aad0a6478e"}],"tempest/tests/lib/cmd/test_check_uuid.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        source_code \u003d \"from unittest import mock\\n\""},{"line_number":105,"context_line":"        fake_file.write(source_code)"},{"line_number":106,"context_line":"        fake_file.close()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        class Fake_src_parsed():"},{"line_number":109,"context_line":"            body \u003d [TestTestChecker.get_mocked_ast_object("},{"line_number":110,"context_line":"                1, 4, \u0027unittest\u0027, \u0027mock\u0027, ast.ImportFrom)]"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_dbf1c7cf","line":107,"range":{"start_line":107,"start_character":0,"end_line":107,"end_character":0},"updated":"2020-07-30 20:33:59.000000000","message":"modified tests still not capturing the current issue which is line number, let\u0027s check line no also if those are calculated correctly\n\nwe can mock the add_patch() and assert on line number (it should not be 0 in current fake_file in this tests) and then call the add_patch() again to verify the existing assert.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b539e1294298cb87e6bda4acb4bb02241762efdd","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        source_code \u003d \"from unittest import mock\\n\""},{"line_number":105,"context_line":"        fake_file.write(source_code)"},{"line_number":106,"context_line":"        fake_file.close()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        class Fake_src_parsed():"},{"line_number":109,"context_line":"            body \u003d [TestTestChecker.get_mocked_ast_object("},{"line_number":110,"context_line":"                1, 4, \u0027unittest\u0027, \u0027mock\u0027, ast.ImportFrom)]"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_c1171aed","line":107,"range":{"start_line":107,"start_character":0,"end_line":107,"end_character":0},"in_reply_to":"9f560f44_dbf1c7cf","updated":"2020-07-30 20:35:36.000000000","message":"I added the line number check in https://review.opendev.org/#/c/744048/ where we should see the failure for no tempest case.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        fake_file \u003d tempfile.NamedTemporaryFile(\"w+t\", delete\u003dFalse)"},{"line_number":125,"context_line":"        import_1 \u003d \"from unittest import mock\\n\\n\""},{"line_number":126,"context_line":"        import_2 \u003d \"from tempest import u_fake_module\\n\""},{"line_number":127,"context_line":"        source_code \u003d import_1 + import_2"},{"line_number":128,"context_line":"        fake_file.write(source_code)"},{"line_number":129,"context_line":"        fake_file.close()"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_5bac979f","line":127,"range":{"start_line":127,"start_character":0,"end_line":127,"end_character":41},"updated":"2020-07-30 20:33:59.000000000","message":"ditto here to check the line number.\n\nit should be 2","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1482c836fb593767b66fcb353edd967a2172a5e2","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        expected_source \u003d patcher._quote(import_1) + \u0027{\u0027 + patch_id + \u0027:s}\u0027 + \\"},{"line_number":145,"context_line":"            patcher._quote(import_2)"},{"line_number":146,"context_line":"        self.assertEqual(expected_source,"},{"line_number":147,"context_line":"                         patcher.source_files[fake_file.name])"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_5bcdb7b6","line":147,"range":{"start_line":147,"start_character":0,"end_line":147,"end_character":62},"updated":"2020-07-30 20:33:59.000000000","message":"along with these two cases, let\u0027s cover the no import case also (no import and no tempest import)\n\nwe need to test this case with some license para text and assert line no is \u003e \u0027license para text\u0027 lines","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0948a2a1883e0e12f26c5ea0e766b00e6492d69f","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        expected_source \u003d patcher._quote(import_1) + \u0027{\u0027 + patch_id + \u0027:s}\u0027 + \\"},{"line_number":145,"context_line":"            patcher._quote(import_2)"},{"line_number":146,"context_line":"        self.assertEqual(expected_source,"},{"line_number":147,"context_line":"                         patcher.source_files[fake_file.name])"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_db86c313","line":147,"range":{"start_line":147,"start_character":0,"end_line":147,"end_character":62},"in_reply_to":"9f560f44_1002da3d","updated":"2020-08-02 17:43:21.000000000","message":"as i commented in code, let\u0027s check this case does not add any patch. we can mock the add_patch and assert on it is not called.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2281910a09f9d57ae73f326be3fb05a2cda74e85","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        expected_source \u003d patcher._quote(import_1) + \u0027{\u0027 + patch_id + \u0027:s}\u0027 + \\"},{"line_number":145,"context_line":"            patcher._quote(import_2)"},{"line_number":146,"context_line":"        self.assertEqual(expected_source,"},{"line_number":147,"context_line":"                         patcher.source_files[fake_file.name])"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_1002da3d","line":147,"range":{"start_line":147,"start_character":0,"end_line":147,"end_character":62},"in_reply_to":"9f560f44_5bcdb7b6","updated":"2020-08-02 11:29:06.000000000","message":"As I mentioned before, such case can not be.","commit_id":"1ced0575d231fd4ae2c07331dcbaa5880cee857b"}]}