)]}'
{"nova/tests/unit/api_samples_test_base/test_compare_result.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"98c9cfd2d98257b34f78a2bdd4bf0d3a760ec2f2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from nova.tests.functional import api_samples_test_base"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"class TestCompareResult(api_samples_test_base.ApiSampleTestBase):"},{"line_number":19,"context_line":"    \"\"\"Provide test coverage for result comparison logic in functional tests\"\"\""},{"line_number":20,"context_line":"    # TODO(auggy) fails to load without this, need to double check"},{"line_number":21,"context_line":"    api_major_version \u003d \u0027v2\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_e1d9a184","line":18,"updated":"2016-01-19 20:11:40.000000000","message":"This was a different direction than I was expecting, which I guess works, but I\u0027d be a little more comfortable if we didn\u0027t inherit from it, and instead instantiated it from the outside.","commit_id":"770666f8766074fc7df99c87a0ed9fc86e249b9f"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"bf4eb69d2ac516cf6740da582e3d11b351e676df","unresolved":false,"context_lines":[{"line_number":48,"context_line":"                subs\u003dself._get_regexes(),"},{"line_number":49,"context_line":"                expected\u003dsample_data,"},{"line_number":50,"context_line":"                result\u003dresponse_data,"},{"line_number":51,"context_line":"                result_str\u003d\"Test\")"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_template_strings_match(self):"},{"line_number":54,"context_line":"        \"\"\"compare 2 template strings (contain %) that match\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_2207b81d","line":51,"updated":"2016-01-19 21:06:19.000000000","message":"I think \u0027with ExpectedException(api_samples_test_base.NoMatch):\u0027 would probably result in more clarity here and below.","commit_id":"770666f8766074fc7df99c87a0ed9fc86e249b9f"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # required by ApiSampleTestBase"},{"line_number":33,"context_line":"        ast_instance.api_major_version \u003d \u0027v2\u0027"},{"line_number":34,"context_line":"        ast_instance._project_id \u003d \u0027True\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        # automagically create magic methods usually handled by test classes"},{"line_number":37,"context_line":"        ast_instance.compute \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_8c9a8b13","line":34,"updated":"2016-02-03 14:26:43.000000000","message":"seems ugly to set a private var like this, is there an alternative?","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # required by ApiSampleTestBase"},{"line_number":33,"context_line":"        ast_instance.api_major_version \u003d \u0027v2\u0027"},{"line_number":34,"context_line":"        ast_instance._project_id \u003d \u0027True\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        # automagically create magic methods usually handled by test classes"},{"line_number":37,"context_line":"        ast_instance.compute \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_080f5ee2","line":34,"in_reply_to":"5a5ae5dd_8c9a8b13","updated":"2016-02-03 16:38:37.000000000","message":"I\u0027m open to suggestions, but this is the only example we have of this class being instantiated like this. Typically this class is inherited via other test classes, so this is not a typical use case.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.assertEqual("},{"line_number":58,"context_line":"                expected\u003dNone,"},{"line_number":59,"context_line":"                observed\u003dresult,"},{"line_number":60,"context_line":"                 message\u003d\u0027Check _compare_result of 2 bare strings\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def test_bare_strings_no_match(self):"},{"line_number":63,"context_line":"        \"\"\"check 2 bare strings that don\u0027t match\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_0c5c5b4a","line":60,"range":{"start_line":60,"start_character":16,"end_line":60,"end_character":17},"updated":"2016-02-03 14:26:43.000000000","message":"nit: extra space here, not sure why pep8 didn\u0027t pick it up?\n\nin general you seem to be getting away with some funky indentation... style is to indent by 4, 8 or visually. Not by a multiple of four that nearly lines up visually. My very personal taste is to avoid visual indent, our 80char line limit and proclivity for long function names makes it annoying.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.assertEqual("},{"line_number":58,"context_line":"                expected\u003dNone,"},{"line_number":59,"context_line":"                observed\u003dresult,"},{"line_number":60,"context_line":"                 message\u003d\u0027Check _compare_result of 2 bare strings\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def test_bare_strings_no_match(self):"},{"line_number":63,"context_line":"        \"\"\"check 2 bare strings that don\u0027t match\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_e8c76243","line":60,"in_reply_to":"5a5ae5dd_0c5c5b4a","updated":"2016-02-03 16:38:37.000000000","message":"Sure I\u0027ll fix the indentation, totally missed it cuz pep8 didn\u0027t complain. We should probably update pep8 so it catches that.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def test_template_strings_match(self):"},{"line_number":74,"context_line":"        \"\"\"compare 2 template strings (contain %) that match\"\"\""},{"line_number":75,"context_line":"        template_data \u003d u\u0027%(id)s\u0027"},{"line_number":76,"context_line":"        response_data \u003d u\u0027858f295a-8543-45fa-804a-08f8356d616d\u0027"},{"line_number":77,"context_line":"        result \u003d self.ast._compare_result(subs\u003dself.ast._get_regexes(),"},{"line_number":78,"context_line":"                                      expected\u003dtemplate_data,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_ef68d1bc","line":75,"updated":"2016-02-03 14:26:43.000000000","message":"Where\u0027s the UUID that gets filled in here? Is it hardcoded in ApiSampleTestBase? If possible it\u0027d be nice to refer to that for response_data instead of hardcoding.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def test_template_strings_match(self):"},{"line_number":74,"context_line":"        \"\"\"compare 2 template strings (contain %) that match\"\"\""},{"line_number":75,"context_line":"        template_data \u003d u\u0027%(id)s\u0027"},{"line_number":76,"context_line":"        response_data \u003d u\u0027858f295a-8543-45fa-804a-08f8356d616d\u0027"},{"line_number":77,"context_line":"        result \u003d self.ast._compare_result(subs\u003dself.ast._get_regexes(),"},{"line_number":78,"context_line":"                                      expected\u003dtemplate_data,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_e879a230","line":75,"in_reply_to":"5a5ae5dd_ef68d1bc","updated":"2016-02-03 16:38:37.000000000","message":"There\u0027s no actual UUID that is compared here. I could put anything in there as long as it matches the regex for the \"id\" field in the template. I prefer explicit hard coding in this case as it makes it clear what\u0027s being tested. I can put a comment to clarify that.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                                      result\u003dresponse_data,"},{"line_number":96,"context_line":"                                      result_str\u003d\"Test\")"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    # TODO(auggy): _compare_result needs a consistent return value"},{"line_number":99,"context_line":"    def test_bare_int_match(self):"},{"line_number":100,"context_line":"        \"\"\"check 2 bare ints that match\"\"\""},{"line_number":101,"context_line":"        sample_data \u003d 42"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_0fcf6d57","line":98,"updated":"2016-02-03 14:26:43.000000000","message":"I don\u0027t understand how to action this TODO. OK, I think I see now but some mention of None vs response_data would be nice.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                                      result\u003dresponse_data,"},{"line_number":96,"context_line":"                                      result_str\u003d\"Test\")"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    # TODO(auggy): _compare_result needs a consistent return value"},{"line_number":99,"context_line":"    def test_bare_int_match(self):"},{"line_number":100,"context_line":"        \"\"\"check 2 bare ints that match\"\"\""},{"line_number":101,"context_line":"        sample_data \u003d 42"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_a81deabe","line":98,"in_reply_to":"5a5ae5dd_0fcf6d57","updated":"2016-02-03 16:38:37.000000000","message":"Yeah, the method is confusing hence my comment. In some cases if a thing matches, it returns that matched value. In other cases, it just returns None, because if it didn\u0027t match, it would throw an exception. This is why we\u0027re writing unit tests around this method because these are things we need to fix. I\u0027ll update my comment so it\u0027s more clear.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                expected\u003dNone,"},{"line_number":109,"context_line":"                observed\u003dresult,"},{"line_number":110,"context_line":"                message\u003d\u0027Check _compare_result of 2 bare ints\u0027)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    # TODO(auggy): _compare_result should throw NoMatch for ints"},{"line_number":113,"context_line":"    # this gets to objectify() and it throws a TypeError"},{"line_number":114,"context_line":"    # versions are a case where ints a compared,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_4fb565cb","line":111,"updated":"2016-02-03 14:26:43.000000000","message":"it\u0027s around this point that I notice a couple of repeated patterns. You\u0027re checking for things that work or don\u0027t, with very similar code each time. Would test_bare_int_match be easier to read as this or this?\n\n  self.assertGoodCompare(42, 42)\n  self.assertGoodCompare(expect\u003d42, actual\u003d42)\n\nSimilarly test_bare_int_no_match:\n\n  self.assertBadCompare(42, 43, TypeError)\n  self.assertBadCompare(expect\u003d42, actual\u003d43, error_class\u003dTypeError)\n\nThe need for the helper method wouldn\u0027t be as pressing if you weren\u0027t naming positional arguments every time. This definitely avoids any confusion and was recommended by Raymond Hettinger. However... it also makes the code very longhand. It\u0027s not wrong but using 10 lines instead of 1, with the pattern repetition, does make me wince a bit.\n\nIt\u0027s possible I\u0027m missing more nuances like the inconsistent _compare_result return values which makes this impossible, if so ignore me.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                expected\u003dNone,"},{"line_number":109,"context_line":"                observed\u003dresult,"},{"line_number":110,"context_line":"                message\u003d\u0027Check _compare_result of 2 bare ints\u0027)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    # TODO(auggy): _compare_result should throw NoMatch for ints"},{"line_number":113,"context_line":"    # this gets to objectify() and it throws a TypeError"},{"line_number":114,"context_line":"    # versions are a case where ints a compared,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_0b6e78c9","line":111,"in_reply_to":"5a5ae5dd_4fb565cb","updated":"2016-02-03 16:38:37.000000000","message":"I tend to prefer named arguments even in this case because I tend to switch expected and observed (I also have problems telling my right from my left). Also most of the time with unit tests, you\u0027re looking at a single test because you\u0027re debugging the one that failed, not reading the whole file. I also think it adds clarity because in many cases the expected result is \"None\" so I want to add clarity to that. Once we decide what _compare_result should consistently return, I\u0027ll take your comments into consideration regarding the named arguments.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    # TODO(auggy): _compare_result should throw NoMatch for ints"},{"line_number":113,"context_line":"    # this gets to objectify() and it throws a TypeError"},{"line_number":114,"context_line":"    # versions are a case where ints a compared,"},{"line_number":115,"context_line":"    # currently if the version in the json sample doesn\u0027t match what\u0027s in the"},{"line_number":116,"context_line":"    # template, this throws a confusing \"TypeError\""},{"line_number":117,"context_line":"    def test_bare_int_no_match(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_525c2e25","line":114,"range":{"start_line":114,"start_character":37,"end_line":114,"end_character":38},"updated":"2016-02-03 14:26:43.000000000","message":"femtonit: are","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"10675c318ed4d04f3977bc3f07804d659f60ee5b","unresolved":false,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_list_match_extra_item_template(self):"},{"line_number":345,"context_line":"        \"\"\"check extra list items in template \"\"\""},{"line_number":346,"context_line":"        template_data \u003d {"},{"line_number":347,"context_line":"            u\u0027links\u0027:"},{"line_number":348,"context_line":"            ["},{"line_number":349,"context_line":"                {"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_b20322b8","line":346,"updated":"2016-02-03 14:26:43.000000000","message":"Since this is getting big, might make more sense to do:\n\n  template_data \u003d copy.deep_copy(response_data)\n  template_data[u\u0027links\u0027].append(u\u0027foo\u0027)\n\nOtherwise the test reader has to diff by eye","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":17714,"name":"Augustina Ragwitz","email":"auggy@cpan.org","username":"mmmpork"},"change_message_id":"441f7712ad18ac710c4e5669de6d82434b50dc8b","unresolved":false,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_list_match_extra_item_template(self):"},{"line_number":345,"context_line":"        \"\"\"check extra list items in template \"\"\""},{"line_number":346,"context_line":"        template_data \u003d {"},{"line_number":347,"context_line":"            u\u0027links\u0027:"},{"line_number":348,"context_line":"            ["},{"line_number":349,"context_line":"                {"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a5ae5dd_4b18f084","line":346,"in_reply_to":"5a5ae5dd_b20322b8","updated":"2016-02-03 16:38:37.000000000","message":"The structure is the same but the values are different enough that a copy doesn\u0027t make sense here. I\u0027ll put a comment that makes the difference more clear.","commit_id":"dac235dac8ce8d5fd7984913396713fa3de00763"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4ce45e3d400ab546cdf3e96cedaeaafb9f72fe29","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        sample_data \u003d u\u0027foo\u0027"},{"line_number":76,"context_line":"        response_data \u003d u\u0027bar\u0027"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        with testtools.ExpectedException(api_samples_test_base.NoMatch):"},{"line_number":79,"context_line":"            self.ast._compare_result("},{"line_number":80,"context_line":"                    subs\u003dself.ast._get_regexes(),"},{"line_number":81,"context_line":"                    expected\u003dsample_data,"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a5ae5dd_c58531cf","line":78,"updated":"2016-02-05 19:15:00.000000000","message":"Nice, I was not aware of this context manager.","commit_id":"95cb626c6e8be0f734985778611f9073154ebeca"}]}