)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"28f11336b89f23af1e6ab21c091f22d031dbd723","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix ignoring the existent metadata"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fix the test_create_get_delete_snapshot_metadata from volume module"},{"line_number":10,"context_line":"to consider the metadata already cerated for a snapshot."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fix Bug1379460"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baa201ad_20a47656","line":10,"updated":"2014-10-10 14:06:28.000000000","message":"Typo: created!","commit_id":"0deb837bc9c0452c6f50d928246026ea5ffd4e5d"},{"author":{"_account_id":7179,"name":"Brad Pokorny","email":"brad_pokorny@symantec.com","username":"bpokorny"},"change_message_id":"6a884f77b9144e19146f6b063f7a36eb0923c94a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Fix the test_create_get_delete_snapshot_metadata from volume module"},{"line_number":10,"context_line":"to consider the metadata already created for a snapshot."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fix Bug1379460"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I78e6f0601d21dc25fa95dc35818c907f40a931a3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baa201ad_6c02f40c","line":12,"updated":"2014-10-10 18:19:56.000000000","message":"Change this to Closes-Bug: #1379460 so the link works properly.","commit_id":"a5fb2088fe315dc0956d59ca9e9f93d9aef7898d"},{"author":{"_account_id":7872,"name":"Marc Koderer","email":"marc@koderer.com","username":"mkoderer"},"change_message_id":"bccf8e5425da13e7df0a0490b90d8266f76185ff","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix ignoring the existent metadata"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fix the class test_snapshot_metadata tests from volume module to consider"},{"line_number":10,"context_line":"the metadata already created for a snapshot."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1379460"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"5afe65bd_24023e0c","line":10,"updated":"2015-06-03 09:59:21.000000000","message":"Please answer the \"why\" question here. Why is this fix needed.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"}],"tempest/api/volume/test_snapshot_metadata.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"28f11336b89f23af1e6ab21c091f22d031dbd723","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                    \"key3\": \"value3\"}"},{"line_number":46,"context_line":"        expected \u003d {\"key2\": \"value2\","},{"line_number":47,"context_line":"                    \"key3\": \"value3\"}"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        _, body \u003d self.client.create_snapshot_metadata(self.snapshot_id,"},{"line_number":50,"context_line":"                                                       metadata)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_005a1283","line":48,"updated":"2014-10-10 14:06:28.000000000","message":"no need to add this extra line","commit_id":"0deb837bc9c0452c6f50d928246026ea5ffd4e5d"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"28f11336b89f23af1e6ab21c091f22d031dbd723","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        # Get the metadata of the snapshot"},{"line_number":57,"context_line":"        _, body \u003d self.client.get_snapshot_metadata(self.snapshot_id)"},{"line_number":58,"context_line":"        self.assertEqual(metadata, body)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        # Delete one item metadata of the snapshot"},{"line_number":61,"context_line":"        self.client.delete_snapshot_metadata_item("},{"line_number":62,"context_line":"            self.snapshot_id, \"key1\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_80760205","line":59,"updated":"2014-10-10 14:06:28.000000000","message":"no need to add this extra line.","commit_id":"0deb837bc9c0452c6f50d928246026ea5ffd4e5d"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"c950031057cc26683d51d266ac9ba03307656499","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                                                       metadata)"},{"line_number":76,"context_line":"        # Append actual metadata to those used in the test"},{"line_number":77,"context_line":"        metadata.update(actual_metadata)"},{"line_number":78,"context_line":"        update.update(actual_metadata)"},{"line_number":79,"context_line":"        # Get the metadata of the snapshot"},{"line_number":80,"context_line":"        _, body \u003d self.client.get_snapshot_metadata(self.snapshot_id)"},{"line_number":81,"context_line":"        self.assertEqual(metadata, body)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_8aa62ac0","line":78,"updated":"2014-12-08 15:31:02.000000000","message":"So for this test and the one below I\u0027d rather not see update include any preexisting metadata. If you want to still do an assertEqual below after the updates that\u0027s fine, although you could do something like an assertIn or something instead. \n\nBut, I think it\u0027s incorrect to include actual_metadata in the update payload. Just move this line to below the update call and everything should work the same.","commit_id":"21edcb3ef23f1cc31162c1ccc81ec41009228692"},{"author":{"_account_id":13553,"name":"Lucas Ribeiro","email":"lucasribeiro1990@gmail.com","username":"lucasribeiro1990"},"change_message_id":"882ce677686eb474bb5dd53c91c5719f506f668e","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                                                       metadata)"},{"line_number":76,"context_line":"        # Append actual metadata to those used in the test"},{"line_number":77,"context_line":"        metadata.update(actual_metadata)"},{"line_number":78,"context_line":"        update.update(actual_metadata)"},{"line_number":79,"context_line":"        # Get the metadata of the snapshot"},{"line_number":80,"context_line":"        _, body \u003d self.client.get_snapshot_metadata(self.snapshot_id)"},{"line_number":81,"context_line":"        self.assertEqual(metadata, body)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_08ce4d2e","line":78,"in_reply_to":"3a961159_8aa62ac0","updated":"2014-12-19 13:36:32.000000000","message":"I totally agree to remove it in the first case (test_update_snapshot_metadata) considering the result will be the same but I don\u0027t understand why remove it in the second (test_update_snapshot_metadata_item) as the last assert will always fail. And ins the second the actual_metadata is not carried in a load, it\u0027s just used to make sure the assert will be right.\n\nThis is my view over this, please correct me with more info if I\u0027m wrong.\n\nThank you for feedback.\nRegards,\nLucas R.","commit_id":"21edcb3ef23f1cc31162c1ccc81ec41009228692"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"f58509a1033bc0af3ca3073fd3d3a4f0a5e22924","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_402db945","line":38,"updated":"2015-01-29 10:20:12.000000000","message":"This is duplicating too much code. Plus it makes unnecessary API calls to retrieve actual metadata first.\n\nYou should replace every assertEqual (eg L55, L80, L108) by something that only check key1, key2 and key3 are here, *and* have the expected value.\n\nThen you should replace assertEqual line 60, 86, 116, with something that check that the \"correct\" keys were indeed removed or updated.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"7e6b625275ed531ed48c4080300a0859af90391c","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_62d70cc0","line":38,"in_reply_to":"fa81d914_020bb025","updated":"2015-01-29 17:49:33.000000000","message":"I see your point but I don\u0027t see any advantage in comparing value by value. Both ways are valid.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"923fa350c19dc4bc9f4dc6e7065ea1c31cfdfebe","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_020bb025","line":38,"in_reply_to":"fa81d914_29118205","updated":"2015-01-29 17:25:52.000000000","message":"You should check that what you put, ie key1, key2 and key3 are here and has the correct value. You don\u0027t need to check all the metadata, just check the keys you are interested in.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":13553,"name":"Lucas Ribeiro","email":"lucasribeiro1990@gmail.com","username":"lucasribeiro1990"},"change_message_id":"5ba261e20af9c9d9ac47fd445a3d57d0bc8c58c6","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_a9f3b29c","line":38,"in_reply_to":"fa81d914_402db945","updated":"2015-01-29 15:49:16.000000000","message":"I can\u0027t change the test structure (asserts) as it\u0027s been working as designed. I just added a new call to get the metadata preexisting, created by the driver. So I need to append it to the metadata used in the test then the actual asserts work as always and considering the preexisting the metadata.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"678a8911e741cb7c3229ff0ef610582d3ecd87bc","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_29118205","line":38,"in_reply_to":"fa81d914_402db945","updated":"2015-01-29 15:43:23.000000000","message":"The additional API calls are not unnecessary. Some back-ends add metadata to the existing metadata which this test is not considering and therefore it gives false negatives. We need to consider those values. assertEqual is doing it meant to do.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"ea337d3426427480fe1af4e46ba9032f55fdb922","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_663e232a","line":38,"in_reply_to":"fa81d914_62d70cc0","updated":"2015-01-30 09:58:37.000000000","message":"2 advantages :\n\n1) So this test wants to test a feature called \"get snapshot metadata\", but if we do it your way, the first thing this test would rely on would be that get snapshot metadata works...\n\n2) You avoid unnecessary API calls, so your test is more \"straight to the point\".","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"},{"author":{"_account_id":7872,"name":"Marc Koderer","email":"marc@koderer.com","username":"mkoderer"},"change_message_id":"bccf8e5425da13e7df0a0490b90d8266f76185ff","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        super(SnapshotV2MetadataTestJSON, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @test.attr(type\u003d\u0027gate\u0027)"},{"line_number":38,"context_line":"    def test_create_get_delete_snapshot_metadata(self):"},{"line_number":39,"context_line":"        # Get the actual metadata"},{"line_number":40,"context_line":"        actual_metadata \u003d self.client.get_snapshot_metadata("},{"line_number":41,"context_line":"            self.snapshot_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5afe65bd_efd3df36","line":38,"in_reply_to":"fa81d914_a9f3b29c","updated":"2015-06-03 09:59:21.000000000","message":"I agree with Jordan. The get_snapshot_metadata call does not give us any more test coverage. It simply adds all additional information to both dicts. If drivers are allowed to add information to  snapshot_metadata we have to change the way we assert.","commit_id":"c4737d2391c5cf5ef0e850eb0f6fe12fad315eb3"}]}