)]}'
{"nova/tests/test_notifications.py":[{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"3c8cee599be0b487ece527ecbf1de10a4ed282ad","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        exception \u003d None"},{"line_number":97,"context_line":"        try:"},{"line_number":98,"context_line":"            # Get a real exception with a call stack."},{"line_number":99,"context_line":"            raise Exception(\"junk\")"},{"line_number":100,"context_line":"        except Exception, e:"},{"line_number":101,"context_line":"            exception \u003d e"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALH%2F%2FWNk%3D","line":99,"updated":"2012-09-19 18:37:01.000000000","message":"My pet peeve is using Exception for testing. Can you use test.TestingException instead?\n\nIt\u0027s not likely to cause a problem in this case because of how extremely simple it is, but it\u0027ll make my life easier when I audit the testing code periodically to find uses of Exception that actually are risky.","commit_id":"b16225bd60fca02de5ae116b3ea795c6a1829594"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"75b7c5648ebba91431c3f79ab520bf137cd30b46","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        exception \u003d None"},{"line_number":97,"context_line":"        try:"},{"line_number":98,"context_line":"            # Get a real exception with a call stack."},{"line_number":99,"context_line":"            raise Exception(\"junk\")"},{"line_number":100,"context_line":"        except Exception, e:"},{"line_number":101,"context_line":"            exception \u003d e"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALH%2F%2FWH8%3D","line":99,"in_reply_to":"AAAALH%2F%2FWK8%3D","updated":"2012-09-19 19:45:54.000000000","message":"It\u0027s not a problem with your test here, but the problem is that Exception is the base class of (almost) every exception.\n\nIf your test fails because of some other reason (like a bug in your test), then you\u0027re only testing that something failed, not that the right thing failed.\n\nI\u0027ve found bugs that were caused by incorrectly asserting that Exception was raised. (See commit 8813ab185d0b6ad1c111e7f9e346e2ce91c8113b and the referenced other reviews).\n\nThere is a brief snippet about exceptions and testing in nova/testing/README.rst. While it discusses assertRaises(), that isn\u0027t the only source of problems. I\u0027ll work on expanding that section.\n\nAgain, your code here is fine (even with Exception) since you\u0027re looking for any exception. I only asked for it to be changed so it would be easier to audit the code for the legitimate sources of bugs.","commit_id":"b16225bd60fca02de5ae116b3ea795c6a1829594"},{"author":{"_account_id":688,"name":"Sandy Walsh","email":"sandy.walsh@rackspace.com","username":"sandy-walsh"},"change_message_id":"a614ced5e8110f86e506de248f3edde90d8da975","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        exception \u003d None"},{"line_number":97,"context_line":"        try:"},{"line_number":98,"context_line":"            # Get a real exception with a call stack."},{"line_number":99,"context_line":"            raise Exception(\"junk\")"},{"line_number":100,"context_line":"        except Exception, e:"},{"line_number":101,"context_line":"            exception \u003d e"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALH%2F%2FWK8%3D","line":99,"in_reply_to":"AAAALH%2F%2FWNk%3D","updated":"2012-09-19 19:08:34.000000000","message":"Made the change, but I\u0027m not clear on why it\u0027s an issue? The test.TestingException class is no help since it has no comment in it explaining what the concern is. Perhaps that might help?","commit_id":"b16225bd60fca02de5ae116b3ea795c6a1829594"}]}