)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"f91dad314a8569c013d5ae13614aa79e5ebb7d07","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Adds more functional tests for commands"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds additional functional tests for the commands API."},{"line_number":10,"context_line":"It also restructures the current functional test to run in "},{"line_number":11,"context_line":"sequence as a monolithic test to preserve IPA state and test "},{"line_number":12,"context_line":"order between test runs. The time to wait for IPA to start "},{"line_number":13,"context_line":"was increased due to an intermittent race condition that "}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba15a1d1_a79cd80e","line":10,"updated":"2015-09-16 23:57:52.000000000","message":"nit: trailing whitespace on this and the next 3 lines","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Adds more functional tests for commands"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds additional functional tests for the commands API."},{"line_number":10,"context_line":"It also restructures the current functional test to run in "},{"line_number":11,"context_line":"sequence as a monolithic test to preserve IPA state and test "},{"line_number":12,"context_line":"order between test runs. The time to wait for IPA to start "},{"line_number":13,"context_line":"was increased due to an intermittent race condition that "}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba15a1d1_28986969","line":10,"in_reply_to":"ba15a1d1_a79cd80e","updated":"2015-09-17 19:08:31.000000000","message":"Think this is fixed in the next patch set, for some reason that happened when I manually edited the commit message from gerrit. I\u0027ll make another patch set if it still isn\u0027t fixed.","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"}],"ironic_python_agent/tests/functional/base.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1972ab1bf1580413fd6b884e8c1d1d6aeb8bd9c3","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":51,"context_line":"                                        test_port)"},{"line_number":52,"context_line":"            except requests.ConnectionError:"},{"line_number":53,"context_line":"                time.sleep(sleep_time)"},{"line_number":54,"context_line":"                tries +\u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_36330a16","line":51,"updated":"2015-09-17 08:53:23.000000000","message":"nit: indentation is off","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":51,"context_line":"                                        test_port)"},{"line_number":52,"context_line":"            except requests.ConnectionError:"},{"line_number":53,"context_line":"                time.sleep(sleep_time)"},{"line_number":54,"context_line":"                tries +\u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_e2cdb066","line":51,"in_reply_to":"ba15a1d1_36330a16","updated":"2015-09-17 19:08:31.000000000","message":"Fixed, made it consistent with hanging indentation present elsewhere","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6eaa944bf77351fa957058d3c4d4801abbabe93c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                tries +\u003d 1"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        raise IOError(\u0027Agent did not start after %s seconds.\u0027 % (max_tries *"},{"line_number":57,"context_line":"            sleep_time))"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_a93d462c","line":57,"updated":"2015-09-18 11:32:23.000000000","message":"I think we need a base request function here, like:\n\n def request(method, url, expect_error\u003dNone, **kwargs):\n   res \u003d getattr(requests, method)(\u0027http://localhost:%s/v1/%s\u0027 % (self.test_port, url), **kwargs)\n   if expect_error:\n     self.assertEqual(expect_error, res.status_code)\n   else:\n     res.raise_for_status()\n   return res","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"151038d54e25fcdbfff82cf478d73ef6d2361a94","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                tries +\u003d 1"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        raise IOError(\u0027Agent did not start after %s seconds.\u0027 % (max_tries *"},{"line_number":57,"context_line":"            sleep_time))"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_214a9456","line":57,"in_reply_to":"ba15a1d1_a93d462c","updated":"2015-09-18 21:54:52.000000000","message":"Done","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"13eda52eb5bf49933c2c44ed25a34b5b6d87459f","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return requests.get("},{"line_number":51,"context_line":"                    \u0027http://localhost:%s/v1/commands\u0027 % self.test_port)"},{"line_number":52,"context_line":"            except requests.ConnectionError:"},{"line_number":53,"context_line":"                time.sleep(sleep_time)"},{"line_number":54,"context_line":"                tries +\u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_46474a4b","line":51,"updated":"2015-09-28 04:56:47.000000000","message":"since the connection string is used in two places you should probably make it a class variable or module constant.","commit_id":"47fccbc0f75562c8f02fd9fee584ccdc4082e715"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"78019feb0f4ad6c57c49a729a67fa310326e1275","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return requests.get("},{"line_number":51,"context_line":"                    \u0027http://localhost:%s/v1/commands\u0027 % self.test_port)"},{"line_number":52,"context_line":"            except requests.ConnectionError:"},{"line_number":53,"context_line":"                time.sleep(sleep_time)"},{"line_number":54,"context_line":"                tries +\u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_af94d6e1","line":51,"in_reply_to":"9a1a9d01_46474a4b","updated":"2015-09-28 19:10:21.000000000","message":"Good catch. I changed it to use the request function that another reviewer wanted me to create. I hope that suffices.","commit_id":"47fccbc0f75562c8f02fd9fee584ccdc4082e715"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"d26f206ac511dfb159745ae421c080b81f53d599","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from oslotest import base as test_base"},{"line_number":21,"context_line":"import requests"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic_python_agent import agent"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FunctionalBase(test_base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_7eef17e9","line":23,"updated":"2015-09-29 20:20:05.000000000","message":"I wonder if should create a requests.Session() object? But I\u0027m not sure. Maybe that could alter testing?","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"b49dcaf20c365592ce77314caff73dded22797f4","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from oslotest import base as test_base"},{"line_number":21,"context_line":"import requests"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic_python_agent import agent"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FunctionalBase(test_base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_ef06131b","line":23,"in_reply_to":"9a1a9d01_49f05ffe","updated":"2015-09-29 21:37:40.000000000","message":"I was thinking here as a global.\n\nThe benefit is that it keeps the connection open and could speed up performance.","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"0e2d84371312ed1c1086042ff68f202d7f190789","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from oslotest import base as test_base"},{"line_number":21,"context_line":"import requests"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic_python_agent import agent"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FunctionalBase(test_base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_49f05ffe","line":23,"in_reply_to":"9a1a9d01_7eef17e9","updated":"2015-09-29 21:30:04.000000000","message":"Do you mean here (in the import, where the comment is located), or while querying the agent? The agent is its own process so that we can test an agent that\u0027s actually running on the host.\n\nFor querying the agent, what would the benefit be?","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"d26f206ac511dfb159745ae421c080b81f53d599","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        # Wait for process to start, otherwise we have a race for tests"},{"line_number":45,"context_line":"        sleep_time \u003d 0.1"},{"line_number":46,"context_line":"        tries \u003d 0"},{"line_number":47,"context_line":"        max_tries \u003d os.environ.get(\u0027IPA_WAIT_TRIES\u0027, \u0027100\u0027)"},{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return self.request(\u0027get\u0027, \u0027commands\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_7ebcf705","line":47,"updated":"2015-09-29 20:20:05.000000000","message":"nit: maybe it could convert it to int here, instead of doing it up to 100 times","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"0e2d84371312ed1c1086042ff68f202d7f190789","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        # Wait for process to start, otherwise we have a race for tests"},{"line_number":45,"context_line":"        sleep_time \u003d 0.1"},{"line_number":46,"context_line":"        tries \u003d 0"},{"line_number":47,"context_line":"        max_tries \u003d os.environ.get(\u0027IPA_WAIT_TRIES\u0027, \u0027100\u0027)"},{"line_number":48,"context_line":"        while tries \u003c int(max_tries):"},{"line_number":49,"context_line":"            try:"},{"line_number":50,"context_line":"                return self.request(\u0027get\u0027, \u0027commands\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_e90cd3ed","line":47,"in_reply_to":"9a1a9d01_7ebcf705","updated":"2015-09-29 21:30:04.000000000","message":"Converted to int here, good catch. 100 is just the default, it can be specified in the configuration if we want to do it less times. We can probably get away with less max wait time here, I\u0027m just keeping it this high to be sure it starts.","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"d26f206ac511dfb159745ae421c080b81f53d599","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        :returns: the response object"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        res \u003d getattr(requests, method)(\u0027http://localhost:%s/v1/%s\u0027 %"},{"line_number":69,"context_line":"                (self.test_port, path), **kwargs)"},{"line_number":70,"context_line":"        if expect_error is not None:"},{"line_number":71,"context_line":"            self.assertEqual(expect_error, res.status_code)"},{"line_number":72,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_9e154be6","line":69,"updated":"2015-09-29 20:20:05.000000000","message":"Please use requests.request()\n\nhttp://docs.python-requests.org/en/v1.0.4/api/\n\nProbably could replace most/all of this function.","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"0e2d84371312ed1c1086042ff68f202d7f190789","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        :returns: the response object"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        res \u003d getattr(requests, method)(\u0027http://localhost:%s/v1/%s\u0027 %"},{"line_number":69,"context_line":"                (self.test_port, path), **kwargs)"},{"line_number":70,"context_line":"        if expect_error is not None:"},{"line_number":71,"context_line":"            self.assertEqual(expect_error, res.status_code)"},{"line_number":72,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_69ef63e9","line":69,"in_reply_to":"9a1a9d01_9e154be6","updated":"2015-09-29 21:30:04.000000000","message":"Good idea, this function looks better now.\n\nI kept the function around because another reviewer wanted error checking within a base class function, but it uses requests.request now.","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2c13ff388179d19f390012672b46e5473dd062a1","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        raise IOError(\u0027Agent did not start after %s seconds.\u0027 % (max_tries *"},{"line_number":56,"context_line":"            sleep_time))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def request(self, method, path, expect_error\u003dNone, **kwargs):"},{"line_number":59,"context_line":"        \"\"\"Send a request to the agent and verifies response."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        :param: method type of request to send as a string"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_38adaa31","line":58,"updated":"2015-10-02 15:54:28.000000000","message":"one more simplification idea: you can add expect_json\u003dTrue here, and then below\n\n if expect_json:\n   return res.json()\n else:\n   return res","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8df5040934d5524e076d3b15b7767082dc978f2f","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        raise IOError(\u0027Agent did not start after %s seconds.\u0027 % (max_tries *"},{"line_number":56,"context_line":"            sleep_time))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def request(self, method, path, expect_error\u003dNone, **kwargs):"},{"line_number":59,"context_line":"        \"\"\"Send a request to the agent and verifies response."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        :param: method type of request to send as a string"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_ce72a3e3","line":58,"in_reply_to":"9a1a9d01_38adaa31","updated":"2015-10-02 18:26:28.000000000","message":"I like that idea, as I have done it in my own little REST query programs.","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        raise IOError(\u0027Agent did not start after %s seconds.\u0027 % (max_tries *"},{"line_number":56,"context_line":"            sleep_time))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def request(self, method, path, expect_error\u003dNone, **kwargs):"},{"line_number":59,"context_line":"        \"\"\"Send a request to the agent and verifies response."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        :param: method type of request to send as a string"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_f73ad333","line":58,"in_reply_to":"9a1a9d01_ce72a3e3","updated":"2015-10-02 23:04:21.000000000","message":"Done","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2c13ff388179d19f390012672b46e5473dd062a1","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        :param: path desired API endpoint to request, for example \u0027commands\u0027"},{"line_number":63,"context_line":"        :param: expect_error error code to expect, if an error is expected"},{"line_number":64,"context_line":"        :param **kwargs: keyword args to pass to the request method"},{"line_number":65,"context_line":"        :raises: HTTPError if an error is returned that was not expected"},{"line_number":66,"context_line":"        :returns: the response object"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        res \u003d requests.request(method, \u0027http://localhost:%s/v1/%s\u0027 %"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_d8a90e26","line":65,"updated":"2015-10-02 15:54:28.000000000","message":":raises: AssertionError if a received HTTP status code does not match expect_error","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        :param: path desired API endpoint to request, for example \u0027commands\u0027"},{"line_number":63,"context_line":"        :param: expect_error error code to expect, if an error is expected"},{"line_number":64,"context_line":"        :param **kwargs: keyword args to pass to the request method"},{"line_number":65,"context_line":"        :raises: HTTPError if an error is returned that was not expected"},{"line_number":66,"context_line":"        :returns: the response object"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        res \u003d requests.request(method, \u0027http://localhost:%s/v1/%s\u0027 %"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_974917d3","line":65,"in_reply_to":"9a1a9d01_d8a90e26","updated":"2015-10-02 23:04:21.000000000","message":"Fixed","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"de63f7118b4901fa25a5a22bc19a1b984e587484","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        :param: method type of request to send as a string"},{"line_number":64,"context_line":"        :param: path desired API endpoint to request, for example \u0027commands\u0027"},{"line_number":65,"context_line":"        :param: expect_error error code to expect, if an error is expected"},{"line_number":66,"context_line":"        :param **kwargs: keyword args to pass to the request method"},{"line_number":67,"context_line":"        :raises: HTTPError if an error is returned that was not expected"},{"line_number":68,"context_line":"        :raises: AssertionError if a received HTTP status code does not match"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a2fa921_4e880223","line":65,"updated":"2015-10-06 06:54:25.000000000","message":"Sorry, but you\u0027re now missing expect_json in the docstring","commit_id":"479ad0cd688ff0269c42b21a7c224e2b7675323d"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"0a3cc36b85fa1ad2a1420b7f892285d80ad8e495","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        :param: method type of request to send as a string"},{"line_number":64,"context_line":"        :param: path desired API endpoint to request, for example \u0027commands\u0027"},{"line_number":65,"context_line":"        :param: expect_error error code to expect, if an error is expected"},{"line_number":66,"context_line":"        :param **kwargs: keyword args to pass to the request method"},{"line_number":67,"context_line":"        :raises: HTTPError if an error is returned that was not expected"},{"line_number":68,"context_line":"        :raises: AssertionError if a received HTTP status code does not match"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a2fa921_9decf5c8","line":65,"in_reply_to":"7a2fa921_4e880223","updated":"2015-10-06 15:43:13.000000000","message":"Thanks, I added the parameter.","commit_id":"479ad0cd688ff0269c42b21a7c224e2b7675323d"}],"ironic_python_agent/tests/functional/test_commands.py":[{"author":{"_account_id":17328,"name":"Lekha Jeevan","email":"lekha.jeevan@gmail.com","username":"ljee"},"change_message_id":"eb19699c00bfa15bee894182e31ec0faa3101797","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        # NOTE(mariojv): get_clean_steps always returns the default"},{"line_number":37,"context_line":"        # HardwareManager clean steps if there\u0027s not a more specific HWM. So,"},{"line_number":38,"context_line":"        # this command succeeds even with an empty node and port."},{"line_number":39,"context_line":"        command \u003d {\u0027name\u0027: \u0027clean.get_clean_steps\u0027,"},{"line_number":40,"context_line":"                \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_5cf802c3","line":39,"updated":"2015-09-17 17:30:29.000000000","message":"can this be param we pass to `step_2_run_command` so we can test other commands as well.","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        # NOTE(mariojv): get_clean_steps always returns the default"},{"line_number":37,"context_line":"        # HardwareManager clean steps if there\u0027s not a more specific HWM. So,"},{"line_number":38,"context_line":"        # this command succeeds even with an empty node and port."},{"line_number":39,"context_line":"        command \u003d {\u0027name\u0027: \u0027clean.get_clean_steps\u0027,"},{"line_number":40,"context_line":"                \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_220ba87e","line":39,"in_reply_to":"ba15a1d1_5cf802c3","updated":"2015-09-17 19:08:31.000000000","message":"What do you think that should look like? Right now the actual monolithic test that runs doesn\u0027t introspect on the test being run, it just runs them in order, so it would actually have to figure out that this is the step being run so that it only passes the parameter to this step.\n\nMaybe we can just make a helper method, and have the step define the command and pass it to the helper?","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1972ab1bf1580413fd6b884e8c1d1d6aeb8bd9c3","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        command \u003d {\u0027name\u0027: \u0027clean.get_clean_steps\u0027,"},{"line_number":40,"context_line":"                \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"},{"line_number":43,"context_line":"                data\u003djson.dumps(command), headers\u003d{\u0027Content-Type\u0027:"},{"line_number":44,"context_line":"                    \u0027application/json\u0027})"},{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_d65dee47","line":42,"updated":"2015-09-17 08:53:23.000000000","message":"this can be moved to top level as a constant","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        command \u003d {\u0027name\u0027: \u0027clean.get_clean_steps\u0027,"},{"line_number":40,"context_line":"                \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"},{"line_number":43,"context_line":"                data\u003djson.dumps(command), headers\u003d{\u0027Content-Type\u0027:"},{"line_number":44,"context_line":"                    \u0027application/json\u0027})"},{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_22b5e87c","line":42,"in_reply_to":"ba15a1d1_d65dee47","updated":"2015-09-17 19:08:31.000000000","message":"Done. I attached it in the setUp method of the superclass so that other functional tests can use it, too.","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1972ab1bf1580413fd6b884e8c1d1d6aeb8bd9c3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"},{"line_number":43,"context_line":"                data\u003djson.dumps(command), headers\u003d{\u0027Content-Type\u0027:"},{"line_number":44,"context_line":"                    \u0027application/json\u0027})"},{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":46,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_7656622b","line":44,"updated":"2015-09-17 08:53:23.000000000","message":"please pep8ise indentation here and below","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        response \u003d requests.post(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":42,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027),"},{"line_number":43,"context_line":"                data\u003djson.dumps(command), headers\u003d{\u0027Content-Type\u0027:"},{"line_number":44,"context_line":"                    \u0027application/json\u0027})"},{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":46,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_054a2e26","line":44,"in_reply_to":"ba15a1d1_7656622b","updated":"2015-09-17 19:08:31.000000000","message":"Fixed, I made it hanging indentation 4 spaces to the right of previous line start here and elsewhere where it\u0027s needed to match what was in base.py.","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":17328,"name":"Lekha Jeevan","email":"lekha.jeevan@gmail.com","username":"ljee"},"change_message_id":"eb19699c00bfa15bee894182e31ec0faa3101797","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":46,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    def step_3_get_commands(self):"},{"line_number":49,"context_line":"        commands \u003d requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":50,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027))"},{"line_number":51,"context_line":"        self.assertEqual(200, commands.status_code)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_1c9a8a6e","line":48,"updated":"2015-09-17 17:30:29.000000000","message":"Can we also add a step to get the command using the command id (http://localhost:\u003cport\u003e/v1/commands/\u003ccommand_id\u003e)","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":46,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    def step_3_get_commands(self):"},{"line_number":49,"context_line":"        commands \u003d requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":50,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027))"},{"line_number":51,"context_line":"        self.assertEqual(200, commands.status_code)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_08e6c514","line":48,"in_reply_to":"ba15a1d1_1c9a8a6e","updated":"2015-09-17 19:08:31.000000000","message":"Done","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1972ab1bf1580413fd6b884e8c1d1d6aeb8bd9c3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        commands \u003d requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":50,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027))"},{"line_number":51,"context_line":"        self.assertEqual(200, commands.status_code)"},{"line_number":52,"context_line":"        self.assertEqual(1, len(commands.json()[\u0027commands\u0027]))"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def step_4_run_non_existent_command(self):"},{"line_number":55,"context_line":"        fake_command \u003d {\u0027name\u0027: \u0027bad_extension.fake_command\u0027, \u0027params\u0027: {}}"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_9653f638","line":52,"updated":"2015-09-17 08:53:23.000000000","message":"I think we can check that this really the command we expect","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        commands \u003d requests.get(\u0027http://localhost:%s/v1/commands\u0027 %"},{"line_number":50,"context_line":"                os.environ.get(\u0027TEST_PORT\u0027, \u00279999\u0027))"},{"line_number":51,"context_line":"        self.assertEqual(200, commands.status_code)"},{"line_number":52,"context_line":"        self.assertEqual(1, len(commands.json()[\u0027commands\u0027]))"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def step_4_run_non_existent_command(self):"},{"line_number":55,"context_line":"        fake_command \u003d {\u0027name\u0027: \u0027bad_extension.fake_command\u0027, \u0027params\u0027: {}}"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_e5d4daec","line":52,"in_reply_to":"ba15a1d1_9653f638","updated":"2015-09-17 19:08:31.000000000","message":"Done","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6eaa944bf77351fa957058d3c4d4801abbabe93c","unresolved":false,"context_lines":[{"line_number":39,"context_line":"            \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":40,"context_line":"        response \u003d requests.post("},{"line_number":41,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":42,"context_line":"            data\u003djson.dumps(command),"},{"line_number":43,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":44,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":45,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_c933da60","line":42,"updated":"2015-09-18 11:32:23.000000000","message":"nit: json\u003dcommand","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"151038d54e25fcdbfff82cf478d73ef6d2361a94","unresolved":false,"context_lines":[{"line_number":39,"context_line":"            \u0027params\u0027: {\u0027node\u0027: {}, \u0027ports\u0027: {}}}"},{"line_number":40,"context_line":"        response \u003d requests.post("},{"line_number":41,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":42,"context_line":"            data\u003djson.dumps(command),"},{"line_number":43,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":44,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":45,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_e1801c4c","line":42,"in_reply_to":"ba15a1d1_c933da60","updated":"2015-09-18 21:54:52.000000000","message":"Done","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6eaa944bf77351fa957058d3c4d4801abbabe93c","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":42,"context_line":"            data\u003djson.dumps(command),"},{"line_number":43,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":44,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":45,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def step_3_get_commands(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_e936de4f","line":44,"updated":"2015-09-18 11:32:23.000000000","message":"this check won\u0027t be needed if you implement request as above","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"151038d54e25fcdbfff82cf478d73ef6d2361a94","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":42,"context_line":"            data\u003djson.dumps(command),"},{"line_number":43,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":44,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":45,"context_line":"        self.assertIsNone(response.json()[\u0027command_error\u0027])"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def step_3_get_commands(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_01867065","line":44,"in_reply_to":"ba15a1d1_e936de4f","updated":"2015-09-18 21:54:52.000000000","message":"Done","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6eaa944bf77351fa957058d3c4d4801abbabe93c","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        fake_command \u003d {\u0027name\u0027: \u0027bad_extension.fake_command\u0027, \u0027params\u0027: {}}"},{"line_number":76,"context_line":"        response \u003d requests.post("},{"line_number":77,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":78,"context_line":"            data\u003djson.dumps(fake_command),"},{"line_number":79,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":80,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_8942c2ac","line":78,"updated":"2015-09-18 11:32:23.000000000","message":"ditto","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"151038d54e25fcdbfff82cf478d73ef6d2361a94","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        fake_command \u003d {\u0027name\u0027: \u0027bad_extension.fake_command\u0027, \u0027params\u0027: {}}"},{"line_number":76,"context_line":"        response \u003d requests.post("},{"line_number":77,"context_line":"            \u0027http://localhost:%s/v1/commands\u0027 % self.test_port,"},{"line_number":78,"context_line":"            data\u003djson.dumps(fake_command),"},{"line_number":79,"context_line":"            headers\u003d{\u0027Content-Type\u0027: \u0027application/json\u0027})"},{"line_number":80,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_a1e6a4ae","line":78,"in_reply_to":"ba15a1d1_8942c2ac","updated":"2015-09-18 21:54:52.000000000","message":"Done","commit_id":"0a8fe7d20ba012ce707d49137588fe1db43615a4"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"13eda52eb5bf49933c2c44ed25a34b5b6d87459f","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    These tests are structured monolithically as one"},{"line_number":22,"context_line":"    test with multiple steps to preserve ordering and ensure IPA state remains"},{"line_number":23,"context_line":"    consistent across different test runs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":"    def step_1_get_empty_commands(self):"},{"line_number":26,"context_line":"        response \u003d self.request(\u0027get\u0027, \u0027commands\u0027).json()"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_dd20049e","line":23,"updated":"2015-09-28 04:56:47.000000000","message":"Don\u0027t get me wrong, I like _how_ you\u0027ve created the ability to run a series of steps in parallel, but a few questions:\n\n1) None of these steps are inter-dependent. Why not run these as independent tests?\n\n2) Having dependencies between tests are an anti-pattern.  If there needs to be a series of setup tasks, with multiple test points within a scenario, why not just code these as a larger test with a number of asserts?","commit_id":"47fccbc0f75562c8f02fd9fee584ccdc4082e715"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"78019feb0f4ad6c57c49a729a67fa310326e1275","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    These tests are structured monolithically as one"},{"line_number":22,"context_line":"    test with multiple steps to preserve ordering and ensure IPA state remains"},{"line_number":23,"context_line":"    consistent across different test runs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":"    def step_1_get_empty_commands(self):"},{"line_number":26,"context_line":"        response \u003d self.request(\u0027get\u0027, \u0027commands\u0027).json()"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_af4ff6bb","line":23,"in_reply_to":"9a1a9d01_dd20049e","updated":"2015-09-28 19:10:21.000000000","message":"Thanks. The steps aren\u0027t running in parallel, actually, they\u0027re synchronously executed one after the other.\n\n1) I think that\u0027s something that could be added as a different scenario, but since these are functional tests that are supposed to be capturing the actual execution of the agent, it makes sense to have multiple queries run one after the other. I believe when an agent\u0027s running it\u0027s more common to execute multiple functions against the running agent than execute a single unit, which our unit testing mostly takes care of anyway. Also, some steps are inter-dependent - see below.\n\n2) Dependencies are an anti-pattern for unit testing, most definitely. For functional testing, though, perhaps it\u0027s a different story. If we can\u0027t control the execution order as is done here, the state of the agent and what we expect from our asserts will change, since a few of these API calls have side effects. For example, steps 3 and 4 require that step 2 succeeds. To me, this already is a large test with many asserts - it\u0027s just decomposed into separate functions for readability. It actually shows up as 1 big test if you look at the execution (tox -efunc).","commit_id":"47fccbc0f75562c8f02fd9fee584ccdc4082e715"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"e2c907c7c6a74117f78394109a278ca973d50fbd","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    These tests are structured monolithically as one"},{"line_number":22,"context_line":"    test with multiple steps to preserve ordering and ensure IPA state remains"},{"line_number":23,"context_line":"    consistent across different test runs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":"    def step_1_get_empty_commands(self):"},{"line_number":26,"context_line":"        response \u003d self.request(\u0027get\u0027, \u0027commands\u0027).json()"},{"line_number":27,"context_line":"        self.assertEqual({\u0027commands\u0027: []}, response)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a1a9d01_76068e1e","line":24,"updated":"2015-09-28 23:29:14.000000000","message":"Sorry, I said \"parallel\" when I meant \"in series\".  Brain fade :)\n\nI do think it\u0027s better that we have a longer test, with a number of agent calls, that tests a particular scenario that have ordering between tests. I would still prefer to see judicious use of setUp() and tearDown() (or a setup() callout in the test if there needs to be differences between a set of tests) to ensure a known state of the environment under test, and have each test be independently runnable.\n\nAs this is a single test, with multiple facets, it\u0027s straddling the line.  My only concern is that the dependencies between the steps might not be maintained in future modifications of this code. i.e. there\u0027s strong coupling between steps 2 and 3/4 that\u0027s implicit.  Perhaps add a comment to 3/4 saying that 2 needs to succeed so that future maintainers can see the relationship explicitly?\n\nIn any case, thanks for the discussion.  I\u0027m not going to -1 this, as some of my comments are personal preference.","commit_id":"bef0d5bcc87faa455de1af760aa420c160e96c65"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"b84d27b7b6e75d1825061d222f6ff08546af6599","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    These tests are structured monolithically as one"},{"line_number":22,"context_line":"    test with multiple steps to preserve ordering and ensure IPA state remains"},{"line_number":23,"context_line":"    consistent across different test runs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":"    def step_1_get_empty_commands(self):"},{"line_number":26,"context_line":"        response \u003d self.request(\u0027get\u0027, \u0027commands\u0027).json()"},{"line_number":27,"context_line":"        self.assertEqual({\u0027commands\u0027: []}, response)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a1a9d01_126a9eee","line":24,"in_reply_to":"9a1a9d01_76068e1e","updated":"2015-09-29 16:05:35.000000000","message":"Thanks Michael.\n\nFeel free to email me or chat on IRC if you want to continue the discussion independent of the patch. I definitely agree with you in the case of unit testing. I\u0027ve only done something kind of like functional testing once for UI testing, so maybe I need a bit more context to comment intelligently.\n\nI\u0027ll add the comment you suggested.","commit_id":"bef0d5bcc87faa455de1af760aa420c160e96c65"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"cea310766d1029234f2b50e960d556635b817ca5","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def positive_get_post_command_steps(self):"},{"line_number":65,"context_line":"        for name in sorted(dir(self)):"},{"line_number":66,"context_line":"            if name.startswith(\u0027step\u0027):"},{"line_number":67,"context_line":"                yield name, getattr(self, name)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_positive_get_post_commands(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_5e51f334","line":66,"updated":"2015-09-29 20:31:33.000000000","message":"nit: maybe better for \u0027step_\u0027\n\nAlso what happens with step_12 in ordering?  I think need a custom sort key function.","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"0e2d84371312ed1c1086042ff68f202d7f190789","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def positive_get_post_command_steps(self):"},{"line_number":65,"context_line":"        for name in sorted(dir(self)):"},{"line_number":66,"context_line":"            if name.startswith(\u0027step\u0027):"},{"line_number":67,"context_line":"                yield name, getattr(self, name)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_positive_get_post_commands(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_244c329d","line":66,"in_reply_to":"9a1a9d01_5e51f334","updated":"2015-09-29 21:30:04.000000000","message":"Done, thanks for the help in IRC","commit_id":"3453574122306077576d9c7351ff905580bdbdb9"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"74362a06d972d6b4d515d41429df00a5f882749e","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Returns generator with test steps sorted by step number.\"\"\""},{"line_number":66,"context_line":"        steps_unsorted \u003d [step for step in dir(self)"},{"line_number":67,"context_line":"                         if step.startswith(\u0027step_\u0027)]"},{"line_number":68,"context_line":"        steps \u003d sorted(steps_unsorted, key\u003dlambda s: s.split(\u0027_\u0027, 2)[1])"},{"line_number":69,"context_line":"        for name in steps:"},{"line_number":70,"context_line":"            yield name, getattr(self, name)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a1a9d01_2f840b5c","line":68,"updated":"2015-09-29 21:37:49.000000000","message":"need to wrap it in an int()\n\n a \u003d [\u00271\u0027, \u00272\u0027, \u002712\u0027]\n \u003e\u003e\u003e sorted(a)\n [\u00271\u0027, \u002712\u0027, \u00272\u0027]\n\nAnd maybe a comment to help people know what the heck is going on here :)","commit_id":"a06728ff4bfba15039ec6f632e29053dba43c191"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"410c9613feebf295985d5a6ed88de321b4cae613","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Returns generator with test steps sorted by step number.\"\"\""},{"line_number":66,"context_line":"        steps_unsorted \u003d [step for step in dir(self)"},{"line_number":67,"context_line":"                         if step.startswith(\u0027step_\u0027)]"},{"line_number":68,"context_line":"        steps \u003d sorted(steps_unsorted, key\u003dlambda s: s.split(\u0027_\u0027, 2)[1])"},{"line_number":69,"context_line":"        for name in steps:"},{"line_number":70,"context_line":"            yield name, getattr(self, name)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a1a9d01_0fb7c7f5","line":68,"in_reply_to":"9a1a9d01_2f840b5c","updated":"2015-09-29 21:45:08.000000000","message":"Whoops! Good catch, fixed + added comment.","commit_id":"a06728ff4bfba15039ec6f632e29053dba43c191"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2c13ff388179d19f390012672b46e5473dd062a1","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def positive_get_post_command_steps(self):"},{"line_number":65,"context_line":"        \"\"\"Returns generator with test steps sorted by step number.\"\"\""},{"line_number":66,"context_line":"        steps_unsorted \u003d [step for step in dir(self)"},{"line_number":67,"context_line":"                         if step.startswith(\u0027step_\u0027)]"},{"line_number":68,"context_line":"        # The lambda retrieves the step number from the function name and casts"},{"line_number":69,"context_line":"        # it to an integer. This is necessary, otherwise a lexicographic sort"},{"line_number":70,"context_line":"        # would return [\u0027step_1\u0027, \u0027step_12\u0027, \u0027step_3\u0027] after sorting instead of"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_58b5fe7b","line":67,"updated":"2015-10-02 15:54:28.000000000","message":"indentation is a bit off, needs one more space","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def positive_get_post_command_steps(self):"},{"line_number":65,"context_line":"        \"\"\"Returns generator with test steps sorted by step number.\"\"\""},{"line_number":66,"context_line":"        steps_unsorted \u003d [step for step in dir(self)"},{"line_number":67,"context_line":"                         if step.startswith(\u0027step_\u0027)]"},{"line_number":68,"context_line":"        # The lambda retrieves the step number from the function name and casts"},{"line_number":69,"context_line":"        # it to an integer. This is necessary, otherwise a lexicographic sort"},{"line_number":70,"context_line":"        # would return [\u0027step_1\u0027, \u0027step_12\u0027, \u0027step_3\u0027] after sorting instead of"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_b7445bb7","line":67,"in_reply_to":"9a1a9d01_58b5fe7b","updated":"2015-10-02 23:04:21.000000000","message":"Fixed","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"85c0c2c5c54f0f3281372360450f2dc5f1eb7ee6","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        # [\u0027step_1\u0027, \u0027step_3\u0027, \u0027step_12\u0027]."},{"line_number":72,"context_line":"        steps \u003d sorted(steps_unsorted, key\u003dlambda s: int(s.split(\u0027_\u0027, 2)[1]))"},{"line_number":73,"context_line":"        for name in steps:"},{"line_number":74,"context_line":"            yield name, getattr(self, name)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_positive_get_post_commands(self):"},{"line_number":77,"context_line":"        for name, step in self.positive_get_post_command_steps():"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_34f0dbfd","line":74,"updated":"2015-09-30 00:11:32.000000000","message":"Is there a plan to use \u0027name\u0027 in the future? It doesn\u0027t seem to be used by the caller.","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"91161980a38cd67a0f552efe93fcaa96443db73f","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        # [\u0027step_1\u0027, \u0027step_3\u0027, \u0027step_12\u0027]."},{"line_number":72,"context_line":"        steps \u003d sorted(steps_unsorted, key\u003dlambda s: int(s.split(\u0027_\u0027, 2)[1]))"},{"line_number":73,"context_line":"        for name in steps:"},{"line_number":74,"context_line":"            yield name, getattr(self, name)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_positive_get_post_commands(self):"},{"line_number":77,"context_line":"        for name, step in self.positive_get_post_command_steps():"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_ede11a2b","line":74,"in_reply_to":"9a1a9d01_34f0dbfd","updated":"2015-09-30 15:35:10.000000000","message":"Maybe for logging.","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        # [\u0027step_1\u0027, \u0027step_3\u0027, \u0027step_12\u0027]."},{"line_number":72,"context_line":"        steps \u003d sorted(steps_unsorted, key\u003dlambda s: int(s.split(\u0027_\u0027, 2)[1]))"},{"line_number":73,"context_line":"        for name in steps:"},{"line_number":74,"context_line":"            yield name, getattr(self, name)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_positive_get_post_commands(self):"},{"line_number":77,"context_line":"        for name, step in self.positive_get_post_command_steps():"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_d726af15","line":74,"in_reply_to":"9a1a9d01_ede11a2b","updated":"2015-10-02 23:04:21.000000000","message":"Actually, I\u0027ll just take it out for now, we can easily add it back if additional logging is needed.","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2c13ff388179d19f390012672b46e5473dd062a1","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            yield name, getattr(self, name)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_positive_get_post_commands(self):"},{"line_number":77,"context_line":"        for name, step in self.positive_get_post_command_steps():"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                step()"},{"line_number":80,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_78ba4270","line":77,"updated":"2015-10-02 15:54:28.000000000","message":"name is not used","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            yield name, getattr(self, name)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_positive_get_post_commands(self):"},{"line_number":77,"context_line":"        for name, step in self.positive_get_post_command_steps():"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                step()"},{"line_number":80,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_f72173fd","line":77,"in_reply_to":"9a1a9d01_78ba4270","updated":"2015-10-02 23:04:21.000000000","message":"Fixed","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2c13ff388179d19f390012672b46e5473dd062a1","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                step()"},{"line_number":80,"context_line":"            except Exception as e:"},{"line_number":81,"context_line":"                self.fail(\"{} failed ({}: {})\".format(step, type(e), e))"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_18b0e68a","line":81,"updated":"2015-10-02 15:54:28.000000000","message":"please don\u0027t catch exception here, just let is go through","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"fafd2dd56044459da070370e5591539107ae294a","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                step()"},{"line_number":80,"context_line":"            except Exception as e:"},{"line_number":81,"context_line":"                self.fail(\"{} failed ({}: {})\".format(step, type(e), e))"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_972037f9","line":81,"in_reply_to":"9a1a9d01_18b0e68a","updated":"2015-10-02 23:04:21.000000000","message":"Done","commit_id":"672e928b8cddace9a81bb9c730d1705d9232703b"}],"tox.ini":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"f91dad314a8569c013d5ae13614aa79e5ebb7d07","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  VIRTUAL_ENV\u003d{envdir}"},{"line_number":27,"context_line":"  OS_TEST_PATH\u003d./ironic_python_agent/tests/functional"},{"line_number":28,"context_line":"  TEST_PORT\u003d9999"},{"line_number":29,"context_line":"  IPA_WAIT_TRIES\u003d100"},{"line_number":30,"context_line":"commands \u003d"},{"line_number":31,"context_line":"  python setup.py testr --slowest --testr-args\u003d\u0027{posargs:}\u0027"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-properties","patch_set":4,"id":"ba15a1d1_6afbe7c1","line":29,"updated":"2015-09-16 23:57:52.000000000","message":"Are we not concerned that waiting 200 seconds for IPA to start could mask some kind of bug?","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"73a09eac00a6efff46cc3d81cc03ae760f8c83da","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  VIRTUAL_ENV\u003d{envdir}"},{"line_number":27,"context_line":"  OS_TEST_PATH\u003d./ironic_python_agent/tests/functional"},{"line_number":28,"context_line":"  TEST_PORT\u003d9999"},{"line_number":29,"context_line":"  IPA_WAIT_TRIES\u003d100"},{"line_number":30,"context_line":"commands \u003d"},{"line_number":31,"context_line":"  python setup.py testr --slowest --testr-args\u003d\u0027{posargs:}\u0027"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-properties","patch_set":4,"id":"ba15a1d1_e857d155","line":29,"in_reply_to":"ba15a1d1_28aaa985","updated":"2015-09-17 19:10:52.000000000","message":"aha, I misread that initially as 1 second of sleep per try","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1d2679d03b17121e16308e83fea4d787f029a293","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  VIRTUAL_ENV\u003d{envdir}"},{"line_number":27,"context_line":"  OS_TEST_PATH\u003d./ironic_python_agent/tests/functional"},{"line_number":28,"context_line":"  TEST_PORT\u003d9999"},{"line_number":29,"context_line":"  IPA_WAIT_TRIES\u003d100"},{"line_number":30,"context_line":"commands \u003d"},{"line_number":31,"context_line":"  python setup.py testr --slowest --testr-args\u003d\u0027{posargs:}\u0027"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-properties","patch_set":4,"id":"ba15a1d1_28aaa985","line":29,"in_reply_to":"ba15a1d1_6afbe7c1","updated":"2015-09-17 19:08:31.000000000","message":"It actually only waits 10 seconds at a maximum. In base.py we sleep 0.1 seconds after each try. \n\nBefore, this was called \"IPA_WAIT_TIME\" but it actually was the number of tries (not the wait time), so I changed the name to be more descriptive.","commit_id":"16e45b3a5344afc3f8cc1f8a07c6aa9a776a5e81"}]}