)]}'
{"novaclient/tests/unit/test_api_versions.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b934910f3f885f5bd55600c2ae755640a0894512","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.assertNotEqual(v1, v2)"},{"line_number":91,"context_line":"        self.assertEqual(v1, v4)"},{"line_number":92,"context_line":"        self.assertNotEqual(v1, v_null)"},{"line_number":93,"context_line":"        self.assertEqual(v_null, v_null)"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_6febe178","line":93,"updated":"2016-09-06 21:45:19.000000000","message":"In this particular case, I think I prefer the original versions.  My reasoning is that we\u0027re testing the magic methods in the APIVersion class, and these test methods are hiding what\u0027s being done.\n\nPerhaps we can compromise, by explicitly calling the magic method in question, i.e.:\n\n    self.assertTrue(v1.__lt__(v2))","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ba789b88cce56b90d682efcbd04633bec44e10f5","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.assertNotEqual(v1, v2)"},{"line_number":91,"context_line":"        self.assertEqual(v1, v4)"},{"line_number":92,"context_line":"        self.assertNotEqual(v1, v_null)"},{"line_number":93,"context_line":"        self.assertEqual(v_null, v_null)"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_ce9beb16","line":93,"in_reply_to":"3a71b18c_2e7e1f19","updated":"2016-12-05 16:57:39.000000000","message":"My review crossed with your comments, so sorry about that.  My concern is that what we\u0027re trying to do is test the implementation of the magic methods __lt__(), etc.  As such, using a method like assertLess() hides our intent.  For that matter, using the \u0027\u003c\u0027 operator like we do here also hides our intent.  That\u0027s why I was suggesting using \"self.assertTrue(v1.__lt__(v2))\", as it makes clear what the intent is.\n\nFTR, I did briefly look at the review you linked, and based solely on the description, I think my suggestion of explicitly calling the magic methods will still work fine.  Of course, please let me know if there are details I missed that would make my suggestion untenable.","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"d93a5212841f8f8e715849a8ac85356605fd1eee","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.assertNotEqual(v1, v2)"},{"line_number":91,"context_line":"        self.assertEqual(v1, v4)"},{"line_number":92,"context_line":"        self.assertNotEqual(v1, v_null)"},{"line_number":93,"context_line":"        self.assertEqual(v_null, v_null)"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_8ffd5170","line":93,"in_reply_to":"3a71b18c_ce9beb16","updated":"2016-12-06 08:46:50.000000000","message":"Thanks for the great explanation! I modified the code in the\nway you mentioned. Thanks for your review!","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"9cad635a7d6b3f5afe1df89160f251eb7abb8711","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.assertNotEqual(v1, v2)"},{"line_number":91,"context_line":"        self.assertEqual(v1, v4)"},{"line_number":92,"context_line":"        self.assertNotEqual(v1, v_null)"},{"line_number":93,"context_line":"        self.assertEqual(v_null, v_null)"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_2e7e1f19","line":93,"in_reply_to":"9a89bdaa_6febe178","updated":"2016-12-05 16:51:27.000000000","message":"Hey, \n\nIn the near future, [1] will be merged to the master branch\nof hacking. Once it gets merged, we have to use assertion in\nthe way they explained. That\u0027s the reason why I did not\nchanged this. But if you explain, why\nself.assertTrue(v1.__lt__(v2)) is better than the current\none, I will change it, but if I am right, the assertLess method does almost the same thing.\n\n[1]: https://review.openstack.org/#/c/354185/","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"f161f22c9286060dc6062586f897db48eae94ed6","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.assertNotEqual(v1, v2)"},{"line_number":91,"context_line":"        self.assertEqual(v1, v4)"},{"line_number":92,"context_line":"        self.assertNotEqual(v1, v_null)"},{"line_number":93,"context_line":"        self.assertEqual(v_null, v_null)"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_0eaec397","line":93,"updated":"2016-12-05 16:51:31.000000000","message":"In this particular case, I think I prefer the original versions.  My reasoning is that we\u0027re testing the magic methods in the APIVersion class, and these test methods are hiding what\u0027s being done.\n\nPerhaps we can compromise, by explicitly calling the magic method in question, i.e.:\n\n    self.assertTrue(v1.__lt__(v2))","commit_id":"7f048977c39e792da23d6aa1d3b7b1ed8a77c137"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"0b878f2b2753d6c351763236534f2c0e07355ec7","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        v4 \u003d api_versions.APIVersion(\"2.0\")"},{"line_number":86,"context_line":"        v_null \u003d api_versions.APIVersion()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        self.assertTrue(v1.__lt__(v2))"},{"line_number":89,"context_line":"        self.assertTrue(v3.__gt__(v2))"},{"line_number":90,"context_line":"        self.assertTrue(v1.__ne__(v2))"},{"line_number":91,"context_line":"        self.assertTrue(v1.__eq__(v4))"},{"line_number":92,"context_line":"        self.assertTrue(v1.__ne__(v_null))"},{"line_number":93,"context_line":"        self.assertTrue(v_null.__eq__(v_null))"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_7acdbf5c","line":93,"range":{"start_line":88,"start_character":1,"end_line":93,"end_character":46},"updated":"2016-12-07 18:06:09.000000000","message":"This makes me sad. Can you point me to some documentation that considers this a best practice now? \n\nPerhaps I missed the memo, but this is a lot less clear as far as I\u0027m concerned. No offence ;)\n\nAnd in both cases you just get \"False is not true\" [1], so it\u0027s unclear to me how this is an improvement.\n\n[1] http://paste.openstack.org/show/591696/","commit_id":"ce184797f07675d403245a541de42cd48450d320"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"877afd2def382604d67b702b0dcd6c87323ac85b","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        v4 \u003d api_versions.APIVersion(\"2.0\")"},{"line_number":86,"context_line":"        v_null \u003d api_versions.APIVersion()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        self.assertTrue(v1.__lt__(v2))"},{"line_number":89,"context_line":"        self.assertTrue(v3.__gt__(v2))"},{"line_number":90,"context_line":"        self.assertTrue(v1.__ne__(v2))"},{"line_number":91,"context_line":"        self.assertTrue(v1.__eq__(v4))"},{"line_number":92,"context_line":"        self.assertTrue(v1.__ne__(v_null))"},{"line_number":93,"context_line":"        self.assertTrue(v_null.__eq__(v_null))"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"da4df55a_9dd07d55","line":93,"range":{"start_line":88,"start_character":1,"end_line":93,"end_character":46},"in_reply_to":"3a71b18c_0dd2d434","updated":"2016-12-23 12:03:12.000000000","message":"It\u0027s not pretty but being explicit in this particular instance is no harm","commit_id":"ce184797f07675d403245a541de42cd48450d320"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"7e12b3f091fdcc1a632f9a0abc9b3c23e79bf890","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        v4 \u003d api_versions.APIVersion(\"2.0\")"},{"line_number":86,"context_line":"        v_null \u003d api_versions.APIVersion()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        self.assertTrue(v1.__lt__(v2))"},{"line_number":89,"context_line":"        self.assertTrue(v3.__gt__(v2))"},{"line_number":90,"context_line":"        self.assertTrue(v1.__ne__(v2))"},{"line_number":91,"context_line":"        self.assertTrue(v1.__eq__(v4))"},{"line_number":92,"context_line":"        self.assertTrue(v1.__ne__(v_null))"},{"line_number":93,"context_line":"        self.assertTrue(v_null.__eq__(v_null))"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_ba02f709","line":93,"range":{"start_line":88,"start_character":1,"end_line":93,"end_character":46},"in_reply_to":"3a71b18c_7acdbf5c","updated":"2016-12-07 18:17:59.000000000","message":"This is something I asked for, to make explicit that we are testing the magic methods.  The previous version, using the operators, isn\u0027t going to be in compliance with hacking anymore, and I worried that the version using, e.g., assertLess() et. al. would be even less clear on that point.\n\nThe message could be corrected by adding a message parameter to these assertTrue calls, so perhaps that could be a way to address the \"False is not true\" problem?","commit_id":"ce184797f07675d403245a541de42cd48450d320"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"b56371c090ff284bcaeb657365381892053ebafc","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        v4 \u003d api_versions.APIVersion(\"2.0\")"},{"line_number":86,"context_line":"        v_null \u003d api_versions.APIVersion()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        self.assertTrue(v1.__lt__(v2))"},{"line_number":89,"context_line":"        self.assertTrue(v3.__gt__(v2))"},{"line_number":90,"context_line":"        self.assertTrue(v1.__ne__(v2))"},{"line_number":91,"context_line":"        self.assertTrue(v1.__eq__(v4))"},{"line_number":92,"context_line":"        self.assertTrue(v1.__ne__(v_null))"},{"line_number":93,"context_line":"        self.assertTrue(v_null.__eq__(v_null))"},{"line_number":94,"context_line":"        self.assertRaises(TypeError, v1.__le__, \"2.1\")"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_version_matches(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_0dd2d434","line":93,"range":{"start_line":88,"start_character":1,"end_line":93,"end_character":46},"in_reply_to":"3a71b18c_ba02f709","updated":"2016-12-07 19:42:19.000000000","message":"Here\u0027s what using assertGreater would look like on failure:\n\n    Traceback (most recent call last):\n      File \"novaclient/tests/unit/test_api_versions.py\", line 89, in test_version_comparisons\n        self.assertGreater(v3, v2)\n      File \"/Users/diana/workspace/python-novaclient/.tox/py27/lib/python2.7/site-packages/unittest2/case.py\", line 1233, in assertGreater\n        self.fail(self._formatMessage(msg, standardMsg))\n      File \"/Users/diana/workspace/python-novaclient/.tox/py27/lib/python2.7/site-packages/unittest2/case.py\", line 690, in fail\n        raise self.failureException(msg)\n    AssertionError: \u003cAPIVersion: 2.2\u003e not greater than \u003cAPIVersion: 2.5\u003e\n\nPS. That\u0027s the memo I missed: \"using operators isn\u0027t going to be in compliance with hacking anymore\". Good to know, thanks! I\u0027ve updated my usage patch to not use operators in asserts.","commit_id":"ce184797f07675d403245a541de42cd48450d320"}],"novaclient/tests/unit/test_shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b934910f3f885f5bd55600c2ae755640a0894512","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        self.assertEqual(result.emitted, set())"},{"line_number":183,"context_line":"        self.assertIsNone(result.use)"},{"line_number":184,"context_line":"        self.assertFalse(result.real_action_args)"},{"line_number":185,"context_line":"        self.assertIsNone(result.real_action)"},{"line_number":186,"context_line":"        mock_init.assert_called_once_with("},{"line_number":187,"context_line":"            \u0027option_strings\u0027, \u0027dest\u0027, help\u003d\u0027Deprecated\u0027, a\u003d1, b\u003d2, c\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_af8889c6","line":184,"updated":"2016-09-06 21:45:19.000000000","message":"My original intent here was to ensure that real_action_args is exactly False, not just that it evaluates to False.  Perhaps the proper solution is to go to assertIs()?","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"9cad635a7d6b3f5afe1df89160f251eb7abb8711","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        self.assertEqual(result.emitted, set())"},{"line_number":183,"context_line":"        self.assertIsNone(result.use)"},{"line_number":184,"context_line":"        self.assertFalse(result.real_action_args)"},{"line_number":185,"context_line":"        self.assertIsNone(result.real_action)"},{"line_number":186,"context_line":"        mock_init.assert_called_once_with("},{"line_number":187,"context_line":"            \u0027option_strings\u0027, \u0027dest\u0027, help\u003d\u0027Deprecated\u0027, a\u003d1, b\u003d2, c\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_ae906f44","line":184,"in_reply_to":"9a89bdaa_af8889c6","updated":"2016-12-05 16:51:27.000000000","message":"Done","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b934910f3f885f5bd55600c2ae755640a0894512","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        self.assertEqual(result.emitted, set())"},{"line_number":210,"context_line":"        self.assertIsNone(result.use)"},{"line_number":211,"context_line":"        self.assertFalse(result.real_action_args)"},{"line_number":212,"context_line":"        self.assertEqual(result.real_action, action.return_value)"},{"line_number":213,"context_line":"        mock_init.assert_called_once_with("},{"line_number":214,"context_line":"            \u0027option_strings\u0027, \u0027dest\u0027, help\u003d\u0027Deprecated\u0027, a\u003d1, b\u003d2, c\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_ef8e11cf","line":211,"updated":"2016-09-06 21:45:19.000000000","message":"ibid","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"9cad635a7d6b3f5afe1df89160f251eb7abb8711","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        self.assertEqual(result.emitted, set())"},{"line_number":210,"context_line":"        self.assertIsNone(result.use)"},{"line_number":211,"context_line":"        self.assertFalse(result.real_action_args)"},{"line_number":212,"context_line":"        self.assertEqual(result.real_action, action.return_value)"},{"line_number":213,"context_line":"        mock_init.assert_called_once_with("},{"line_number":214,"context_line":"            \u0027option_strings\u0027, \u0027dest\u0027, help\u003d\u0027Deprecated\u0027, a\u003d1, b\u003d2, c\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_ce95ab33","line":211,"in_reply_to":"9a89bdaa_ef8e11cf","updated":"2016-12-05 16:51:27.000000000","message":"Done","commit_id":"babc85fb97c416b10680cf16a04d6eb4c240c242"}]}