)]}'
{"openstack_dashboard/test/integration_tests/helpers.py":[{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                return log.encode(\u0027utf-8\u0027)"},{"line_number":195,"context_line":"        return rec(_log)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def zoom_out(self, times\u003d3):"},{"line_number":198,"context_line":"        \"\"\"Zooming out prevents different elements being driven out of xvfb"},{"line_number":199,"context_line":"        viewport (which in Selenium\u003e\u003d2.50.1 prevents interaction with them."},{"line_number":200,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_26498ed1","side":"PARENT","line":197,"updated":"2016-05-24 12:53:27.000000000","message":"I understand that video doesn\u0027t look pretty with screen zoomed out, but this method was added for a reason. It ensures that all elements Selenium is going to click on are within the viewport, so clicks succeed.","commit_id":"0a0cafbd218b6dfc0ba6fd5bd57cf85dc7c71c79"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                return log.encode(\u0027utf-8\u0027)"},{"line_number":195,"context_line":"        return rec(_log)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def zoom_out(self, times\u003d3):"},{"line_number":198,"context_line":"        \"\"\"Zooming out prevents different elements being driven out of xvfb"},{"line_number":199,"context_line":"        viewport (which in Selenium\u003e\u003d2.50.1 prevents interaction with them."},{"line_number":200,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_9dd7e0be","side":"PARENT","line":197,"in_reply_to":"bab6814e_26498ed1","updated":"2016-05-24 15:58:15.000000000","message":"I\u0027m inclined to believe, this method was added as a fix of .maximize_window() problem above. If window is maximized with such huge resolution, all UI components must be available, isn\u0027t it?\nAnd I think, that zooming can lead to incorrect test results. For ex. with normal viewport some elements can overlap other, but after zooming out they can be available.\nI think, web UI need to test without resolution hacking.\nBtw, in this patch all tests are passed.","commit_id":"0a0cafbd218b6dfc0ba6fd5bd57cf85dc7c71c79"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                return log.encode(\u0027utf-8\u0027)"},{"line_number":195,"context_line":"        return rec(_log)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def zoom_out(self, times\u003d3):"},{"line_number":198,"context_line":"        \"\"\"Zooming out prevents different elements being driven out of xvfb"},{"line_number":199,"context_line":"        viewport (which in Selenium\u003e\u003d2.50.1 prevents interaction with them."},{"line_number":200,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_e8c0e27e","side":"PARENT","line":197,"in_reply_to":"bab6814e_9dd7e0be","updated":"2016-05-24 16:56:57.000000000","message":"Not exactly, maximize_window patch was made earlier (see https://bugs.launchpad.net/horizon/+bug/1338630), while zooming out was made to solve a more recent issue (https://bugs.launchpad.net/horizon/+bug/1541876). IIRC, just increasing viewport size didn\u0027t eliminate the intermittent failures, but zooming out did.\n\nTo understand if it really solves / not solves \u0027out of viewport click issue\u0027 we have to run tests w/o zoom_out on many commits. I suggest to comment out `zoom_out()` invocation (with a corresponding TODO comment), but do not delete method definition. If further test runs will be okay without it, we will delete it completely in 1 month or so.","commit_id":"0a0cafbd218b6dfc0ba6fd5bd57cf85dc7c71c79"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    if IS_SELENIUM_HEADLESS:"},{"line_number":78,"context_line":"        return (1920, 1080)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    if is_app_installed(\u0027xdpyinfo\u0027):"},{"line_number":81,"context_line":"        return subprocess.check_output("},{"line_number":82,"context_line":"            \u0027xdpyinfo | grep dimensions\u0027, shell\u003dTrue).split()[1].split(\u0027x\u0027)"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_68d7d223","line":80,"updated":"2016-05-24 16:56:57.000000000","message":"that\u0027s another call I\u0027d like to invoke module-wide, instead for each test (avoid multiple `subprocess` calls).","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        if self.CONFIG.selenium.maximize_browser:"},{"line_number":148,"context_line":"            self.driver.maximize_window()"},{"line_number":149,"context_line":"            if IS_SELENIUM_HEADLESS:  # force full screen in xvfb"},{"line_number":150,"context_line":"                self.driver.set_window_size(width, height)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        self.driver.implicitly_wait(self.CONFIG.selenium.implicit_wait)"},{"line_number":153,"context_line":"        self.driver.set_page_load_timeout("}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_e6e66615","line":150,"updated":"2016-05-24 12:53:27.000000000","message":"do we really need this, I mean are you sure that .maximize_window() doesn\u0027t work with xvfb?","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        if self.CONFIG.selenium.maximize_browser:"},{"line_number":148,"context_line":"            self.driver.maximize_window()"},{"line_number":149,"context_line":"            if IS_SELENIUM_HEADLESS:  # force full screen in xvfb"},{"line_number":150,"context_line":"                self.driver.set_window_size(width, height)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        self.driver.implicitly_wait(self.CONFIG.selenium.implicit_wait)"},{"line_number":153,"context_line":"        self.driver.set_page_load_timeout("}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_289caa68","line":150,"in_reply_to":"bab6814e_9dc0807e","updated":"2016-05-24 16:56:57.000000000","message":"interesting! hadn\u0027t realized that until saw the video.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        if self.CONFIG.selenium.maximize_browser:"},{"line_number":148,"context_line":"            self.driver.maximize_window()"},{"line_number":149,"context_line":"            if IS_SELENIUM_HEADLESS:  # force full screen in xvfb"},{"line_number":150,"context_line":"                self.driver.set_window_size(width, height)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        self.driver.implicitly_wait(self.CONFIG.selenium.implicit_wait)"},{"line_number":153,"context_line":"        self.driver.set_page_load_timeout("}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_9dc0807e","line":150,"in_reply_to":"bab6814e_e6e66615","updated":"2016-05-24 15:58:15.000000000","message":"Yep, I think so, because in my pc (ubuntu 14.04) maximize_window() doesn\u0027t work properly (may be it happens sometimes, depending from pc configuration, hardware or else). It looks that https://youtu.be/npZiRfusRUs.\n\nAnd some debugger info from my launch:\n\n    ipdb\u003e self.driver.maximize_window()\n    ipdb\u003e self.driver.get_window_size()\n    {u\u0027width\u0027: 1280, u\u0027height\u0027: 972}\n    ipdb\u003e self.driver.set_window_size(width, height)\n    ipdb\u003e self.driver.get_window_size()\n    {u\u0027width\u0027: 1920, u\u0027height\u0027: 1080}     \n    ipdb\u003e self.vdisplay.width\n    1920\n    ipdb\u003e self.vdisplay.height\n    1080","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                    self.video_recorder.file_path))"},{"line_number":203,"context_line":"                return"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"            copyfile(self.video_recorder.file_path,"},{"line_number":206,"context_line":"                     os.path.join(self._test_report_dir, \u0027video.mp4\u0027))"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    def _attach_browser_log(self, exc_info):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_885dd6ee","line":205,"updated":"2016-05-24 16:56:57.000000000","message":"moving file could be a bit cheaper in terms of file io.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        if os.environ.get(\u0027INTEGRATION_TESTS\u0027, False):"},{"line_number":251,"context_line":"            self.driver.quit()"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        self.video_recorder.stop()"},{"line_number":254,"context_line":"        self.video_recorder.clear()"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"        if IS_SELENIUM_HEADLESS:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_06e28a9f","line":253,"updated":"2016-05-24 12:53:27.000000000","message":"What do you think about stopping the video capture not when the error occurs, but until test logs out (or the most outer tearDown is executed)? Sometimes it might be insightful to see what happens after the error occurs.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        if os.environ.get(\u0027INTEGRATION_TESTS\u0027, False):"},{"line_number":251,"context_line":"            self.driver.quit()"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        self.video_recorder.stop()"},{"line_number":254,"context_line":"        self.video_recorder.clear()"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"        if IS_SELENIUM_HEADLESS:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_1dc010e0","line":253,"in_reply_to":"bab6814e_06e28a9f","updated":"2016-05-24 15:58:15.000000000","message":"Agree, that\u0027s a good point.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"f7b73134e634ff4987038e54966d24b27b1ec9e5","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        self._configure_log()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        self.addOnException("},{"line_number":105,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_test_log\u0027, True))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        def cleanup():"},{"line_number":108,"context_line":"            if getattr(self, \u0027_need_attach_test_log\u0027, None):"},{"line_number":109,"context_line":"                self._attach_test_log()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.addCleanup(cleanup)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        width, height \u003d SCREEN_SIZE"},{"line_number":114,"context_line":"        display \u003d \u00270.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_c7c37437","line":111,"range":{"start_line":104,"start_character":0,"end_line":111,"end_character":32},"updated":"2016-06-02 13:05:21.000000000","message":"I see a pattern here and in 2 places below. Consider rewriting this as a context manager:\n\n(as a method of this class)\n\n  @contextlib.contextmanager\n  def delayed_exception_handler(self):\n      flag \u003d uuid.uuid4()\n      self.addOnException(lambda exc_info: setattr(self, flag, True))\n\n      def cleanup():\n          yield flag\n\n      self.addCleanup(cleanup)\n\nThen call it as\n\n  with self.delayed_exception_hanlder() as flag:\n    if getattr(self, flag, False):\n        self._attach_test_log()","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"37e1435d4c0b921968a5bec1d3c6900b853e600e","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        self._configure_log()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        self.addOnException("},{"line_number":105,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_test_log\u0027, True))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        def cleanup():"},{"line_number":108,"context_line":"            if getattr(self, \u0027_need_attach_test_log\u0027, None):"},{"line_number":109,"context_line":"                self._attach_test_log()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.addCleanup(cleanup)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        width, height \u003d SCREEN_SIZE"},{"line_number":114,"context_line":"        display \u003d \u00270.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_d03a0898","line":111,"range":{"start_line":104,"start_character":0,"end_line":111,"end_character":32},"in_reply_to":"9abb7d3a_95735215","updated":"2016-06-02 13:43:27.000000000","message":"but self.driver.quit() should be executed always not only on exception","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"deddeeca6d5bda47d568c26d96498b4ca5971ca5","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        self._configure_log()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        self.addOnException("},{"line_number":105,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_test_log\u0027, True))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        def cleanup():"},{"line_number":108,"context_line":"            if getattr(self, \u0027_need_attach_test_log\u0027, None):"},{"line_number":109,"context_line":"                self._attach_test_log()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.addCleanup(cleanup)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        width, height \u003d SCREEN_SIZE"},{"line_number":114,"context_line":"        display \u003d \u00270.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_d5ecba2b","line":111,"range":{"start_line":104,"start_character":0,"end_line":111,"end_character":32},"in_reply_to":"9abb7d3a_c7c37437","updated":"2016-06-02 13:27:30.000000000","message":"how can you suggest to present such chunk?\n\n        def cleanup():\n            if getattr(self, \u0027_need_attach_browser_log\u0027, None):\n                self._attach_browser_log()\n            self.driver.quit()\n\n        self.addCleanup(cleanup)\n\nlike that?\n    with self.delayed_exception_hanlder() as flag:\n        if getattr(self, \u0027_need_attach_browser_log\u0027, None):\n            self._attach_browser_log()\n\n    self.addCleanup(self.driver.quit)\n\nwell, finally it will be presented as:\n\n    self.addCleanup(self._attach_browser_log)\n    self.addCleanup(self.driver.quit)\n\nI suppose, it\u0027s harder to remember and understand for new developer, why two independent code chunks has such order, rather than when these chunks are under one function.\nAnd, seems it\u0027s better to have only one cleanup for one entity or code unit.","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"a736a24ff08bc2e7c43a75ed9a9fc9b4df086f69","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        self._configure_log()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        self.addOnException("},{"line_number":105,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_test_log\u0027, True))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        def cleanup():"},{"line_number":108,"context_line":"            if getattr(self, \u0027_need_attach_test_log\u0027, None):"},{"line_number":109,"context_line":"                self._attach_test_log()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.addCleanup(cleanup)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        width, height \u003d SCREEN_SIZE"},{"line_number":114,"context_line":"        display \u003d \u00270.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_95735215","line":111,"range":{"start_line":104,"start_character":0,"end_line":111,"end_character":32},"in_reply_to":"9abb7d3a_d5ecba2b","updated":"2016-06-02 13:37:07.000000000","message":"Not exactly like that, with the help of a new context manager it would be written like\n\n  with self.delayed_exception_handler() as flag:\n    if getattr(self, flag, False):\n      self._attach_browser_log()\n    self.driver.quit()\n\nAnyways, it\u0027s just a suggestion from my side, not a requirement.","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"a736a24ff08bc2e7c43a75ed9a9fc9b4df086f69","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        self.video_recorder \u003d VideoRecorder(width, height, display\u003ddisplay)"},{"line_number":139,"context_line":"        self.video_recorder.start()"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        self.addOnException("},{"line_number":142,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_video\u0027, True))"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        def cleanup():"},{"line_number":145,"context_line":"            self.video_recorder.stop()"},{"line_number":146,"context_line":"            if getattr(self, \u0027_need_attach_video\u0027, None):"},{"line_number":147,"context_line":"                self._attach_video()"},{"line_number":148,"context_line":"            else:"},{"line_number":149,"context_line":"                self.video_recorder.clear()"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        self.addCleanup(cleanup)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        # Increase the default Python socket timeout from nothing"},{"line_number":154,"context_line":"        # to something that will cope with slow webdriver startup times."}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_f561fe12","line":151,"range":{"start_line":141,"start_character":0,"end_line":151,"end_character":32},"updated":"2016-06-02 13:37:07.000000000","message":"And this one would look like\n\n  with self.delayed_exception_handler() as flag:\n    self.video_recorder_stop()\n    if getattr(self, flag, False):\n      self._attach_video()\n    else:\n      self.video_recorder.clear()","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"37d23fd282a9a4c68e2e1492e058c92de13d6373","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        self.video_recorder \u003d VideoRecorder(width, height, display\u003ddisplay)"},{"line_number":139,"context_line":"        self.video_recorder.start()"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        self.addOnException("},{"line_number":142,"context_line":"            lambda exc_info: setattr(self, \u0027_need_attach_video\u0027, True))"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        def cleanup():"},{"line_number":145,"context_line":"            self.video_recorder.stop()"},{"line_number":146,"context_line":"            if getattr(self, \u0027_need_attach_video\u0027, None):"},{"line_number":147,"context_line":"                self._attach_video()"},{"line_number":148,"context_line":"            else:"},{"line_number":149,"context_line":"                self.video_recorder.clear()"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        self.addCleanup(cleanup)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        # Increase the default Python socket timeout from nothing"},{"line_number":154,"context_line":"        # to something that will cope with slow webdriver startup times."}],"source_content_type":"text/x-python","patch_set":19,"id":"9abb7d3a_701fdc09","line":151,"range":{"start_line":141,"start_character":0,"end_line":151,"end_character":32},"in_reply_to":"9abb7d3a_f561fe12","updated":"2016-06-02 13:45:00.000000000","message":"delayed_exception_handler - it looks, like it will be executed only on exception, but actually it will be executed always as cleanup, isn\u0027t it? I have another suggestion, let me a minute","commit_id":"76e53b40d99268024ef3fe339e414035ef8f0659"}],"openstack_dashboard/test/integration_tests/video_recorder.py":[{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# TODO(schipiga): later should create module with common functions"},{"line_number":26,"context_line":"# (for ex. utils.py) and move this function there."},{"line_number":27,"context_line":"def is_app_installed(app_name):"},{"line_number":28,"context_line":"    # subprocess returns 0 if app is installed and greater 0 if no"},{"line_number":29,"context_line":"    return not subprocess.call(\u0027which {} \u003e /dev/null 2\u003e\u00261\u0027.format(app_name),"},{"line_number":30,"context_line":"                               shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_c1922c9f","line":27,"updated":"2016-05-24 12:53:27.000000000","message":"There may be an easier way to detect AVCONV presense: run `which avconv` in pre_test_hook and assign $? to AVCONV_INSTALLED environment variable being exported and use it from python. That should be cheaper than forking shell process at the beginning of each test.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# TODO(schipiga): later should create module with common functions"},{"line_number":26,"context_line":"# (for ex. utils.py) and move this function there."},{"line_number":27,"context_line":"def is_app_installed(app_name):"},{"line_number":28,"context_line":"    # subprocess returns 0 if app is installed and greater 0 if no"},{"line_number":29,"context_line":"    return not subprocess.call(\u0027which {} \u003e /dev/null 2\u003e\u00261\u0027.format(app_name),"},{"line_number":30,"context_line":"                               shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_734ea497","line":27,"in_reply_to":"bab6814e_be457b18","updated":"2016-05-24 16:56:57.000000000","message":"What do you think of SELENIUM_HEADLESS and INTEGRATION_TESTS environment variables then :)?\n\nUsing environment variables is a part of Unix-philosophy, where you can pipe together a bunch of one-task scripts, possibly modifying their behavior with these variables. Treat it as a dependency injection, where by altering input data to test script one can choose between different video codecs and write the related logic in shell, which is better suited for these package manager related tasks.\n\nCertainly, POV over architectural issues is hugely influenced by overall background and things like that.\n\nThe bottomline: I would prefer to have codec being defined by a shell script, but in case we can avoid performance overhead with module-level invocation of `subprocess.call()`, I\u0027m fine with it.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# TODO(schipiga): later should create module with common functions"},{"line_number":26,"context_line":"# (for ex. utils.py) and move this function there."},{"line_number":27,"context_line":"def is_app_installed(app_name):"},{"line_number":28,"context_line":"    # subprocess returns 0 if app is installed and greater 0 if no"},{"line_number":29,"context_line":"    return not subprocess.call(\u0027which {} \u003e /dev/null 2\u003e\u00261\u0027.format(app_name),"},{"line_number":30,"context_line":"                               shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_be457b18","line":27,"in_reply_to":"bab6814e_c1922c9f","updated":"2016-05-24 15:58:15.000000000","message":"Yep, it\u0027s possible, but I think, architecturally it\u0027s bad. You suggest to make additional linking and unclear dependency from environment. But my view point, that video_recorder.py should be considered as independent linux-specific module to record video from screen. VideoRecord knows nothing about tests and env, which inside it\u0027s used, it just does his work to record video. And it doesn\u0027t know that somewhere above there is pre_hook, which installed libav-tools.\n\nThis VideoRecorder can use avconv or ffmpeg (as I know in ubuntu above v14.04 ffmpeg is included to default repos again, and may be they decide to remove avconv, who knows. And may be this module will be used not only with ubuntu, but with linux with ffmpeg and without avconv).\n\nTalking about performance, as alternate, I may suggest to define recording application during module calling:\n\n    def is_app_installed(app_name):\n        # subprocess returns 0 if app is installed and greater 0 if no\n        return not subprocess.call(\u0027which {} \u003e /dev/null 2\u003e\u00261\u0027.format(app_name),\n                                   shell\u003dTrue)\n\n\n    def define_recording_app():\n        if not is_app_installed(AVCONV):\n            LOGGER.warn(\"{} isn\u0027t installed. Trying to use {}.\".format(AVCONV,\n                                                                       FFMPEG))\n            if not is_app_installed(FFMPEG):\n                LOGGER.error(\"{} isn\u0027t installed. \"\n                             \"Video recording is skipped.\".format(FFMPEG))\n            else:\n                return FFMPEG\n        else:\n            return AVCONV\n\n\n    VideoRecorder.RECORDING_APP \u003d define_recording_app()","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                               shell\u003dTrue)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def need_recording_app(func):"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @wraps(func)"},{"line_number":36,"context_line":"    def wrapper(self, *args, **kwgs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_a1e608e6","line":33,"updated":"2016-05-24 12:53:27.000000000","message":"If this and below decorators aren\u0027t meant to be invoked outside of this module, it\u0027s better to add leading \u0027_\u0027 to their names. Also please see my below comments about decorators necessity. IMO it would be simpler to just implement their logic as VideoRecorder methods.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                               shell\u003dTrue)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def need_recording_app(func):"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @wraps(func)"},{"line_number":36,"context_line":"    def wrapper(self, *args, **kwgs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_b2ce4ab2","line":33,"in_reply_to":"bab6814e_a1e608e6","updated":"2016-05-24 15:58:15.000000000","message":"Well, yes about \u0027_\u0027, but decorators with \u0027_\u0027 are looked a bit ugly, I suppose :)\n\nTalking about decorators usage, I commented my view point about elegant code with decorators. And more over, I would prefer such architectural approach:\n\nhttp://paste.openstack.org/show/498687/\n\nI\u0027m inclined to believe, that strong architecture prevents bugs producing and provides code scalability.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                               shell\u003dTrue)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def need_recording_app(func):"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @wraps(func)"},{"line_number":36,"context_line":"    def wrapper(self, *args, **kwgs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_8525bfaf","line":33,"in_reply_to":"bab6814e_b2ce4ab2","updated":"2016-05-24 16:56:57.000000000","message":"Read the paste, let\u0027s keep it the existing way. I\u0027m already got a number of complaints that integration tests are overengineered, let\u0027s not make them even more overengineered :).","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        if not is_app_installed(AVCONV):"},{"line_number":38,"context_line":"            LOGGER.warn(\"{} isn\u0027t installed. Trying to use {}.\".format(AVCONV,"},{"line_number":39,"context_line":"                                                                       FFMPEG))"},{"line_number":40,"context_line":"            if not is_app_installed(FFMPEG):"},{"line_number":41,"context_line":"                LOGGER.error(\"{} isn\u0027t installed. \""},{"line_number":42,"context_line":"                             \"Video recording is skipped.\".format(FFMPEG))"},{"line_number":43,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_86167a53","line":40,"updated":"2016-05-24 12:53:27.000000000","message":"Do we really need to fall back to ffmpeg if we don\u0027t install it as a tests prerequisite? That seems dubious to me.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        if not is_app_installed(AVCONV):"},{"line_number":38,"context_line":"            LOGGER.warn(\"{} isn\u0027t installed. Trying to use {}.\".format(AVCONV,"},{"line_number":39,"context_line":"                                                                       FFMPEG))"},{"line_number":40,"context_line":"            if not is_app_installed(FFMPEG):"},{"line_number":41,"context_line":"                LOGGER.error(\"{} isn\u0027t installed. \""},{"line_number":42,"context_line":"                             \"Video recording is skipped.\".format(FFMPEG))"},{"line_number":43,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_fe78a32d","line":40,"in_reply_to":"bab6814e_86167a53","updated":"2016-05-24 15:58:15.000000000","message":"I would consider video_recorder.py as tests independent module (or project), which should know nothing about env configuration (as I talked about)","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        if not is_app_installed(AVCONV):"},{"line_number":38,"context_line":"            LOGGER.warn(\"{} isn\u0027t installed. Trying to use {}.\".format(AVCONV,"},{"line_number":39,"context_line":"                                                                       FFMPEG))"},{"line_number":40,"context_line":"            if not is_app_installed(FFMPEG):"},{"line_number":41,"context_line":"                LOGGER.error(\"{} isn\u0027t installed. \""},{"line_number":42,"context_line":"                             \"Video recording is skipped.\".format(FFMPEG))"},{"line_number":43,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_05954f3f","line":40,"in_reply_to":"bab6814e_fe78a32d","updated":"2016-05-24 16:56:57.000000000","message":"In case you\u0027re going to make it even more pluggable and work with list of codecs sorted in order of preference, try this:\n\n  codecs \u003d (AVCONV, FFMPEG)\n  for i, codec in enumerate(codecs):\n    if is_app_installed(codec):\n      self.RECORDING_APP \u003d codec\n      break\n    elif i \u003c len(codecs) - 1:\n      LOGGER.warn(\"{} isn\u0027t installed. Trying to use {}.\".format(codec, codecs[i]))\n    else:\n      LOGGER.error(\"{} isn\u0027t installed. Video recording is skipped\".format(codec))\n      return\n  return func(self, *args, **kwargs)\n\n;)","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    return wrapper"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"def if_not_launched(func):"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @wraps(func)"},{"line_number":69,"context_line":"    def wrapper(self, *args, **kwgs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_33feeb05","line":66,"updated":"2016-05-24 12:53:27.000000000","message":"@if_not_launched and @if_launched could be reduced to just one VideoRecorder method ._check_launched(target_state)","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    return wrapper"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"def if_not_launched(func):"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @wraps(func)"},{"line_number":69,"context_line":"    def wrapper(self, *args, **kwgs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_c4451018","line":66,"in_reply_to":"bab6814e_33feeb05","updated":"2016-05-24 15:58:15.000000000","message":"I commented above about architecture and below about elegant code","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                                \u0027-codec\u0027, \u0027libx264\u0027, self.file_path]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @if_not_launched"},{"line_number":107,"context_line":"    @need_recording_app"},{"line_number":108,"context_line":"    def start(self):"},{"line_number":109,"context_line":"        fnull \u003d open(os.devnull, \u0027w\u0027)"},{"line_number":110,"context_line":"        LOGGER.info(\u0027Record video via {!r}\u0027.format(\u0027 \u0027.join(self._cmd)))"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_41c0fce0","line":107,"updated":"2016-05-24 12:53:27.000000000","message":"@need_recording_app is used only one time. Is it worth to define it as a decorator, or better to define it as VideoRecorder method?","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                                \u0027-codec\u0027, \u0027libx264\u0027, self.file_path]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @if_not_launched"},{"line_number":107,"context_line":"    @need_recording_app"},{"line_number":108,"context_line":"    def start(self):"},{"line_number":109,"context_line":"        fnull \u003d open(os.devnull, \u0027w\u0027)"},{"line_number":110,"context_line":"        LOGGER.info(\u0027Record video via {!r}\u0027.format(\u0027 \u0027.join(self._cmd)))"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_21b04e8b","line":107,"in_reply_to":"bab6814e_41c0fce0","updated":"2016-05-24 15:58:15.000000000","message":"I seem, decorators provide more readable and elegant code, allowing to describe a method only the code that is needed directly. Here they are used to provide the rules of method calls.\nAnd I consider them as functions over class, which tune it\u0027s behavior. Like a small child with adult mentors :)","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self.is_launched \u003d True"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    @if_launched"},{"line_number":115,"context_line":"    @if_recording_app"},{"line_number":116,"context_line":"    def stop(self):"},{"line_number":117,"context_line":"        self._popen.send_signal(signal.SIGINT)"},{"line_number":118,"context_line":"        self._popen.communicate()"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_817144e3","line":115,"updated":"2016-05-24 12:53:27.000000000","message":"Same is true for @if_recording_app","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self.is_launched \u003d True"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    @if_launched"},{"line_number":115,"context_line":"    @if_recording_app"},{"line_number":116,"context_line":"    def stop(self):"},{"line_number":117,"context_line":"        self._popen.send_signal(signal.SIGINT)"},{"line_number":118,"context_line":"        self._popen.communicate()"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_c4ceb0b3","line":115,"in_reply_to":"bab6814e_817144e3","updated":"2016-05-24 15:58:15.000000000","message":"I commented above","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0c7b32bbaf36e708b455278622edf79bf4828645","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    @if_recording_app"},{"line_number":116,"context_line":"    def stop(self):"},{"line_number":117,"context_line":"        self._popen.send_signal(signal.SIGINT)"},{"line_number":118,"context_line":"        self._popen.communicate()"},{"line_number":119,"context_line":"        self.is_launched \u003d False"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    @if_not_launched"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_6111c0af","line":118,"updated":"2016-05-24 12:53:27.000000000","message":"What .communicate() is intended to do? If send_signal() succeeds, but .communicate() fails, is_launched is going to True, but that doesn\u0027t seem like a proper thing.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"d39086e6896a37b201fdb48e1e2a5b17a205069d","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    @if_recording_app"},{"line_number":116,"context_line":"    def stop(self):"},{"line_number":117,"context_line":"        self._popen.send_signal(signal.SIGINT)"},{"line_number":118,"context_line":"        self._popen.communicate()"},{"line_number":119,"context_line":"        self.is_launched \u003d False"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    @if_not_launched"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_a1043ec6","line":118,"in_reply_to":"bab6814e_6111c0af","updated":"2016-05-24 15:58:15.000000000","message":".communicates() waits for process to terminate (it\u0027s said in documentation). I suppose, process can\u0027t be finished right after signal sending. And .communicate() is used without try...except wrapper in subprocess.py, for ex.:\n\n    def check_output(*popenargs, **kwargs):\n        if \u0027stdout\u0027 in kwargs:\n            raise ValueError(\u0027stdout argument not allowed, it will be overridden.\u0027)\n        process \u003d Popen(stdout\u003dPIPE, *popenargs, **kwargs)\n        output, unused_err \u003d process.communicate()\n        retcode \u003d process.poll()\n        if retcode:\n            cmd \u003d kwargs.get(\"args\")\n            if cmd is None:\n                cmd \u003d popenargs[0]\n            raise CalledProcessError(retcode, cmd, output\u003doutput)\n        return output\n\nSo, I\u0027m not sure, that we should afraid .communicate() exception.","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    @if_recording_app"},{"line_number":116,"context_line":"    def stop(self):"},{"line_number":117,"context_line":"        self._popen.send_signal(signal.SIGINT)"},{"line_number":118,"context_line":"        self._popen.communicate()"},{"line_number":119,"context_line":"        self.is_launched \u003d False"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    @if_not_launched"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_6516dba8","line":118,"in_reply_to":"bab6814e_a1043ec6","updated":"2016-05-24 16:56:57.000000000","message":"Okay, sounds reasonable that nothing bad should happen between send_signal() and communicate(). Another thing to think about: is it possible that after sending a signal communicate would take to long? Should we have some kind of a watchdog, which waits, say 5 minutes and then sends signal.SIGKILL ?","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"0e1f222845e33572bf0e9d152153a51c373bc6e0","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            LOGGER.warn(\"{!r} is absent already\".format(self.file_path))"},{"line_number":125,"context_line":"            return"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        os.remove(self.file_path)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @property"},{"line_number":130,"context_line":"    def _cmd(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_48f2ce9b","line":127,"updated":"2016-05-24 16:56:57.000000000","message":"Possible (and more pythonic) alternative to that would be:\n\n  try:\n    os.remove(self.file_path)\n  except OSError:\n    LOGGER.warn(...)","commit_id":"22e3c7949fda17df8c5b1f735deb3c2586c04f53"}],"tools/gate/integration/post_test_hook.sh":[{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"4f3f31fa6ef497f5bf7e3de5e472b1c4cebe075f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"# install avconv to capture video of failed tests"},{"line_number":8,"context_line":"sudo apt-get install -y libav-tools"},{"line_number":9,"context_line":"export AVCONV_INSTALLED\u003d1"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"cd /opt/stack/new/horizon"},{"line_number":12,"context_line":"sudo -H -u stack tox -e py27integration"}],"source_content_type":"text/x-sh","patch_set":16,"id":"9abb7d3a_06b49532","line":9,"updated":"2016-06-01 16:00:41.000000000","message":"It doesn\u0027t work too, because tox runs tests in another shell environment","commit_id":"c04c7317589b0267b047ebe99494319a4b209f49"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"1b868d9bd31c8c908887d6336637809cf0423d2f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"# install avconv to capture video of failed tests"},{"line_number":8,"context_line":"sudo apt-get install -y libav-tools"},{"line_number":9,"context_line":"export AVCONV_INSTALLED\u003d1"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"cd /opt/stack/new/horizon"},{"line_number":12,"context_line":"sudo -H -u stack tox -e py27integration"}],"source_content_type":"text/x-sh","patch_set":16,"id":"9abb7d3a_dcac158e","line":9,"in_reply_to":"9abb7d3a_06b49532","updated":"2016-06-01 18:10:25.000000000","message":"It\u0027s not another shell issue, it\u0027s a sudo issue, specifically that it doesn\u0027t preserve environment variables without -E argument. Consider this:\n\n  tsufiev@tsufiev-pc:~/develop/horizon$ export SAMPLE\u003d10\n  tsufiev@tsufiev-pc:~/develop/horizon$ echo $SAMPLE\n  \u003e\u003e 10\n  tsufiev@tsufiev-pc:~/develop/horizon$ bash -c \u0027echo $SAMPLE\u0027\n  \u003e\u003e 10\n  tsufiev@tsufiev-pc:~/develop/horizon$ sudo bash -c \u0027echo $SAMPLE\u0027\n  \u003e\u003e\n  tsufiev@tsufiev-pc:~/develop/horizon$ sudo -E bash -c \u0027echo $SAMPLE\u0027\n  \u003e\u003e 10","commit_id":"c04c7317589b0267b047ebe99494319a4b209f49"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"5d6b6f6b510583f7864426d322fba8d22c375d46","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"# install avconv to capture video of failed tests"},{"line_number":8,"context_line":"sudo apt-get install -y libav-tools"},{"line_number":9,"context_line":"export AVCONV_INSTALLED\u003d1"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"cd /opt/stack/new/horizon"},{"line_number":12,"context_line":"sudo -H -E -u stack tox -e py27integration"}],"source_content_type":"text/x-sh","patch_set":17,"id":"9abb7d3a_a2d59543","line":9,"updated":"2016-06-02 05:57:35.000000000","message":"that\u0027s doesn\u0027t work again with sudo -E too. Seems the reason is deeper. Need to investigate","commit_id":"a488133ed3473d6352c25ad3ef4467c51430c09e"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"bb2eba2a5dab5ef341590b78042fa5db3acf7d66","unresolved":false,"context_lines":[{"line_number":5,"context_line":"set -x"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"# install avconv to capture video of failed tests"},{"line_number":8,"context_line":"sudo apt-get install -y libav-tools"},{"line_number":9,"context_line":"export AVCONV_INSTALLED\u003d1"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"cd /opt/stack/new/horizon"},{"line_number":12,"context_line":"sudo -H -E -u stack tox -e py27integration"}],"source_content_type":"text/x-sh","patch_set":18,"id":"9abb7d3a_9d2edea3","line":9,"range":{"start_line":8,"start_character":0,"end_line":9,"end_character":25},"updated":"2016-06-02 12:15:58.000000000","message":"One more thing: it\u0027s better to join these 2 lines with \u0026\u0026 so the variable wouldn\u0027t be set unless the previous operation succeeds. \n\nIt doesn\u0027t make sense for install operation to fail and then still set environment variable as if it succeeded.","commit_id":"0d89c6ab0b79bc232c0b93f22de89e0f7e8affdd"}],"tools/gate/integration/pre_test_hook.sh":[{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"9467a2bc6e06d24c4d0b52d8a7865e07976f8526","unresolved":false,"context_lines":[{"line_number":8,"context_line":"mv _20_integration_tests_scaffolds.py.example _20_integration_tests_scaffolds.py"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"# install avconv to capture video of failed tests"},{"line_number":11,"context_line":"sudo apt-get install -y libav-tools"}],"source_content_type":"text/x-sh","patch_set":14,"id":"9abb7d3a_4f6b72f1","line":11,"updated":"2016-06-01 07:40:17.000000000","message":"Not possible to export environment variable about avconv installation in pre_test_hook, because pre_test_hook is launched in another shell console:\n\n    timeout -s 9 79m bash -c \u0027source /home/jenkins/workspace/gate-horizon-dsvm-integration/devstack-gate/functions.sh \u0026\u0026 call_hook_if_defined pre_test_hook\u0027\n\n    schipiga@schipiga ~ (git::stable/mitaka) $ bash -c \"export AVCONV\u003d1\"\n    schipiga@schipiga ~ (git::stable/mitaka) $ echo $AVCONV","commit_id":"d48664aabc8658cd5e995a36f0fa71a5299379f3"},{"author":{"_account_id":20772,"name":"Sergei Chipiga","email":"schipiga@mirantis.com","username":"schipiga"},"change_message_id":"8946c2d253bf85bf7c15713ef30f005f767fd3b9","unresolved":false,"context_lines":[{"line_number":8,"context_line":"mv _20_integration_tests_scaffolds.py.example _20_integration_tests_scaffolds.py"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"# install avconv to capture video of failed tests"},{"line_number":11,"context_line":"sudo apt-get install -y libav-tools"}],"source_content_type":"text/x-sh","patch_set":15,"id":"9abb7d3a_2f154e48","line":11,"updated":"2016-06-01 08:56:31.000000000","message":"Not possible to export environment variable about avconv installation in pre_test_hook, because pre_test_hook is launched in another shell console:\n    timeout -s 9 79m bash -c \u0027source /home/jenkins/workspace/gate-horizon-dsvm-integration/devstack-gate/functions.sh \u0026\u0026 call_hook_if_defined pre_test_hook\u0027\n    schipiga@schipiga ~ (git::stable/mitaka) $ bash -c \"export AVCONV\u003d1\"\n    schipiga@schipiga ~ (git::stable/mitaka) $ echo $AVCONV","commit_id":"09aad2eb6ec4cd5fd51783d34942b221e33a97ff"},{"author":{"_account_id":8040,"name":"Timur Sufiev","email":"tsufiev@gmail.com","username":"tsufiev"},"change_message_id":"f62836fcc8f0908d0fac9a5e7deaef6fc05752d9","unresolved":false,"context_lines":[{"line_number":8,"context_line":"mv _20_integration_tests_scaffolds.py.example _20_integration_tests_scaffolds.py"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"# install avconv to capture video of failed tests"},{"line_number":11,"context_line":"sudo apt-get install -y libav-tools"}],"source_content_type":"text/x-sh","patch_set":15,"id":"9abb7d3a_da66f9eb","line":11,"in_reply_to":"9abb7d3a_2f154e48","updated":"2016-06-01 09:19:38.000000000","message":"What\u0027s about the post_test_hook then?","commit_id":"09aad2eb6ec4cd5fd51783d34942b221e33a97ff"}]}