)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64c04315df07cc15adddf0e7708e296761e184c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"34ff6ba0_c424a815","updated":"2024-04-03 15:11:17.000000000","message":"seems like a nice test update","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98952b7e2cd0bab817bcbaf409a76b5ce699bd59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"36598607_d97e7141","updated":"2024-04-03 18:46:13.000000000","message":"Docs build should be fixed now. I\u0027m tempted to get this on master, though maybe we want to consider the `params\u003d{}` case and Clay\u0027s comment about tests a little more.","commit_id":"ddeb814184297fe716af2f77eeba292989ba6ac1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"63145bc0928401cfb100b37f3ded76d719d48979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e9708ba1_59175194","updated":"2024-04-04 14:55:47.000000000","message":"recheck\n\nunrelated error in func tests","commit_id":"f0e1713c8efda8d979d68a8c1e0e39351d701ae9"}],"test/unit/common/test_internal_client.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64c04315df07cc15adddf0e7708e296761e184c1","unresolved":true,"context_lines":[{"line_number":726,"context_line":"                super(FakeApp, self).__init__()"},{"line_number":727,"context_line":"                self.env \u003d None"},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"            def __call__(self, env, start_response):"},{"line_number":730,"context_line":"                self.env \u003d env"},{"line_number":731,"context_line":"                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])"},{"line_number":732,"context_line":"                return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"68765bf5_8c2ad796","line":729,"updated":"2024-04-03 15:11:17.000000000","message":"a little surprised to see a subclass of FakeSwift have to override __call__","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f032cfb0e2464c6560ca3fcafbe565ba736216b7","unresolved":false,"context_lines":[{"line_number":726,"context_line":"                super(FakeApp, self).__init__()"},{"line_number":727,"context_line":"                self.env \u003d None"},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"            def __call__(self, env, start_response):"},{"line_number":730,"context_line":"                self.env \u003d env"},{"line_number":731,"context_line":"                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])"},{"line_number":732,"context_line":"                return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fd18a28_3698c105","line":729,"in_reply_to":"68765bf5_8c2ad796","updated":"2024-04-04 13:22:26.000000000","message":"Acknowledged","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64c04315df07cc15adddf0e7708e296761e184c1","unresolved":true,"context_lines":[{"line_number":727,"context_line":"                self.env \u003d None"},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"            def __call__(self, env, start_response):"},{"line_number":730,"context_line":"                self.env \u003d env"},{"line_number":731,"context_line":"                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])"},{"line_number":732,"context_line":"                return []"},{"line_number":733,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"142d5067_7d0e302f","line":730,"updated":"2024-04-03 15:11:17.000000000","message":"I guess FakeSwift \"only\" captures the method, path and headers tuples - perhaps you really wanted to make assertions on the actual environment/QUERY_STRING","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f032cfb0e2464c6560ca3fcafbe565ba736216b7","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                self.env \u003d None"},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"            def __call__(self, env, start_response):"},{"line_number":730,"context_line":"                self.env \u003d env"},{"line_number":731,"context_line":"                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])"},{"line_number":732,"context_line":"                return []"},{"line_number":733,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bbec0b99_55fc3caf","line":730,"in_reply_to":"142d5067_7d0e302f","updated":"2024-04-04 13:22:26.000000000","message":"followed the pattern of many other tests in this module - I suspect they originally pre-dated FakeSwift and over time it\u0027s grown messy","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64c04315df07cc15adddf0e7708e296761e184c1","unresolved":true,"context_lines":[{"line_number":749,"context_line":"                            params\u003d{\u0027param1\u0027: \u0027one\u0027})"},{"line_number":750,"context_line":"        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)"},{"line_number":751,"context_line":"        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)"},{"line_number":752,"context_line":"        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027param1\u003done\u0027)"},{"line_number":753,"context_line":""},{"line_number":754,"context_line":"    def test_make_request_error_case(self):"},{"line_number":755,"context_line":"        class FakeApp(FakeSwift):"}],"source_content_type":"text/x-python","patch_set":1,"id":"aeff2fae_ac9e2f69","line":752,"updated":"2024-04-03 15:11:17.000000000","message":"if you rephrased like:\n\n\tdiff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py\n\tindex 4bd04814c..4f15e610c 100644\n\t--- a/test/unit/common/test_internal_client.py\n\t+++ b/test/unit/common/test_internal_client.py\n\t@@ -721,35 +721,26 @@ class TestInternalClient(unittest.TestCase):\n\t\t\t\t  \u0027some_agent\u0027)\n\t \n\t     def test_make_request_query_string(self):\n\t-        class FakeApp(FakeSwift):\n\t-            def __init__(self):\n\t-                super(FakeApp, self).__init__()\n\t-                self.env \u003d None\n\t-\n\t-            def __call__(self, env, start_response):\n\t-                self.env \u003d env\n\t-                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])\n\t-                return []\n\t-\n\t+        swift \u003d FakeSwift()\n\t+        swift.register(\u0027PUT\u0027, \u0027/path\u0027, swob.HTTPOk, {})\n\t\t client \u003d internal_client.InternalClient(\n\t\t     None, \u0027some_agent\u0027, 3, use_replication_network\u003dFalse,\n\t-            app\u003dFakeApp())\n\t+            app\u003dswift)\n\t \n\t\t client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,))\n\t-        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)\n\t-        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)\n\t-        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027query\u003dstring\u0027)\n\t+        self.assertEqual(\u0027PUT\u0027, swift._calls[0].method)\n\t+        self.assertEqual(\u0027/path?query\u003dstring\u0027, swift._calls[0].path)\n\t \n\t+        swift.clear_calls()\n\t\t client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,), params\u003d{})\n\t-        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)\n\t-        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)\n\t-        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027query\u003dstring\u0027)\n\t+        self.assertEqual(\u0027PUT\u0027, swift._calls[0].method)\n\t+        self.assertEqual(\u0027/path?query\u003dstring\u0027, swift._calls[0].path)\n\t \n\t+        swift.clear_calls()\n\t\t client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,),\n\t\t\t\t     params\u003d{\u0027param1\u0027: \u0027one\u0027})\n\t-        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)\n\t-        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)\n\t-        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027param1\u003done\u0027)\n\t+        self.assertEqual(\u0027PUT\u0027, swift._calls[0].method)\n\t+        self.assertEqual(\u0027/path?param1\u003done\u0027, swift._calls[0].path)\n\t \n\t     def test_make_request_error_case(self):\n\t\t class FakeApp(FakeSwift):\n\n\n... it\u0027s not obviously much weaker, and might be more \"idiomatic\" - although perhaps less obviously correct in that we\u0027re trusting FakeSwift to do QUERY_STRING/path normalization correctly (which it seems to do since PATH_INFO never has a query string)  Really, the \"params kwarg overwrites QUERY_STRING\" happens before we ever get into our FakeApp.","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f032cfb0e2464c6560ca3fcafbe565ba736216b7","unresolved":false,"context_lines":[{"line_number":749,"context_line":"                            params\u003d{\u0027param1\u0027: \u0027one\u0027})"},{"line_number":750,"context_line":"        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)"},{"line_number":751,"context_line":"        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)"},{"line_number":752,"context_line":"        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027param1\u003done\u0027)"},{"line_number":753,"context_line":""},{"line_number":754,"context_line":"    def test_make_request_error_case(self):"},{"line_number":755,"context_line":"        class FakeApp(FakeSwift):"}],"source_content_type":"text/x-python","patch_set":1,"id":"de1ca049_49a392c8","line":752,"in_reply_to":"aeff2fae_ac9e2f69","updated":"2024-04-04 13:22:26.000000000","message":"I went with the style of many other tests in this module, but agree that since we have FakeSwift and are using it then let\u0027s avoid adding more baggage.\n\nAs for all the other tests...perhaps there was a desire to make assertions on env? perhaps they originally pre-dated FakeSwift and then that got added later - I suspect mostly to provide the _pipeline_final_app property (which is annoying)? Maybe FakeSwift should also stash copies of the environs it is called with as ``raw_calls``??\n\nCould be low hanging fruit for someone to clean all the tests up.","commit_id":"92499eaf0640ad73f3d997bf59b1c67ebc6c2045"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98952b7e2cd0bab817bcbaf409a76b5ce699bd59","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,), params\u003d{})"},{"line_number":744,"context_line":"        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)"},{"line_number":745,"context_line":"        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)"},{"line_number":746,"context_line":"        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027query\u003dstring\u0027)"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,),"},{"line_number":749,"context_line":"                            params\u003d{\u0027param1\u0027: \u0027one\u0027})"}],"source_content_type":"text/x-python","patch_set":2,"id":"887b0098_95b4711f","line":746,"updated":"2024-04-03 18:46:13.000000000","message":"Oh, interesting! And subtle. I was kinda expecting it to clear it...\n\nDo we have any sense for what might break if we changed it to `if params is not None:` instead of `if params:`?","commit_id":"ddeb814184297fe716af2f77eeba292989ba6ac1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f032cfb0e2464c6560ca3fcafbe565ba736216b7","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,), params\u003d{})"},{"line_number":744,"context_line":"        self.assertEqual(client.app.env[\u0027REQUEST_METHOD\u0027], \u0027PUT\u0027)"},{"line_number":745,"context_line":"        self.assertEqual(client.app.env[\u0027PATH_INFO\u0027], \u0027/path\u0027)"},{"line_number":746,"context_line":"        self.assertEqual(client.app.env[\u0027QUERY_STRING\u0027], \u0027query\u003dstring\u0027)"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        client.make_request(\u0027PUT\u0027, \u0027/path?query\u003dstring\u0027, {}, (200,),"},{"line_number":749,"context_line":"                            params\u003d{\u0027param1\u0027: \u0027one\u0027})"}],"source_content_type":"text/x-python","patch_set":2,"id":"f3f42f3c_818d6c0c","line":746,"in_reply_to":"887b0098_95b4711f","updated":"2024-04-04 13:22:26.000000000","message":"Yeah, I doubt this was intentionally different from swob.Request\u0027s behaviour. I haven\u0027t (and don\u0027t really want to) audit every call into InternalClient to figure out if we can safely change this 😄","commit_id":"ddeb814184297fe716af2f77eeba292989ba6ac1"}],"test/unit/common/test_swob.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"689791f760562a8ea0432bad4bf4efb65f4e5f64","unresolved":true,"context_lines":[{"line_number":611,"context_line":"        self.assertDictEqual(dict(new_params), req.params)"},{"line_number":612,"context_line":""},{"line_number":613,"context_line":"        req \u003d swob.Request.blank(\u0027/?a\u003db\u0026c\u003dd\u0027, params\u003d{\u0027x\u0027: \u0027y\u0027})"},{"line_number":614,"context_line":"        self.assertEqual(req.params, {\u0027x\u0027: \u0027y\u0027})"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"    def test_unicode_params(self):"},{"line_number":617,"context_line":"        # NB: all of these strings are WSGI strings"}],"source_content_type":"text/x-python","patch_set":2,"id":"baedefea_13a37065","line":614,"updated":"2024-04-03 18:49:36.000000000","message":"Should we add a `params\u003d{}` case here, too? Looks like swob *would* clear `QUERY_STRING` in that case...","commit_id":"ddeb814184297fe716af2f77eeba292989ba6ac1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f032cfb0e2464c6560ca3fcafbe565ba736216b7","unresolved":false,"context_lines":[{"line_number":611,"context_line":"        self.assertDictEqual(dict(new_params), req.params)"},{"line_number":612,"context_line":""},{"line_number":613,"context_line":"        req \u003d swob.Request.blank(\u0027/?a\u003db\u0026c\u003dd\u0027, params\u003d{\u0027x\u0027: \u0027y\u0027})"},{"line_number":614,"context_line":"        self.assertEqual(req.params, {\u0027x\u0027: \u0027y\u0027})"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"    def test_unicode_params(self):"},{"line_number":617,"context_line":"        # NB: all of these strings are WSGI strings"}],"source_content_type":"text/x-python","patch_set":2,"id":"99d0c7f0_df035cd1","line":614,"in_reply_to":"baedefea_13a37065","updated":"2024-04-04 13:22:26.000000000","message":"Done","commit_id":"ddeb814184297fe716af2f77eeba292989ba6ac1"}]}