)]}'
{"placement/tests/unit/test_fixtures.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7a2a60cdbb74d915de764c56f9f97a52f335bb3e","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def test_register_opts_fail(self):"},{"line_number":28,"context_line":"        pf \u003d placement.PlacementFixture(register_opts\u003dFalse)"},{"line_number":29,"context_line":"        # NOTE(efried): We should assertRaises(oslo_config.cfg.NoSuchOptError,"},{"line_number":30,"context_line":"        # but don\u0027t want to drag in a req on oslo.config."},{"line_number":31,"context_line":"        exc \u003d self.assertRaises(Exception, pf.setUp)"},{"line_number":32,"context_line":"        self.assertEqual(\u0027NoSuchOptError\u0027, exc.args[0][0].__name__)"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_64830706","line":30,"updated":"2019-03-21 17:31:20.000000000","message":"why not? you already have it above and use it below.\n\nIn any case asserting generic Exception is bad news. That could be anything.","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"91448ced5abd031047176d9be2e2a1a3a30b8cb1","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def test_register_opts_fail(self):"},{"line_number":28,"context_line":"        pf \u003d placement.PlacementFixture(register_opts\u003dFalse)"},{"line_number":29,"context_line":"        # NOTE(efried): We should assertRaises(oslo_config.cfg.NoSuchOptError,"},{"line_number":30,"context_line":"        # but don\u0027t want to drag in a req on oslo.config."},{"line_number":31,"context_line":"        exc \u003d self.assertRaises(Exception, pf.setUp)"},{"line_number":32,"context_line":"        self.assertEqual(\u0027NoSuchOptError\u0027, exc.args[0][0].__name__)"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_c74f5d68","line":30,"in_reply_to":"5fc1f717_64830706","updated":"2019-03-21 18:08:12.000000000","message":"\u003e why not? you already have it above and use it below.\n\nYeah, duh, I looked in test-requirements rather than requirements and thought we were being careful to keep the import footprint small. But of course we need oslo.config. And I wrote this before I wrote the stuff below that uses it.\n\n \u003e In any case asserting generic Exception is bad news. That could be\n \u003e anything.\n\nRight, that\u0027s why I peeled NoSuchOptError out of it below. I still have to do something similar because the fixtures lib throws in an extra SetupError and then testtools packages both into MultipleExceptions. But at least I get to assert the exact exception class instead of the yucky string match.","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fbd12b3f35b4ad3f1452fec9464d926e0521d434","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        pf \u003d placement.PlacementFixture(register_opts\u003dFalse)"},{"line_number":29,"context_line":"        # NOTE(efried): We should assertRaises(oslo_config.cfg.NoSuchOptError,"},{"line_number":30,"context_line":"        # but don\u0027t want to drag in a req on oslo.config."},{"line_number":31,"context_line":"        exc \u003d self.assertRaises(Exception, pf.setUp)"},{"line_number":32,"context_line":"        self.assertEqual(\u0027NoSuchOptError\u0027, exc.args[0][0].__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def test_register_opts_already_registered(self):"},{"line_number":35,"context_line":"        conf_fixture \u003d self.useFixture(config_fixture.Config(cfg.ConfigOpts()))"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_04680385","line":32,"range":{"start_line":31,"start_character":8,"end_line":32,"end_character":67},"updated":"2019-03-21 17:24:41.000000000","message":"should possibly mention that this happens because the fixture setUp goes looking for the [placement_database] section. It\u0027s okay if it\u0027s empty (as shown in test_no_kwargs) but we blow up if the group isn\u0027t registered.","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"91448ced5abd031047176d9be2e2a1a3a30b8cb1","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        pf \u003d placement.PlacementFixture(register_opts\u003dFalse)"},{"line_number":29,"context_line":"        # NOTE(efried): We should assertRaises(oslo_config.cfg.NoSuchOptError,"},{"line_number":30,"context_line":"        # but don\u0027t want to drag in a req on oslo.config."},{"line_number":31,"context_line":"        exc \u003d self.assertRaises(Exception, pf.setUp)"},{"line_number":32,"context_line":"        self.assertEqual(\u0027NoSuchOptError\u0027, exc.args[0][0].__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def test_register_opts_already_registered(self):"},{"line_number":35,"context_line":"        conf_fixture \u003d self.useFixture(config_fixture.Config(cfg.ConfigOpts()))"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_67e6a9eb","line":32,"range":{"start_line":31,"start_character":8,"end_line":32,"end_character":67},"in_reply_to":"5fc1f717_04680385","updated":"2019-03-21 18:08:12.000000000","message":"Done","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7a2a60cdbb74d915de764c56f9f97a52f335bb3e","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def test_register_opts_already_registered(self):"},{"line_number":35,"context_line":"        conf_fixture \u003d self.useFixture(config_fixture.Config(cfg.ConfigOpts()))"},{"line_number":36,"context_line":"        conf.register_opts(conf_fixture.conf)"},{"line_number":37,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"},{"line_number":38,"context_line":"        pf \u003d placement.PlacementFixture("},{"line_number":39,"context_line":"            conf_fixture\u003dconf_fixture, register_opts\u003dFalse)"},{"line_number":40,"context_line":"        pf.setUp()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_c4bafbbe","line":37,"updated":"2019-03-21 17:31:20.000000000","message":"maybe assert that it\u0027s not there before you register, and there when you  do, and then there after you call the fixture","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"91448ced5abd031047176d9be2e2a1a3a30b8cb1","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def test_register_opts_already_registered(self):"},{"line_number":35,"context_line":"        conf_fixture \u003d self.useFixture(config_fixture.Config(cfg.ConfigOpts()))"},{"line_number":36,"context_line":"        conf.register_opts(conf_fixture.conf)"},{"line_number":37,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"},{"line_number":38,"context_line":"        pf \u003d placement.PlacementFixture("},{"line_number":39,"context_line":"            conf_fixture\u003dconf_fixture, register_opts\u003dFalse)"},{"line_number":40,"context_line":"        pf.setUp()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_87595536","line":37,"in_reply_to":"5fc1f717_c4bafbbe","updated":"2019-03-21 18:08:12.000000000","message":"Done","commit_id":"209bbe6592842f92b4bc64088c57937ea0cdcbe9"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"73d5a56a3e14c4ca31de87dee6ab78809a2382da","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        # registered, will raise NoSuchOptError. Then the fixtures code will"},{"line_number":34,"context_line":"        # raise fixtures.fixture.SetupError as a result, which we don\u0027t care"},{"line_number":35,"context_line":"        # about. And the test harness will package those together into a"},{"line_number":36,"context_line":"        # MultipleExceptions."},{"line_number":37,"context_line":"        exc \u003d self.assertRaises(runtest.MultipleExceptions, pf.setUp)"},{"line_number":38,"context_line":"        # The first raised exception was our NoSuchOptError."},{"line_number":39,"context_line":"        # NOTE(efried): Today, exc[0][0].group.name is \u0027placement_database\u0027 and"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_5d36dee9","line":36,"updated":"2019-03-21 19:10:57.000000000","message":"You might consider simplifying this to:\n\n    pf \u003d placement.PlacementFixture(register_opts\u003dFalse,db\u003dFalse)\n    self.assertRaises(cfg.NoSuchGroupError, pf.setUp)\n\nOtherwise what you\u0027re confirming with this is that NoSuchOptError was raised by placement.tests.fixtures.Database, not the PlacementFixture, which might be a layer of indirection that you don\u0027t want in a unit test.\n\nNoSuchGroupError happens from line 75 PlacementFixture. Your test is getting NoSuchOptError from generate_schema_create_all in the database fixture (and in very round about way at that, through some oslo_db fixtures).\n\nIt also avoids the MultipleExceptions thing, which might be confusing.","commit_id":"43598595f517ed70bf2d0143300b48eed9e05ec3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e3cd24aadfb7897d17c89fc4414737142a4d4fb6","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        # registered, will raise NoSuchOptError. Then the fixtures code will"},{"line_number":34,"context_line":"        # raise fixtures.fixture.SetupError as a result, which we don\u0027t care"},{"line_number":35,"context_line":"        # about. And the test harness will package those together into a"},{"line_number":36,"context_line":"        # MultipleExceptions."},{"line_number":37,"context_line":"        exc \u003d self.assertRaises(runtest.MultipleExceptions, pf.setUp)"},{"line_number":38,"context_line":"        # The first raised exception was our NoSuchOptError."},{"line_number":39,"context_line":"        # NOTE(efried): Today, exc[0][0].group.name is \u0027placement_database\u0027 and"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_5361ee38","line":36,"in_reply_to":"5fc1f717_5d36dee9","updated":"2019-03-21 21:36:25.000000000","message":"Yes, that\u0027s much better, thanks.","commit_id":"43598595f517ed70bf2d0143300b48eed9e05ec3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9e195cbcacc4df896ad4287e24c0d2b78a54c1ab","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def test_register_opts_fail(self):"},{"line_number":29,"context_line":"        pf \u003d placement.PlacementFixture(register_opts\u003dFalse, db\u003dFalse)"},{"line_number":30,"context_line":"        exc \u003d self.assertRaises(cfg.NoSuchGroupError, pf.setUp)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def test_register_opts_already_registered(self):"},{"line_number":33,"context_line":"        conf_fixture \u003d self.useFixture(config_fixture.Config(cfg.ConfigOpts()))"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_761c1877","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":14},"updated":"2019-03-21 22:54:58.000000000","message":"nit: this can go too since you\u0027re not using it, and it upsets pep8","commit_id":"bd57e8061acd2223b12a80c6ab99b9efe249e154"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9e195cbcacc4df896ad4287e24c0d2b78a54c1ab","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        conf.register_opts(conf_fixture.conf)"},{"line_number":36,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"},{"line_number":37,"context_line":"        pf \u003d placement.PlacementFixture("},{"line_number":38,"context_line":"            conf_fixture\u003dconf_fixture, register_opts\u003dFalse)"},{"line_number":39,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"},{"line_number":40,"context_line":"        pf.setUp()"},{"line_number":41,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_f9a8f9d9","line":38,"updated":"2019-03-21 22:54:58.000000000","message":"if you don\u0027t call db\u003dFalse here, then the error describe in the comment at line 40 below happens in here\n\noverall you probably want db\u003dFalse here, simply to remove descent through modules, but doing that highlights that we\u0027re gonna have these tests, then we should have tests on the Database fixture too.","commit_id":"bd57e8061acd2223b12a80c6ab99b9efe249e154"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9e195cbcacc4df896ad4287e24c0d2b78a54c1ab","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        pf \u003d placement.PlacementFixture("},{"line_number":38,"context_line":"            conf_fixture\u003dconf_fixture, register_opts\u003dFalse)"},{"line_number":39,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"},{"line_number":40,"context_line":"        pf.setUp()"},{"line_number":41,"context_line":"        self.assertIn(\u0027debug\u0027, conf_fixture.conf)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_197dc56d","line":40,"updated":"2019-03-21 22:54:58.000000000","message":"If db\u003dFalse is set here, we\u0027ll reach this line, and then things will fail because _check_required_opts() is called on the conf object, and [placement_database]/connection is required.\n\nadd something like the following after line 35 to not tickle that\n\n        conf_fixture.config(connection\u003d\u0027sqlite://\u0027, group\u003d\u0027placement_database\u0027)","commit_id":"bd57e8061acd2223b12a80c6ab99b9efe249e154"}]}