)]}'
{"tempest/tests/cmd/test_workspace.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a1e105ee17814ee06f5aad2e9a96e50896fc41eb","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import subprocess"},{"line_number":18,"context_line":"import tempfile"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from mock import patch"},{"line_number":21,"context_line":"try:"},{"line_number":22,"context_line":"    from StringIO import StringIO"},{"line_number":23,"context_line":"except ImportError:"},{"line_number":24,"context_line":"    from io import StringIO"},{"line_number":25,"context_line":"from tempest.cmd import workspace"},{"line_number":26,"context_line":"from tempest.lib.common.utils import data_utils"},{"line_number":27,"context_line":"from tempest.tests import base"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_b0b84a11","line":24,"range":{"start_line":20,"start_character":0,"end_line":24,"end_character":27},"updated":"2018-06-19 22:49:42.000000000","message":"Two changes required:\n\n1) Add a newline after this because we should separate third-party library imports from tempest imports.\n2) Add \n\n    try:\n        from StringIO import StringIO\n    except ImportError:\n        from io import StringIO\n\nto L16 (after shutil) because these are Python libraries.\n\nOrder of imports should be:\n\n\u003cpython libraries\u003e\n\n\u003cthird-party libraries\u003e\n\n\u003ctempest modules\u003e\n\nMore info: https://docs.openstack.org/hacking/latest/user/hacking.html#import-order-template","commit_id":"0865abc9dd5eb208f9690f6ee0832765f67fd24b"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"9d03a70b58a023ac9d1f6560148f8f9355e03c52","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        self.workspace_manager.register_new_workspace(name, path)"},{"line_number":148,"context_line":"        self.assertIsNotNone(self.workspace_manager.get_workspace(name))"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"},{"line_number":151,"context_line":"    def test_workspace_name_not_exists(self, mock_stdout):"},{"line_number":152,"context_line":"        nonexistent_name \u003d data_utils.rand_name()"},{"line_number":153,"context_line":"        ex \u003d self.assertRaises(SystemExit,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_25ffbf34","line":150,"updated":"2018-06-25 08:35:28.000000000","message":"heh, I think we should change the message to stderr instead of stdout because it\u0027s a warning message. But so far, this is legtimate.","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":26198,"name":"Anna Pankiewicz","email":"anna.e.pankiewicz@att.com","username":"apankiewicz"},"change_message_id":"2cf58f8256f8bff200a34fddd9250ed0179ed7ef","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        self.workspace_manager.register_new_workspace(name, path)"},{"line_number":148,"context_line":"        self.assertIsNotNone(self.workspace_manager.get_workspace(name))"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"},{"line_number":151,"context_line":"    def test_workspace_name_not_exists(self, mock_stdout):"},{"line_number":152,"context_line":"        nonexistent_name \u003d data_utils.rand_name()"},{"line_number":153,"context_line":"        ex \u003d self.assertRaises(SystemExit,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_00314359","line":150,"in_reply_to":"5f7c97a3_25ffbf34","updated":"2018-06-25 19:02:57.000000000","message":"I went ahead and addressed the other comments with this patch set since it initially looks like I\u0027d need to adjust more than just the test to use stderr, but I\u0027ll look more at it in the next patch set.","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"0434f1185c1fca2d23ee32a853e0245359094987","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                               self.workspace_manager._name_exists,"},{"line_number":155,"context_line":"                               nonexistent_name)"},{"line_number":156,"context_line":"        self.assertEqual(1, ex.code)"},{"line_number":157,"context_line":"        self.assertRegex(mock_stdout.getvalue(),"},{"line_number":158,"context_line":"                         \"^A workspace was not found with name: .*$\")"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_cc086a2e","line":157,"range":{"start_line":157,"start_character":19,"end_line":157,"end_character":24},"updated":"2018-06-25 16:16:29.000000000","message":"you can use assertEqual here\n\nPdb) mock_stdout.getvalue()\n\u0027A workspace already exists with name: 8c7958ef-f51a-4277-9141-7cb6ea96e90d.\\n\u0027\n\nby changing this assert to the following:\n\nself.assertEqual(\n    mock_stdout.getvalue(),\n    \"A workspace was not found with name: %s\\n\" % self.name)","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"0434f1185c1fca2d23ee32a853e0245359094987","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                               self.workspace_manager._workspace_name_exists,"},{"line_number":165,"context_line":"                               duplicate_name)"},{"line_number":166,"context_line":"        self.assertEqual(1, ex.code)"},{"line_number":167,"context_line":"        self.assertRegex(mock_stdout.getvalue(),"},{"line_number":168,"context_line":"                         \"^A workspace already exists with name: .*$\")"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_8cfef203","line":167,"range":{"start_line":167,"start_character":19,"end_line":167,"end_character":24},"updated":"2018-06-25 16:16:29.000000000","message":"similarly\n\nyou can use assertEqual here\n\nself.assertEqual(\n    mock_stdout.getvalue(),\n    \"A workspace was not found with name: %s\\n\" % duplicate_name)","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"0434f1185c1fca2d23ee32a853e0245359094987","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self.assertRegex(mock_stdout.getvalue(),"},{"line_number":168,"context_line":"                         \"^A workspace already exists with name: .*$\")"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"},{"line_number":171,"context_line":"    def test_workspace_manager_validate_path(self, mock_stdout):"},{"line_number":172,"context_line":"        fake_path \u003d \"fake_path\""},{"line_number":173,"context_line":"        ex \u003d self.assertRaises(SystemExit,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_ec41a664","line":170,"range":{"start_line":170,"start_character":4,"end_line":170,"end_character":47},"updated":"2018-06-25 16:16:29.000000000","message":"using this pattern makes it difficult to use the pdb in this test, can you change it to wrap only the workspace_manager call, like so:\n\nwith patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO) as mock_stdout:\n    ex \u003d self.assertRaises(\n        SystemExit,\n        self.workspace_manager._workspace_name_exists,\n        duplicate_name)","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":17887,"name":"Doug Schveninger","email":"ds6901@att.com","username":"Doug.Schveninger"},"change_message_id":"20755f775110c7d9c0dfe358c3344773efd14391","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                         \"^A workspace already exists with name: .*$\")"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    @patch(\u0027sys.stdout\u0027, new_callable\u003dStringIO)"},{"line_number":171,"context_line":"    def test_workspace_manager_validate_path(self, mock_stdout):"},{"line_number":172,"context_line":"        fake_path \u003d \"fake_path\""},{"line_number":173,"context_line":"        ex \u003d self.assertRaises(SystemExit,"},{"line_number":174,"context_line":"                               self.workspace_manager._validate_path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_525bbe84","line":171,"range":{"start_line":171,"start_character":8,"end_line":171,"end_character":45},"updated":"2018-06-22 10:15:35.000000000","message":"can you be more specific here with the test name. instead of test_workspace_manager_validate_path try test_workspace_manager_path_not_exist","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"0434f1185c1fca2d23ee32a853e0245359094987","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                               self.workspace_manager._validate_path,"},{"line_number":175,"context_line":"                               fake_path)"},{"line_number":176,"context_line":"        self.assertEqual(1, ex.code)"},{"line_number":177,"context_line":"        self.assertRegex(mock_stdout.getvalue(),"},{"line_number":178,"context_line":"                         \"^Path does not exist. *\")"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def test_workspace_manager_list_workspaces(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_0f21749e","line":177,"range":{"start_line":177,"start_character":19,"end_line":177,"end_character":24},"updated":"2018-06-25 16:16:29.000000000","message":"you can use assertEqual here by changing the assert to\n\nself.assertEqual(mock_stdout.getvalue(),\n                 \"Path does not exist.\\n\")","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":17887,"name":"Doug Schveninger","email":"ds6901@att.com","username":"Doug.Schveninger"},"change_message_id":"20755f775110c7d9c0dfe358c3344773efd14391","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":181,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":182,"context_line":"        self.assertIsNotNone(listed)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_92f39673","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":36},"updated":"2018-06-22 10:15:35.000000000","message":"since the class setup() set up a new workspace and the class has an attributes called self.name and self.path can you also assert the list for those value.","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"9d03a70b58a023ac9d1f6560148f8f9355e03c52","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":181,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":182,"context_line":"        self.assertIsNotNone(listed)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_c5f7eb59","line":182,"updated":"2018-06-25 08:35:28.000000000","message":"yeah, I agree with Doug. We could assert that, too. I think it should be better even if the method just returns the `self.workspaces` now, though.","commit_id":"7a1c8620b662238e43bebeef10edd6d236ec481b"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"557ad0d10cc572a53545c719ae50f22367167bfd","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        self.assertEqual(mock_stdout.getvalue(),"},{"line_number":180,"context_line":"                         \"Path does not exist.\\n\")"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":183,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":184,"context_line":"        for name, path in self.workspace_manager.workspaces.items():"},{"line_number":185,"context_line":"            self.assertIn(name, listed)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_29cf6c7f","line":182,"updated":"2018-06-26 21:15:33.000000000","message":"I think doug was saying to use self.path and self.name. By inspecting the setup method it appears this is iterating on a list of one tuple so maybe just confirm its length is 1 and remove the for loop like so:\n\nlisted \u003d self.workspace_manager.list_workspaces()\nself.assertIn(self.name, listed)\nself.assertEqual(1, len(listed))\nself.assertEqual(self.path, self.workspace_manager.workspaces.get(self.name))\nself.assertDictEqual(self.workspace_manager.workspaces, listed)","commit_id":"f209b777fefc780afb972dc71a549778bd8e51a0"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"b5ad82b3fc2d217ac9599a1b9b42f0080bfa76ee","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        for name, path in self.workspace_manager.workspaces.items():"},{"line_number":185,"context_line":"            self.assertIn(name, listed)"},{"line_number":186,"context_line":"            self.assertEqual(self.workspace_manager.workspaces.get(name), path)"},{"line_number":187,"context_line":"        self.assertDictEqual(self.workspace_manager.workspaces, listed)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_85d1c059","line":187,"updated":"2018-06-27 02:15:38.000000000","message":"I feel this assert is too much. I think we don\u0027t need to check the consistency between workspace_manager.workspaces and list_workspace(). But we should assert `listed` and `self.name` and `self.path`. Can we do like this?\n\n listed \u003d self.workspace_manager.list_workspaces()\n self.assertEqual(1, len(listed))\n self.assertTrue(listed.has_key(self.name))\n self.assertEqual(self.path, listed.get(self.name))","commit_id":"f209b777fefc780afb972dc71a549778bd8e51a0"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"b7dd0bd33b6c0e39f5764d6adba513f4998cfe4b","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":183,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":184,"context_line":"        self.assertEqual(1, len(listed))"},{"line_number":185,"context_line":"        self.assertTrue(self.name in listed)"},{"line_number":186,"context_line":"        self.assertEqual(self.path, listed.get(self.name))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_d76e772b","line":185,"range":{"start_line":185,"start_character":8,"end_line":185,"end_character":44},"updated":"2018-06-27 21:43:32.000000000","message":"I prefer the way Masayuki suggest of: listed.has_key(self.name). Otherwise this is the best approach IMO:\n\n    self.assertIn(self.name, listed)\n\nhttp://testtools.readthedocs.io/en/latest/api.html#testtools.TestCase.assertIn","commit_id":"42a765b46416bbf205aac698320c08c35fcb93f9"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"1ffc8b70960be5bf9dac4564d68422d51f27a755","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":183,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":184,"context_line":"        self.assertEqual(1, len(listed))"},{"line_number":185,"context_line":"        self.assertTrue(self.name in listed)"},{"line_number":186,"context_line":"        self.assertEqual(self.path, listed.get(self.name))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_83e75ce4","line":185,"updated":"2018-06-28 02:35:13.000000000","message":"Yeah, I think assertIn would be better. That can show an more appropriate error message when the test fails.","commit_id":"42a765b46416bbf205aac698320c08c35fcb93f9"},{"author":{"_account_id":26198,"name":"Anna Pankiewicz","email":"anna.e.pankiewicz@att.com","username":"apankiewicz"},"change_message_id":"a76d27673f9f9ff53e19b064b77a627fcd6248ea","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    def test_workspace_manager_list_workspaces(self):"},{"line_number":183,"context_line":"        listed \u003d self.workspace_manager.list_workspaces()"},{"line_number":184,"context_line":"        self.assertEqual(1, len(listed))"},{"line_number":185,"context_line":"        self.assertTrue(self.name in listed)"},{"line_number":186,"context_line":"        self.assertEqual(self.path, listed.get(self.name))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_65aa3b1b","line":185,"in_reply_to":"5f7c97a3_83e75ce4","updated":"2018-06-28 12:47:54.000000000","message":"Using has_key was causing the test to fail the py35 tox tests sadly. But I did go ahead and change that line to use assertIn.","commit_id":"42a765b46416bbf205aac698320c08c35fcb93f9"}]}