)]}'
{"nova/tests/unit/test_quota.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c3d18f70cee9d5b47b80c789e7fc4f043ac90440","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"                  {\u0027project_values\u0027: {\u0027instances\u0027: 0},"},{"line_number":2503,"context_line":"                   \u0027user_values\u0027: {\u0027instances\u0027: 0}}]"},{"line_number":2504,"context_line":"        for kwarg in kwargs:"},{"line_number":2505,"context_line":"            self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    def _stub_quota_reserve(self):"},{"line_number":2508,"context_line":"        def fake_quota_reserve(context, resources, quotas, user_quotas, deltas,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf091321_40914952","line":2505,"updated":"2017-06-08 12:45:17.000000000","message":"I\u0027m confused about how this test works. As far as I can see there is never an assertion being made.\n\nIf I change the integer values in kwargs to 5, 6, 7 and 8 nothing about the test changes. It still passes.\n\nWhat would the input need to be for this to be a test failure and how would we know?\n\nHow is this different from the test above? By which I mean how are we asserting the difference from the test above?\n\nIf I set all the 0 to None I get:\n\n\u003e nova.exception.InvalidQuotaValue: Change would make usage less than 0 for the following resources: [\u0027fixed_ips\u0027]\n\nWhich suggests that instead of assertions we are just letting exceptions indicate pass or fail on the test and not raising an exception means \"we exercised the code\".\n\nThat\u0027s all a bit weird to me.","commit_id":"b1647af2906db7eedc93063557497aa6204255a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bb2fa71b448bd514c8681a152656fb9f94241cec","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"                  {\u0027project_values\u0027: {\u0027instances\u0027: 0},"},{"line_number":2503,"context_line":"                   \u0027user_values\u0027: {\u0027instances\u0027: 0}}]"},{"line_number":2504,"context_line":"        for kwarg in kwargs:"},{"line_number":2505,"context_line":"            self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    def _stub_quota_reserve(self):"},{"line_number":2508,"context_line":"        def fake_quota_reserve(context, resources, quotas, user_quotas, deltas,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf091321_e0c61d17","line":2505,"in_reply_to":"bf091321_40914952","updated":"2017-06-08 12:55:06.000000000","message":"Well, sure, that\u0027s a testing pattern which is a bit confusing, but we have a lot other tests doing that.\nFor example, like you said, if we merge this test without the bugfix, then limit_check_project_and_user() would raise an exception which would just hit.\n\nFWIW, I\u0027m fine with that, I don\u0027t think we need to add a specific explicit assertion.","commit_id":"b1647af2906db7eedc93063557497aa6204255a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f5912c0fe86f161ca174edf4c19b7749f1ab27ef","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"                  {\u0027project_values\u0027: {\u0027instances\u0027: 0},"},{"line_number":2503,"context_line":"                   \u0027user_values\u0027: {\u0027instances\u0027: 0}}]"},{"line_number":2504,"context_line":"        for kwarg in kwargs:"},{"line_number":2505,"context_line":"            self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    def _stub_quota_reserve(self):"},{"line_number":2508,"context_line":"        def fake_quota_reserve(context, resources, quotas, user_quotas, deltas,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf091321_ba0ded38","line":2505,"in_reply_to":"bf091321_940d6040","updated":"2017-06-08 15:02:58.000000000","message":"This is what we get in the py35 run if we remove it:\n\nnova.tests.unit.test_quota.DbQuotaDriverTestCase.test_limit_check_project_and_user_zero_values\n----------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    b\u0027Traceback (most recent call last):\u0027\n    b\u0027  File \"/home/user/git/nova/nova/tests/unit/test_quota.py\", line 2505, in test_limit_check_project_and_user_zero_values\u0027\n    b\u0027    self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)\u0027\n    b\u0027  File \"/home/user/git/nova/nova/quota.py\", line 659, in limit_check_project_and_user\u0027\n    b\u0027    overs \u003d [key for key, val in merged_values.items()\u0027\n    b\u0027  File \"/home/user/git/nova/nova/quota.py\", line 660, in \u003clistcomp\u003e\u0027\n    b\u0027    if merged_quotas[key] \u003e\u003d 0 and merged_quotas[key] \u003c val]\u0027\n    b\u0027TypeError: unorderable types: int() \u003c NoneType()\u0027\n    b\u0027\u0027\n\n\nI don\u0027t want to hold up this series on this, but I\u0027m going to push a separate follow up change to add comments about what this is testing.","commit_id":"b1647af2906db7eedc93063557497aa6204255a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14950f9da95605d463d3981d3d6ec607db3fb93e","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"                  {\u0027project_values\u0027: {\u0027instances\u0027: 0},"},{"line_number":2503,"context_line":"                   \u0027user_values\u0027: {\u0027instances\u0027: 0}}]"},{"line_number":2504,"context_line":"        for kwarg in kwargs:"},{"line_number":2505,"context_line":"            self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    def _stub_quota_reserve(self):"},{"line_number":2508,"context_line":"        def fake_quota_reserve(context, resources, quotas, user_quotas, deltas,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf091321_3aed5def","line":2505,"in_reply_to":"bf091321_ba0ded38","updated":"2017-06-08 15:15:34.000000000","message":"https://review.openstack.org/#/c/472287/","commit_id":"b1647af2906db7eedc93063557497aa6204255a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cbdcd364f1317004863d7c622270c29680e0e8c7","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"                  {\u0027project_values\u0027: {\u0027instances\u0027: 0},"},{"line_number":2503,"context_line":"                   \u0027user_values\u0027: {\u0027instances\u0027: 0}}]"},{"line_number":2504,"context_line":"        for kwarg in kwargs:"},{"line_number":2505,"context_line":"            self.driver.limit_check_project_and_user(ctxt, resources, **kwarg)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    def _stub_quota_reserve(self):"},{"line_number":2508,"context_line":"        def fake_quota_reserve(context, resources, quotas, user_quotas, deltas,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf091321_940d6040","line":2505,"in_reply_to":"bf091321_e0c61d17","updated":"2017-06-08 14:39:28.000000000","message":"The test method could use a docstring, or a comment near the limit check call, that this is implicitly asserting we don\u0027t get a TypeError when comparing 0 to None in py3.","commit_id":"b1647af2906db7eedc93063557497aa6204255a6"}]}