)]}'
{"ceilometer/tests/test_notification.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"dda8bb4f8b7b733f28b065ae9764d2d0bb4ca844","unresolved":false,"context_lines":[{"line_number":396,"context_line":"        notifier.info(context.RequestContext(), \u0027compute.instance.create.end\u0027,"},{"line_number":397,"context_line":"                      TEST_NOTICE_PAYLOAD)"},{"line_number":398,"context_line":"        start \u003d timeutils.utcnow()"},{"line_number":399,"context_line":"        with mock.patch(\u0027hmac.new\u0027) as fake_hmac:"},{"line_number":400,"context_line":"            while timeutils.delta_seconds(start, timeutils.utcnow()) \u003c 600:"},{"line_number":401,"context_line":"                if (len(self.publisher.samples) \u003e\u003d self.expected_samples and"},{"line_number":402,"context_line":"                        len(self.publisher.events) \u003e\u003d self.expected_events):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_992c0f4c","line":399,"updated":"2015-08-04 08:03:54.000000000","message":"I\u0027m not sure that still test something, hmac.new is now mocked only during the timeutils processing and the eventlet.sleep(0), if eventlet process the notification before with call eventlet.sleep(0), the hmac.new will be not mocked.\n\nSo the test will always success.\n\nI think the test fail sometime to do the usage of the global conf object.","commit_id":"3802a4c7c8cd6bb986bd918f9fbd1fc84ba03ed4"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"b183e269bb72515d96b2861cfe021cd7e3254c7d","unresolved":false,"context_lines":[{"line_number":396,"context_line":"        notifier.info(context.RequestContext(), \u0027compute.instance.create.end\u0027,"},{"line_number":397,"context_line":"                      TEST_NOTICE_PAYLOAD)"},{"line_number":398,"context_line":"        start \u003d timeutils.utcnow()"},{"line_number":399,"context_line":"        with mock.patch(\u0027hmac.new\u0027) as fake_hmac:"},{"line_number":400,"context_line":"            while timeutils.delta_seconds(start, timeutils.utcnow()) \u003c 600:"},{"line_number":401,"context_line":"                if (len(self.publisher.samples) \u003e\u003d self.expected_samples and"},{"line_number":402,"context_line":"                        len(self.publisher.events) \u003e\u003d self.expected_events):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_e66802f8","line":399,"in_reply_to":"1a4dcd0f_992c0f4c","updated":"2015-08-04 12:02:23.000000000","message":"which global conf object? what if i wrap the notifier.info line above as well?","commit_id":"3802a4c7c8cd6bb986bd918f9fbd1fc84ba03ed4"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"3650c023900f8f9d2191157f12e48cef1baf1d2b","unresolved":false,"context_lines":[{"line_number":396,"context_line":"        notifier.info(context.RequestContext(), \u0027compute.instance.create.end\u0027,"},{"line_number":397,"context_line":"                      TEST_NOTICE_PAYLOAD)"},{"line_number":398,"context_line":"        start \u003d timeutils.utcnow()"},{"line_number":399,"context_line":"        with mock.patch(\u0027hmac.new\u0027) as fake_hmac:"},{"line_number":400,"context_line":"            while timeutils.delta_seconds(start, timeutils.utcnow()) \u003c 600:"},{"line_number":401,"context_line":"                if (len(self.publisher.samples) \u003e\u003d self.expected_samples and"},{"line_number":402,"context_line":"                        len(self.publisher.events) \u003e\u003d self.expected_events):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_a931660d","line":399,"in_reply_to":"1a4dcd0f_e66802f8","updated":"2015-08-04 14:09:07.000000000","message":"Your new patchset fixes the eventlet scheduling issue.\n\nBut I\u0027m unsure this change fixes the referenced bug, in Ceilometer we call hmac.new only when telemetry_secret is not None, so if the test fail this is because something else have changed the configuration option or the \u0027hmac.new\u0027 is not called by our code.\n\nIf you think this because hmac.new is used somewhere here outside of ceilometer, you should mock it like this:\n\n    @mock.patch(\"ceilometer.publisher.utils.hmac.new\")\n\nOtherwise, this is perhaps an eventlet / oslo.config.cfg.CONF issue:\n\nIn fact, self.CONF is just a fixture that use the global configuration object \u0027oslo.config.cfg.CONF\u0027.\n\nBecause of eventlet, another test can change the configuration object between the line override the option (L390), and when \nthe notification listener use it to control the message signature. Another test can be run when we call \u0027self.srv.start()\u0027, \u0027notifier.info()\u0027, or eventlet.sleep(0), because they allow greenlet switch.\n\nSomeother tests that relies on this configuration option:\n\n  https://github.com/openstack/ceilometer/blob/master/ceilometer/tests/test_collector.py#L49\nhttps://github.com/openstack/ceilometer/blob/master/ceilometer/tests/publisher/test_udp.py#L114\n\nI guess, other tests won\u0027t fail because their don\u0027t use eventlet.sleep(0)\n\nIf that the root cause of this bug, one more times thanks to eventlet to make thing non-deterministic...\n\nFor sure your fix should reduce a lot the occurrence of the bug in this case.","commit_id":"3802a4c7c8cd6bb986bd918f9fbd1fc84ba03ed4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"56df9abd29b6d33fd8274415561188d027874922","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                      TEST_NOTICE_PAYLOAD)"},{"line_number":398,"context_line":"        start \u003d timeutils.utcnow()"},{"line_number":399,"context_line":"        with mock.patch(\u0027hmac.new\u0027) as fake_hmac:"},{"line_number":400,"context_line":"            while timeutils.delta_seconds(start, timeutils.utcnow()) \u003c 600:"},{"line_number":401,"context_line":"                if (len(self.publisher.samples) \u003e\u003d self.expected_samples and"},{"line_number":402,"context_line":"                        len(self.publisher.events) \u003e\u003d self.expected_events):"},{"line_number":403,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_d42fbe9f","line":400,"updated":"2015-08-04 07:51:00.000000000","message":"Is there any possible that the notification process has been broken, but this test can pass for timeout with 600s ?\n\nmaybe add this is better:\n\nself.assertEqual(self.expected_samples, len(self.publisher.samples))\n\nbut, other tests will test this too ;)","commit_id":"3802a4c7c8cd6bb986bd918f9fbd1fc84ba03ed4"}]}