)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"39129cb6980d569412c315c31b0fa7dbbd982edf","unresolved":true,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"06076442_ccded34e","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"updated":"2022-10-10 05:06:19.000000000","message":"@rosmaita, do you know about this, I am not able to configure it and not able to recollect what it is?","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff7fe397cc25de59859cbd6c5482da3a7ea4fe8e","unresolved":true,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fc146689_749c6a84","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"in_reply_to":"06076442_ccded34e","updated":"2022-10-10 08:46:00.000000000","message":"I\u0027m not sure whether I like the http://neverssl.com/online solution. Using an external server for testing never sounds like a good idea. Do the tests make any calls to the server? If yes, I think we should not use it. If not, we should use some random placeholder.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"372d185bcba4c999d3066a5fcc10071f0ea98a6c","unresolved":true,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5a557c17_c9047dfb","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"in_reply_to":"302d9e6d_54ad678b","updated":"2022-10-10 16:17:34.000000000","message":"Okay, it turns out that the failure is not due to https, but rather the query parameters in the github mirror redirect, which glance_store is ignoring. That\u0027s fixed in master, but not released, which is why this doesn\u0027t work there.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66155bc948f5dd8030f5706b581697190fc9aac1","unresolved":false,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"f015c901_1aaed5ba","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"in_reply_to":"5a557c17_c9047dfb","updated":"2022-10-14 04:56:14.000000000","message":"Done","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"e6228d67d89cdb7ce99fd48959fcd4478d8d5b0b","unresolved":true,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"302d9e6d_54ad678b","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"in_reply_to":"a3a25ff3_69d126aa","updated":"2022-10-10 14:05:08.000000000","message":"Ok, I understand now :). Yes, it would be better to mark it as WIP then.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b49ac8edd0eeb195bd299cd47f08faeff9a74ff1","unresolved":true,"context_lines":[{"line_number":10,"context_line":"images. A feature must be enabled for this to work, so a feature"},{"line_number":11,"context_line":"flag is added to control it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"There is something in glance that must be enabled to allow https"},{"line_number":14,"context_line":"URLs which is not working, so I\u0027m using neverssl.com as a temporary"},{"line_number":15,"context_line":"measure, but that should be fixed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I779c959096b3aa9cc3d0fbf1e3c506a22d0a49fb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"a3a25ff3_69d126aa","line":14,"range":{"start_line":13,"start_character":29,"end_line":14,"end_character":25},"in_reply_to":"fc146689_749c6a84","updated":"2022-10-10 13:53:10.000000000","message":"Not expecting to merge with this, I just needed something to demonstrate that it works. Hoping for help from Abhi and Brian to figure out why Glance won\u0027t accept an https image URL. I should have marked it as WIP for that reason.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"e6228d67d89cdb7ce99fd48959fcd4478d8d5b0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3a3f1a14_0c63867e","updated":"2022-10-10 14:05:08.000000000","message":"Ack, if I understand it correctly it would be better to mark this as WIP for now :).","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0b795b938fa2f2c6d64bab8f46d34b309719fed0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"56801bbd_c2b47356","updated":"2022-10-10 05:52:11.000000000","message":"Looks good and must have coverage for location APIs, thank you Dan!","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff7fe397cc25de59859cbd6c5482da3a7ea4fe8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"55d66de5_f9c65173","updated":"2022-10-10 08:46:00.000000000","message":"Thanks for the patch! I added a few comments. ","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b49ac8edd0eeb195bd299cd47f08faeff9a74ff1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b6e2929e_22827d0f","updated":"2022-10-10 13:53:10.000000000","message":"Thanks for the surprisingly quick review! I put this up on friday so Abhi could look at what I had so far. Probably should have slapped a WIP in front :)","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"4ea15f673f3e9f29d61971a06292b341499b20d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5448b1d2_7008b213","updated":"2022-10-10 08:07:47.000000000","message":"since you are addding a feature flag, I think we need a reno here. Other thing the test is getting skipped in setupclass in many of tests. I have added the logs below for the reference.\nplease add the note to the test ,a reno  ..after that i can revote.\n\n\n[1] https://f71619cfa10cd3fc99b7-3a4749c0c4f3cc7a5cc841370b346a1f.ssl.cf5.rackcdn.com/860722/5/check/tempest-multinode-full-py3/6bd58a2/testr_results.html\n[2] https://e5a4d98bf447739cb1ce-539aa5196f981f7e99519799b47525b8.ssl.cf1.rackcdn.com/860722/5/check/tempest-full-wallaby-py3/64053ef/testr_results.html\n[3]https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_8ae/860722/5/check/tempest-full-xena/8aeccc5/testr_results.html\n[4] https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_d68/860722/5/check/tempest-full-yoga/d68ae71/testr_results.html\n[5] https://fbfc0099df69f60d5eef-39a519f1e914655bdb6fac4a25e72862.ssl.cf1.rackcdn.com/860722/5/check/tempest-full-zed/4011a0c/testr_results.html","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff7fe397cc25de59859cbd6c5482da3a7ea4fe8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"dac1950f_010bb7f8","in_reply_to":"5448b1d2_7008b213","updated":"2022-10-10 08:46:00.000000000","message":"I think we need to enable the GLANCE_SHOW_MULTIPLE_LOCATIONS in devstack jobs [1]. \n\n[1] https://opendev.org/openstack/cinder/src/commit/4a6792fba3c716a4af6443eeaa688b6f32a60c08/.zuul.yaml#L309","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b49ac8edd0eeb195bd299cd47f08faeff9a74ff1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"cda7ae0c_d889b79d","in_reply_to":"dac1950f_010bb7f8","updated":"2022-10-10 13:53:10.000000000","message":"Yeah we will only want to enable that in select jobs, and our ceph jobs should have that enabled because nova needs it as well. So I will set this flag in those jobs, I just didn\u0027t finish that on Friday before I pushed this up for visibility to the glance people.\n\nAck on the reno.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"efb8758aefd7a8fc619890d41638febac2deedb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"0a8d7844_9d203941","updated":"2022-10-12 09:43:07.000000000","message":"The tests are getting skipped in following jobs:-\n\n1. tempest-full-zed\n2. tempest-full-xena\n3. tempest-full-yoga\n4. tempest-multinode-full-py3\n\nCan you add a test patch wherein the feature is enabled and the test run successfully? because without testing we aren\u0027t able to determine how much time these tests do take and wether the test are running fine.\n\n[1] https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_03b/860722/10/check/tempest-full-zed/03bdffd/testr_results.html\n[2] https://1716c56e89cc5527e240-c7b77f54f6ba700618609de2e81876cb.ssl.cf2.rackcdn.com/860722/10/check/tempest-full-xena/717ad93/testr_results.html\n3. https://5251e9b1e089f0754774-48807f3c89ead7ddc30bbc606994251a.ssl.cf5.rackcdn.com/860722/10/check/tempest-full-yoga/d7248d3/testr_results.html\n4. https://13e30d73ec5b0855d4d8-3df34f04e18be629eb587340c626577b.ssl.cf2.rackcdn.com/860722/10/check/tempest-multinode-full-py3/9965112/testr_results.html","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"dee96ae1cebb299200521b135c9d128378b4d9cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9d13783f_7dae19a7","updated":"2022-10-14 09:56:56.000000000","message":"looks good, all tests are passing (even the skipped one o.O)\n\nhttps://9f22da0fb6a3d4fc8657-b9a59a8a677ef305df7c90cc2cb9167f.ssl.cf5.rackcdn.com/860863/2/check/nova-ceph-multistore/cd6f4a0/testr_results.html","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"d59d7c280c0167feba4e91e5e6330305b7dd9ef1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"0799403a_dde73e83","updated":"2022-10-13 06:54:48.000000000","message":"tests are passing in the nova-ceph-multistore job. LGTM!","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2f55952d11070bc801fefe3756025d3685da6922","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"31ccdc6f_344818ff","in_reply_to":"0a8d7844_9d203941","updated":"2022-10-12 14:26:24.000000000","message":"It doesn\u0027t make any sense to run these on those jobs and they don\u0027t have a deployment layout where the location API is or would be enabled. I\u0027m modifying nova\u0027s ceph-multistore job to enable them here:\n\nhttps://review.opendev.org/c/openstack/nova/+/860863/2\n\nreport here:\n\nhttps://9f22da0fb6a3d4fc8657-b9a59a8a677ef305df7c90cc2cb9167f.ssl.cf5.rackcdn.com/860863/2/check/nova-ceph-multistore/cd6f4a0/testr_results.html","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5e05a7031821613a60620de6193cf91431387715","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2c28a75b_5fd79abe","updated":"2022-10-28 19:32:37.000000000","message":"I think Lukas point is valid. Commented the details inline","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"14c82fbfd04e3d24d9c19d573026ff8cc024f07f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"68b92a43_96b9f52f","updated":"2022-10-28 06:36:02.000000000","message":"I totally agree with lukas","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"26734f69d43ffa0a4a17912508e0afac710ec027","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"bb0ac315_4c12af42","updated":"2022-10-17 08:26:08.000000000","message":"Looks good to me!","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"658ac54e68fdae021eebfb0f7b108f7ee5307ea6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"5a009cd3_5eb18c6d","updated":"2022-10-14 20:25:36.000000000","message":"tests are passing https://0eb8bf1f49f1b12e44de-fa8c367d29960a6ba7cc5d4b52d5b2a7.ssl.cf1.rackcdn.com/860863/2/check/nova-ceph-multistore/2d1a371/testr_results.html","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"eecafe81_e7c1a98e","updated":"2022-11-01 14:24:32.000000000","message":"Looks good to me. \n\nI would only remove the return statement from test_location_after_upload.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"c2c90488_513f6e6a","updated":"2022-10-31 14:31:32.000000000","message":"Thanks :)!","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d1e14d99f6d448cdde01792ad3e91a8ae434e441","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"2471903b_e1d2d940","updated":"2022-10-31 17:49:03.000000000","message":"This current approach lgtm,\n\nAll test passing: https://dd3eaae23a70e6dd7b17-4c67d681abf13f527cb7d280eb684e4e.ssl.cf5.rackcdn.com/860863/2/check/nova-ceph-multistore/24f14a3/testr_results.html\n\nLogs are much readable: https://zuul.opendev.org/t/openstack/build/24f14a3c625f4f95affb04387f59ecc3/log/controller/logs/tempest_log.txt#30544-30590","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5fe650bafd7e488260998cef41d4b9c338ef27e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d3eb3468_51c44f9d","updated":"2022-11-01 14:59:36.000000000","message":"approving it as all tests are passing. agree to remove the return statement from one test which can be fixed in follow up. ","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2fad0090f1e5bd80d98036199c0b5b488e433297","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"890b6d54_ae9619f3","updated":"2022-11-01 17:10:47.000000000","message":"recheck 500 error from nova/cinder during resource create phase of grenade","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a1017279ead6a30c19456d51261623682facc032","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"6667c22f_f9b73ae4","updated":"2022-11-01 19:16:03.000000000","message":"recheck failed some unrelated keystone role test","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"}],"tempest/api/image/v2/test_images.py":[{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"4ea15f673f3e9f29d61971a06292b341499b20d3","unresolved":true,"context_lines":[{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027http://neverssl.com/online\u0027}"},{"line_number":849,"context_line":"        self.client.update_image(image[\u0027id\u0027], ["},{"line_number":850,"context_line":"            dict(add\u003d\u0027/locations/-\u0027, value\u003dnew_loc)])"},{"line_number":851,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"4e88a3bc_30a8cd52","line":848,"updated":"2022-10-10 08:07:47.000000000","message":"please add a note here for - \u0027There is something in glance that must be enabled to allow https\nURLs which is not working, so I\u0027m using neverssl.com as a temporary\nmeasure, but that should be fixed.\u0027, so that everybody will have an idea to get this fixed","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5ca14fd2816114077eb969182636421a52c5a889","unresolved":false,"context_lines":[{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027http://neverssl.com/online\u0027}"},{"line_number":849,"context_line":"        self.client.update_image(image[\u0027id\u0027], ["},{"line_number":850,"context_line":"            dict(add\u003d\u0027/locations/-\u0027, value\u003dnew_loc)])"},{"line_number":851,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"23a34f9f_2422906e","line":848,"in_reply_to":"4e88a3bc_30a8cd52","updated":"2022-10-14 13:33:38.000000000","message":"Done","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff7fe397cc25de59859cbd6c5482da3a7ea4fe8e","unresolved":true,"context_lines":[{"line_number":869,"context_line":"        return image"},{"line_number":870,"context_line":""},{"line_number":871,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":872,"context_line":"    @testtools.skip"},{"line_number":873,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":874,"context_line":"    def test_replace_location(self):"},{"line_number":875,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":5,"id":"c3b6e248_d9aa291b","line":872,"range":{"start_line":872,"start_character":3,"end_line":872,"end_character":19},"updated":"2022-10-10 08:46:00.000000000","message":"If this test does not work we should not merge it. \n\nOr we could create a bug in tempest\u0027s launchpad and then link the bug via @decorators.skip_because().","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"e6228d67d89cdb7ce99fd48959fcd4478d8d5b0b","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        return image"},{"line_number":870,"context_line":""},{"line_number":871,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":872,"context_line":"    @testtools.skip"},{"line_number":873,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":874,"context_line":"    def test_replace_location(self):"},{"line_number":875,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9291f806_e73e41cb","line":872,"range":{"start_line":872,"start_character":3,"end_line":872,"end_character":19},"in_reply_to":"76e639aa_ab1fb483","updated":"2022-10-10 14:05:08.000000000","message":"Ack","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b49ac8edd0eeb195bd299cd47f08faeff9a74ff1","unresolved":true,"context_lines":[{"line_number":869,"context_line":"        return image"},{"line_number":870,"context_line":""},{"line_number":871,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":872,"context_line":"    @testtools.skip"},{"line_number":873,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":874,"context_line":"    def test_replace_location(self):"},{"line_number":875,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":5,"id":"76e639aa_ab1fb483","line":872,"range":{"start_line":872,"start_character":3,"end_line":872,"end_character":19},"in_reply_to":"c3b6e248_d9aa291b","updated":"2022-10-10 13:53:10.000000000","message":"I put this here as a FIXME looking for Abhi or someone else to help me determine if this *should* work (i.e. keep the test) and if so, what the proper patch syntax is.","commit_id":"78a52433812fa94d61c4f5aa9fe6ea6a6d98f830"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"dee96ae1cebb299200521b135c9d128378b4d9cb","unresolved":true,"context_lines":[{"line_number":871,"context_line":"        return image"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":874,"context_line":"    @testtools.skip"},{"line_number":875,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":876,"context_line":"    def test_replace_location(self):"},{"line_number":877,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":10,"id":"d0b64ae2_f62ec871","line":874,"range":{"start_line":874,"start_character":4,"end_line":874,"end_character":19},"updated":"2022-10-14 09:56:56.000000000","message":"shouldn\u0027t this be testtools.skip(\u0027reason\u0027)? Hm, does it work? the test is not skipped here - https://9f22da0fb6a3d4fc8657-b9a59a8a677ef305df7c90cc2cb9167f.ssl.cf5.rackcdn.com/860863/2/check/nova-ceph-multistore/cd6f4a0/testr_results.html","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ec621837e3305de72e270803fb913515962c0b6b","unresolved":false,"context_lines":[{"line_number":871,"context_line":"        return image"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":874,"context_line":"    @testtools.skip"},{"line_number":875,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":876,"context_line":"    def test_replace_location(self):"},{"line_number":877,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":10,"id":"4c0d141c_038a8359","line":874,"range":{"start_line":874,"start_character":4,"end_line":874,"end_character":19},"in_reply_to":"aef09f31_bec08342","updated":"2022-10-14 13:56:16.000000000","message":"Done","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"e6014e3e1e229d94dba18f482a4254fbcfa823df","unresolved":true,"context_lines":[{"line_number":871,"context_line":"        return image"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":874,"context_line":"    @testtools.skip"},{"line_number":875,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":876,"context_line":"    def test_replace_location(self):"},{"line_number":877,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":10,"id":"eb5f3872_83cdc0f1","line":874,"range":{"start_line":874,"start_character":4,"end_line":874,"end_character":19},"in_reply_to":"d0b64ae2_f62ec871","updated":"2022-10-14 11:52:53.000000000","message":"I agree it should be skip(\u0027reason\u0027) without the \"reason\" the test is not skipped. I\u0027ve just checked it manually. I think that we should decide whether this test is valid. If yes, we should remove the FIXME (as it looks like the test is passing).","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6898ac29697a26d04d32417ef1c38bae255e3fb","unresolved":true,"context_lines":[{"line_number":871,"context_line":"        return image"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":874,"context_line":"    @testtools.skip"},{"line_number":875,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":876,"context_line":"    def test_replace_location(self):"},{"line_number":877,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":10,"id":"aef09f31_bec08342","line":874,"range":{"start_line":874,"start_character":4,"end_line":874,"end_character":19},"in_reply_to":"de3a2763_c8e6b3e8","updated":"2022-10-14 13:49:21.000000000","message":"Yeah, this @skip is making it report success, but it is not being run. If I un-@skip it, it fails. Weird.\n\nAnyway, I will pull it out to another patch.","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5ca14fd2816114077eb969182636421a52c5a889","unresolved":true,"context_lines":[{"line_number":871,"context_line":"        return image"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":"    # FIXME(danms): This fails with an invalid JSON pointer. Should this work?"},{"line_number":874,"context_line":"    @testtools.skip"},{"line_number":875,"context_line":"    @decorators.idempotent_id(\u0027bf6e0009-c039-4884-b498-db074caadb10\u0027)"},{"line_number":876,"context_line":"    def test_replace_location(self):"},{"line_number":877,"context_line":"        image \u003d self.test_set_multiple_locations()"}],"source_content_type":"text/x-python","patch_set":10,"id":"de3a2763_c8e6b3e8","line":874,"range":{"start_line":874,"start_character":4,"end_line":874,"end_character":19},"in_reply_to":"eb5f3872_83cdc0f1","updated":"2022-10-14 13:33:38.000000000","message":"Sorry, I\u0027m still looking for Abhi (or someone) to say whether or not this *should* work with glance.\n\nIt was not passing for me locally, but because it (apparently) is passing here I forgot that I need to resolve it. I\u0027ll test is locally again and either un-skip it or pull it out of this patch and into a followup so we can get these landed.","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":804,"context_line":"        if \u0027direct_url\u0027 in image:"},{"line_number":805,"context_line":"            self.assertEqual(image[\u0027direct_url\u0027], image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    @decorators.idempotent_id(\u002737599b8a-d5c0-4590-aee5-73878502be15\u0027)"},{"line_number":810,"context_line":"    def test_set_location(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"d1cb7fb1_68758068","line":807,"range":{"start_line":807,"start_character":0,"end_line":807,"end_character":4},"updated":"2022-10-20 12:57:07.000000000","message":"ditto","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":804,"context_line":"        if \u0027direct_url\u0027 in image:"},{"line_number":805,"context_line":"            self.assertEqual(image[\u0027direct_url\u0027], image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    @decorators.idempotent_id(\u002737599b8a-d5c0-4590-aee5-73878502be15\u0027)"},{"line_number":810,"context_line":"    def test_set_location(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"15990eb9_c4817ad8","line":807,"range":{"start_line":807,"start_character":0,"end_line":807,"end_character":4},"in_reply_to":"d1cb7fb1_68758068","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":838,"context_line":"        self.assertIsNone(None, image[\u0027os_hash_algo\u0027])"},{"line_number":839,"context_line":"        self.assertIsNone(None, image[\u0027os_hash_value\u0027])"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        return image"},{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fe2c5a9c_50c784c0","line":841,"range":{"start_line":841,"start_character":8,"end_line":841,"end_character":20},"updated":"2022-10-20 12:57:07.000000000","message":"ditto","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":838,"context_line":"        self.assertIsNone(None, image[\u0027os_hash_algo\u0027])"},{"line_number":839,"context_line":"        self.assertIsNone(None, image[\u0027os_hash_value\u0027])"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        return image"},{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"25f07cb0_f330567c","line":841,"range":{"start_line":841,"start_character":8,"end_line":841,"end_character":20},"in_reply_to":"fe2c5a9c_50c784c0","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"706d3149_6d359adc","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"updated":"2022-10-20 12:57:07.000000000","message":"I have just noticed this. I\u0027m not sure whether we should reuse one test inside another one. I think that it might be better to have a separate functions that execute the common code (lines 52-86).","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0f4514363d7357d5ba8519042a2fc2e2a6dd715e","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"ac34062b_139ec016","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"289d902d_3b5176d3","updated":"2022-10-28 19:54:52.000000000","message":"If the logger is walking the stack to find the first test_* method so it can claim that\u0027s what\u0027s going on, then that\u0027s a reasonable argument. I don\u0027t believe that breaking the *structure* apart buys us anything, and on the server side it all looks like independent tests. So I\u0027ll rename and wrap for the logger\u0027s sake.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8d27c1012c8be05ca0f4c030ab5efe8cef8edf4e","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"6acc9348_e721826b","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"369b0ed4_8eb59163","updated":"2022-10-31 15:25:56.000000000","message":"Actually if you count direct self.test_*() test-reuse and what we have here, which is calling a testcase inner function self._test_*(), there\u0027s a ton more:\n\n $ grep -r \u0027self._*test_[a-z_]*(\u0027 tempest/tempest/api \\\n     {nova,neutron,cinder,glance,tempest}/*/tests | wc -l\n 5549\n\nObviously that only includes cases where people have used the \"_test_*\" pattern, and wouldn\u0027t even catch things like this, which is \"_check_*()\".\n\nInspecting the actual results to make sure they apply, here\u0027s a common pattern I see all over the place:\n\n nova/nova/tests/functional/compute/test_instance_list.py:\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027hostname\u0027)\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027hostname\u0027,\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027hostname\u0027,\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027hostname\u0027,\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027uuid\u0027)\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027uuid\u0027,\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027launched_at\u0027)\n        self._test_get_sorted_with_limit_marker(sort_by\u003d\u0027created_at\u0027)\n\nThat inner function is: https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/tests/functional/compute/test_instance_list.py#L159-L226\n\nWhich does a bunch of things and asserts state about them, but also takes parameters. Called from multiple test_*() top-levels.\n\nHere\u0027s a more direct example of what I had before, which is test_servers_get() which directly builds on top of self.test_servers_post():\n\nhttps://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/tests/functional/api_sample_tests/test_servers.py#L112-L136\n\ni.e. \"assuming test_servers_post() works, assert that a following get() behaves as we expect.\"","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"f683273bf5c8f2004a1879a21c03fa5bc200cb53","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"c0f74faa_3df61d15","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"369b0ed4_8eb59163","updated":"2022-10-31 16:08:50.000000000","message":"I meant in the tempest repo. Sorry I should have been more precise about this.\nDid you find tests that reuse each other in tempest? Because I wasn\u0027t able to find any. By test I mean a function which has an idempotent_id.\n\nI think that tempest project is a bit different from unit tests from other projects. Tempest tests are for example used in project like Refstack. We need to keep some structure with them. If I run some test in Refstack I want to be sure that I just run a single test not twenty other test.\n\nI think we definitely have a different opinion about this. We can keep this up to others to decide.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"ee5ecb31_91b5f972","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"43a7b51a_cb9cc9f6","updated":"2022-10-31 14:31:32.000000000","message":"@dansmith I\u0027m sorry for being so nitpicking about this. \n\nMy belief is that reusing tests within each other is not a good practice at least in the current setting. In this patch it is OK but once you cross this line it is hard to distinguish between tests that are OK to reuse inside other tests and those that are not. \n\nAlso, another point is that it puts more pressure on the test writer in future to find tests that are OK to reuse. By declaring a separate function for a resource creation it\u0027s like saying: \"Hey, here is a piece of code that can be reused in future tests\". In contrast, if you put the resource creation only in a test it might not be immediately obvious that the test can be reused. \n\nTo be honest I didn\u0027t even think about the logger thing and this seems important too.\n\n@gmann, yes I understand this. In this patch, I agree that it is OK but I can imagine cases in which reusing tests inside each other is not a good idea. And as this is the first test suite that uses this practice I think that it sets a precedence which might be difficult to change in the future. \n\n@dansmith sorry again:), and thanks for fixing (or not fixing from your point of view:) this.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5e05a7031821613a60620de6193cf91431387715","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"289d902d_3b5176d3","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"5301c5b2_8b2b92eb","updated":"2022-10-28 19:32:37.000000000","message":"Ok, I think I got what Luksa is trying to say. It\u0027s the log and debugging become more difficult due to the current approach where test_set_location as ID is logged even for other test execution and it becomes confusing to know which test execution phase created this image.\n\nI fetched a few logs here to show how it is confusing in logs.\n \n1. test_set_location execution: \n \nit log the request as shown below (*ImageLocationsTest:test_set_location*) so all good here and we know these APIs request is from test_set_location execution\n\nRequest (ImageLocationsTest:test_set_location): 201 POST https://10.209.0.72/image/v2/images 0.868s\n\nRequest (ImageLocationsTest:test_set_location): 200 PATCH https://10.209.0.72/image/v2/images/f91ce3ac-aaf1-48e8-878d-648bea320dc7 0.634s\n\nhttps://zuul.opendev.org/t/openstack/build/2d1a371ad0744f89b98a3bb696e22474/log/controller/logs/tempest_log.txt#39946-39971\n\n2. test_set_multiple_locations execution: \n \nThis test execution also log the request with *ImageLocationsTest:test_set_location* id as shown below so it become difficult to tell whether this request from which test execution until we trace the image id and find which test operating on that image id.\n\nRequest (ImageLocationsTest:test_set_location): 201 POST https://10.209.0.72/image/v2/images 0.753s\n\nRequest (ImageLocationsTest:test_set_location): 200 PATCH https://10.209.0.72/image/v2/images/ca2dd23e-a144-4695-b132-6483be55044f 0.644s\n\nhttps://zuul.opendev.org/t/openstack/build/2d1a371ad0744f89b98a3bb696e22474/log/controller/logs/tempest_log.txt#40153-40192\n\nLet\u0027s go with the internal non-test function something like _check_get_image_with_location and use among test so that right test id will be used in logs.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"46e7e4b6898c64bd5c649bed2d0e329f8d9836e4","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"5301c5b2_8b2b92eb","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"664bdb5f_a70e60a2","updated":"2022-10-21 09:18:51.000000000","message":"I understand why you did it. I have two thoughts about this: \n\n1) I think that a test function should be a close unit of code that does testing and nothing else. Usually test consists of three stages: setup (creation of resources), testing (you do something to the resources), and teardown (the resources are destroyed). When test A is used for creation of a resource in test B and test B fails then it is hard to tell whether it failed because the resource got \"dirty\" during the testing phase in test A or because there is actually something wrong with test B. \n \nBecause of that, I think it is better to have the logic for creation of resources separated from the testing logic. Using a test for creation of a resource always carries the chance that the resource is influenced by the testing phase. \n\nIt does not look like the test_set_location() is doing something \"dirty\" with the image. It just creates it and checks whether it is created. But I think that it would be better to keep the same approach throughout tempest. I\u0027m not sure but I think that tempest does not contain test cases that reuse each other. \n\n2) Readability wise something like `image \u003d get_image_with_location()` looks better to me than `image \u003d test_set_location()`.\n\nBut this is really just my idea. I can definitely imagine someone having a different opinion about this. I just wanted to mention this :).","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2407141c90974f489e4895019f839b117cf2e9c1","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"664bdb5f_a70e60a2","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"706d3149_6d359adc","updated":"2022-10-20 16:11:40.000000000","message":"Why? This is a pattern I employ quite often and it makes it easier to read more complicated test scenarios that build on each other, without losing the granularity of testing the simpler scenarios by themselves. So, \"check that X works\" and \"assuming X was done, check that Y works too\".\n\nCan you explain why you think this is bad or problematic?","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"df7cc758eaa6646cc550d435ace426a8633f25f0","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"43a7b51a_cb9cc9f6","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"ac34062b_139ec016","updated":"2022-10-28 20:01:32.000000000","message":"agree,\n\n@Lukas, for your resource sharing comment, it does not matter how we execute the code from single test/function, it will always create the seperate resources and all test work on their created resources and cannot clash with each other. so structure and working wise it is all good but its just the logging mechanism making this approach hard to debug.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"343b3285951925f6fedeb206d1953b8a74e1a991","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"eeed7ad2_d3a22d03","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"c0f74faa_3df61d15","updated":"2022-10-31 16:44:38.000000000","message":"\u003e I meant in the tempest repo. Sorry I should have been more precise about this.\n\u003e Did you find tests that reuse each other in tempest? Because I wasn\u0027t able to find any. By test I mean a function which has an idempotent_id.\n\nHere\u0027s the exact pattern in this patch, in tempest:\n\nhttps://github.com/openstack/tempest/blob/f1d0e395e95d3502a9c3904ce56520d116e6bf48/tempest/api/compute/servers/test_server_actions.py#L201-L251\n\nMultiple things call_test_rebuild_server(). One is just a wrapper with an idempotent_id, and another in the class builds on that with some other stuff before and after (two tests down).\n\n\u003e I think that tempest project is a bit different from unit tests from other projects. Tempest tests are for example used in project like Refstack. We need to keep some structure with them. If I run some test in Refstack I want to be sure that I just run a single test not twenty other test.\n\nBut you can certainly do that - this approach doesn\u0027t change your ability to run one test in the slightest. I think what you\u0027re describing here is \"no tests can have any overlap in their expectations or assertions\" which I definitely do not think is accurate. If one test is a total superset of another test, then there\u0027s really no reason it shouldn\u0027t call the exact test case. In fact, it\u0027s better because all the assertions of the simpler case are replicated in the larger one, not just all the setup. If someone comes along later and adds some assertion to the simpler case (due to a bug discovered, for example), they\u0027re likely to find the test that clearly matches the operation they\u0027re fixing. Cascading the tests (where appropriate) means the more complicated cases all get the same assertion.\n\nTo use your case, if I want to run just one test from refstack, and that is the superset case, I would surely hope that it is asserting all the behaviors of the dependent operations. Without it, you could end up with a \"create..update..delete\" test case that doesn\u0027t do all the assertions on the create behavior _pass_ when it shouldn\u0027t because the create behavior isn\u0027t proper, and it doesn\u0027t assert everything about it that the base \"create\" case does. Either all those assertions need to be replicated in the superset case, or it will be incomplete. If anything, the cascading of test cases _bolsters_ the scenario of \"I should be able to pick any one case from the list of tests and it should work in isolation\". IMHO of course :)\n\n\u003e I think we definitely have a different opinion about this. We can keep this up to others to decide.\n\nYep, and yep.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"06020a290f8611f5d9637b4d88067e694b645508","unresolved":true,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @decorators.idempotent_id(\u0027f67495c6-518a-4397-938e-dc7b135698a8\u0027)"},{"line_number":844,"context_line":"    def test_set_multiple_locations(self):"},{"line_number":845,"context_line":"        image \u003d self.test_set_location()"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"},{"line_number":848,"context_line":"                   \u0027url\u0027: \u0027%s#new\u0027 % CONF.image.http_image}"}],"source_content_type":"text/x-python","patch_set":12,"id":"369b0ed4_8eb59163","line":845,"range":{"start_line":845,"start_character":16,"end_line":845,"end_character":40},"in_reply_to":"ee5ecb31_91b5f972","updated":"2022-10-31 14:55:20.000000000","message":"\u003e And as this is the first test suite that uses this practice I think that it sets a precedence which might be difficult to change in the future.\n\n$ grep -r \u0027self.test_[a-z_]*(\u0027 nova/tests | wc -l                                           \n50\n\n$ grep -r \u0027self.test_[a-z_]*(\u0027 glance/tests | wc -l                                                 \n3\n\n$ grep -r \u0027self.test_[a-z_]*(\u0027 neutron/tests | wc -l                                                \n19\n\n$ grep -r \u0027self.test_[a-z_]*(\u0027 cinder/tests | wc -l\n8\n\nThat\u0027s the most naive grep pattern and it finds plenty of precedent. There\u0027s undoubtedly more.","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"    @decorators.idempotent_id(\u00278a648de4-b745-4c28-a7b5-20de1c3da4d2\u0027)"},{"line_number":872,"context_line":"    def test_delete_locations(self):"},{"line_number":873,"context_line":"        image \u003d self.test_set_multiple_locations()"},{"line_number":874,"context_line":"        expected_remaining_loc \u003d image[\u0027locations\u0027][1]"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"        self.client.update_image(image[\u0027id\u0027], ["}],"source_content_type":"text/x-python","patch_set":12,"id":"77cdc1be_fe9ab2b3","line":873,"range":{"start_line":873,"start_character":7,"end_line":873,"end_character":50},"updated":"2022-10-20 12:57:07.000000000","message":"ditto","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"    @decorators.idempotent_id(\u00278a648de4-b745-4c28-a7b5-20de1c3da4d2\u0027)"},{"line_number":872,"context_line":"    def test_delete_locations(self):"},{"line_number":873,"context_line":"        image \u003d self.test_set_multiple_locations()"},{"line_number":874,"context_line":"        expected_remaining_loc \u003d image[\u0027locations\u0027][1]"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"        self.client.update_image(image[\u0027id\u0027], ["}],"source_content_type":"text/x-python","patch_set":12,"id":"f3772b69_702f2388","line":873,"range":{"start_line":873,"start_character":7,"end_line":873,"end_character":50},"in_reply_to":"77cdc1be_fe9ab2b3","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":933,"context_line":"        self.assertNotIn(\u0027validation_data\u0027, image[\u0027locations\u0027][0])"},{"line_number":934,"context_line":"        self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":935,"context_line":""},{"line_number":936,"context_line":"        return image"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    @decorators.idempotent_id(\u0027304c8a19-aa86-47dd-a022-ec4c7f433f1b\u0027)"},{"line_number":939,"context_line":"    def test_set_location_with_hash_second_matching(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"6c62c19d_cd52a088","line":936,"range":{"start_line":936,"start_character":8,"end_line":936,"end_character":20},"updated":"2022-10-20 12:57:07.000000000","message":"ditto","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        self.assertNotIn(\u0027validation_data\u0027, image[\u0027locations\u0027][0])"},{"line_number":934,"context_line":"        self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":935,"context_line":""},{"line_number":936,"context_line":"        return image"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    @decorators.idempotent_id(\u0027304c8a19-aa86-47dd-a022-ec4c7f433f1b\u0027)"},{"line_number":939,"context_line":"    def test_set_location_with_hash_second_matching(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"d2d1feb0_35b4451f","line":936,"range":{"start_line":936,"start_character":8,"end_line":936,"end_character":20},"in_reply_to":"6c62c19d_cd52a088","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"7af75183efd48dbb7b78600058cb996efd8784f8","unresolved":true,"context_lines":[{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    @decorators.idempotent_id(\u0027f3ce99c2-9ffb-4b9f-b2cb-876929382553\u0027)"},{"line_number":961,"context_line":"    def test_set_location_with_hash_not_matching(self):"},{"line_number":962,"context_line":"        orig_image \u003d self.test_set_location_with_hash()"},{"line_number":963,"context_line":"        values \u003d {"},{"line_number":964,"context_line":"            \u0027checksum\u0027: \u00272\u0027 * 32,"},{"line_number":965,"context_line":"            \u0027os_hash_value\u0027: \u0027beefdead\u0027 * 16,"}],"source_content_type":"text/x-python","patch_set":12,"id":"306bff60_214bd965","line":962,"range":{"start_line":962,"start_character":21,"end_line":962,"end_character":55},"updated":"2022-10-20 12:57:07.000000000","message":"ditto","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    @decorators.idempotent_id(\u0027f3ce99c2-9ffb-4b9f-b2cb-876929382553\u0027)"},{"line_number":961,"context_line":"    def test_set_location_with_hash_not_matching(self):"},{"line_number":962,"context_line":"        orig_image \u003d self.test_set_location_with_hash()"},{"line_number":963,"context_line":"        values \u003d {"},{"line_number":964,"context_line":"            \u0027checksum\u0027: \u00272\u0027 * 32,"},{"line_number":965,"context_line":"            \u0027os_hash_value\u0027: \u0027beefdead\u0027 * 16,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5ab2e02e_1ce2aa74","line":962,"range":{"start_line":962,"start_character":21,"end_line":962,"end_character":55},"in_reply_to":"306bff60_214bd965","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"7c3a629d97b21342fe3a55ccadf8b7381c393aca"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":true,"context_lines":[{"line_number":804,"context_line":"        if \u0027direct_url\u0027 in image:"},{"line_number":805,"context_line":"            self.assertEqual(image[\u0027direct_url\u0027], image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"e510a802_cc8c4063","line":807,"updated":"2022-10-31 14:31:32.000000000","message":"ditto","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5fe650bafd7e488260998cef41d4b9c338ef27e0","unresolved":true,"context_lines":[{"line_number":804,"context_line":"        if \u0027direct_url\u0027 in image:"},{"line_number":805,"context_line":"            self.assertEqual(image[\u0027direct_url\u0027], image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"5cb48178_8fb53f95","line":807,"in_reply_to":"e510a802_cc8c4063","updated":"2022-11-01 14:59:36.000000000","message":"yes, this is not needed. let\u0027s fix this in followup patch.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":811,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"17db0991_e6ca9319","line":809,"range":{"start_line":809,"start_character":8,"end_line":809,"end_character":27},"updated":"2022-10-31 14:31:32.000000000","message":"I would put this at the beginning of the class in front of every test. Personal preference. Not important. (Line 781)\n\nI think better name is something like _get_image_with_location() (again personal preference)","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d1e14d99f6d448cdde01792ad3e91a8ae434e441","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":811,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"4804c245_25d28836","line":809,"range":{"start_line":809,"start_character":8,"end_line":809,"end_character":27},"in_reply_to":"002b1e44_792b5e3b","updated":"2022-10-31 17:49:03.000000000","message":"it is ok to keep the internal functions near to their usage which is better in term of readability. And they are at the top of the tests using them.\n\nIf they are used by many/all tests then yes we can keep them at the top of file.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"06020a290f8611f5d9637b4d88067e694b645508","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":811,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"533d8e7c_65d3b723","line":809,"range":{"start_line":809,"start_character":8,"end_line":809,"end_character":27},"in_reply_to":"17db0991_e6ca9319","updated":"2022-10-31 14:55:20.000000000","message":"\u003e I would put this at the beginning of the class in front of every test. Personal preference. Not important. (Line 781)\n\nWhy? So I have to read further up to see what it\u0027s doing?\n\n\u003e I think better name is something like _get_image_with_location() (again personal preference)\n\nYeah, agree it\u0027s personal preference. It\u0027s doing more than getting, it\u0027s also checking. And the name came from gmann\u0027s suggestion.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":811,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"26ff43ba_d0fff0d9","line":809,"range":{"start_line":809,"start_character":8,"end_line":809,"end_character":27},"in_reply_to":"4804c245_25d28836","updated":"2022-11-01 14:24:32.000000000","message":"Ack","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"f683273bf5c8f2004a1879a21c03fa5bc200cb53","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        return image"},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"    def _check_set_location(self):"},{"line_number":810,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":811,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"002b1e44_792b5e3b","line":809,"range":{"start_line":809,"start_character":8,"end_line":809,"end_character":27},"in_reply_to":"533d8e7c_65d3b723","updated":"2022-10-31 16:08:50.000000000","message":"Because the class is more structured this way. (Again in my opinion, you definitely do not have to follow this). You can see the same pattern in the test_images.py file within the ImportImagesTest class [1].\n\n1) Setup functions like: SetUp(), TearDown(), setup_clients(), ...\n2) Common functions for the test suite.\n3) And the tests (that use the common functions from section 2).\n\nIf I am about to write a new test in this class I can just take a look at the common functions which are already implemented. Then I would start to write the new test knowing all the common functions that I can use in this new test.\n\nOk, agreed.\n\n[1] https://opendev.org/openstack/tempest/src/commit/beda3306a25d4927cc0513d5c42c8a38b629fc9d/tempest/api/image/v2/test_images.py#L32","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":true,"context_lines":[{"line_number":843,"context_line":"    def test_set_location(self):"},{"line_number":844,"context_line":"        self._check_set_location()"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _check_set_multiple_locations(self):"},{"line_number":847,"context_line":"        image \u003d self._check_set_location()"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"}],"source_content_type":"text/x-python","patch_set":13,"id":"d304ce8a_d70d683b","line":846,"range":{"start_line":846,"start_character":8,"end_line":846,"end_character":37},"updated":"2022-10-31 14:31:32.000000000","message":"I would put this at the beginning of the class in front of every test. Personal preference. Not important. (Line 781)\n\nI think better name is something like _get_image_with_multiple_locations(). But this is probably not important, too.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":843,"context_line":"    def test_set_location(self):"},{"line_number":844,"context_line":"        self._check_set_location()"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _check_set_multiple_locations(self):"},{"line_number":847,"context_line":"        image \u003d self._check_set_location()"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":"        new_loc \u003d {\u0027metadata\u0027: {\u0027speed\u0027: \u002788mph\u0027},"}],"source_content_type":"text/x-python","patch_set":13,"id":"ab08458d_6a51bbee","line":846,"range":{"start_line":846,"start_character":8,"end_line":846,"end_character":37},"in_reply_to":"d304ce8a_d70d683b","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"414f11223999f9bd77b91b7f06f5b838ab14279e","unresolved":true,"context_lines":[{"line_number":916,"context_line":"                          self.client.update_image, image[\u0027id\u0027], ["},{"line_number":917,"context_line":"                              dict(add\u003d\u0027/locations/-\u0027, value\u003dnew_loc)])"},{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    def _check_set_location_with_hash(self):"},{"line_number":920,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":921,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":922,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"e91d27ee_398d9cd8","line":919,"range":{"start_line":919,"start_character":0,"end_line":919,"end_character":37},"updated":"2022-10-31 14:31:32.000000000","message":"I would put this at the beginning of the class in front of every test. Personal preference. Not important. (Line 781)\n\nI think better name is something like _get_image_with_location_hash(). But this is probably not important, too.","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"},{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"ff374542bf4fd9e37155bf89b156e9650020ee72","unresolved":false,"context_lines":[{"line_number":916,"context_line":"                          self.client.update_image, image[\u0027id\u0027], ["},{"line_number":917,"context_line":"                              dict(add\u003d\u0027/locations/-\u0027, value\u003dnew_loc)])"},{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    def _check_set_location_with_hash(self):"},{"line_number":920,"context_line":"        image \u003d self.client.create_image(container_format\u003d\u0027bare\u0027,"},{"line_number":921,"context_line":"                                         disk_format\u003d\u0027raw\u0027)"},{"line_number":922,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"91b93bb0_5de18740","line":919,"range":{"start_line":919,"start_character":0,"end_line":919,"end_character":37},"in_reply_to":"e91d27ee_398d9cd8","updated":"2022-11-01 14:24:32.000000000","message":"Done","commit_id":"9eaaa5ac3ee8d51563072ad63e1ac76bccf3b748"}],"tempest/config.py":[{"author":{"_account_id":30674,"name":"Lukas Piwowarski","email":"lpiwowar@redhat.com","username":"lukas-piwowarski"},"change_message_id":"e6014e3e1e229d94dba18f482a4254fbcfa823df","unresolved":true,"context_lines":[{"line_number":735,"context_line":"    cfg.BoolOpt(\u0027os_glance_reserved\u0027,"},{"line_number":736,"context_line":"                default\u003dFalse,"},{"line_number":737,"context_line":"                help\u003d\"Should we check that os_glance namespace is reserved\"),"},{"line_number":738,"context_line":"    cfg.BoolOpt(\u0027manage_locations\u0027,"},{"line_number":739,"context_line":"                default\u003dFalse,"},{"line_number":740,"context_line":"                help\u003d(\u0027Is show_multiple_locations enabled in glance. \u0027"},{"line_number":741,"context_line":"                      \u0027Note that at least one http store must be enabled as \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"ce1a7b59_1bf5e8f4","line":738,"range":{"start_line":738,"start_character":17,"end_line":738,"end_character":33},"updated":"2022-10-14 11:52:53.000000000","message":"Just a nit. Shouldn\u0027t we name this option as \u0027show_multiple_locations\u0027 or \u0027show_multiple_locations_enabled\u0027? I\u0027m just thinking about this and I\u0027m not sure.","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"658ac54e68fdae021eebfb0f7b108f7ee5307ea6","unresolved":true,"context_lines":[{"line_number":735,"context_line":"    cfg.BoolOpt(\u0027os_glance_reserved\u0027,"},{"line_number":736,"context_line":"                default\u003dFalse,"},{"line_number":737,"context_line":"                help\u003d\"Should we check that os_glance namespace is reserved\"),"},{"line_number":738,"context_line":"    cfg.BoolOpt(\u0027manage_locations\u0027,"},{"line_number":739,"context_line":"                default\u003dFalse,"},{"line_number":740,"context_line":"                help\u003d(\u0027Is show_multiple_locations enabled in glance. \u0027"},{"line_number":741,"context_line":"                      \u0027Note that at least one http store must be enabled as \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"a0e1859b_07b17f21","line":738,"range":{"start_line":738,"start_character":17,"end_line":738,"end_character":33},"in_reply_to":"adfe8b24_fe66dcb9","updated":"2022-10-14 20:25:36.000000000","message":"I am ok with either one.","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5ca14fd2816114077eb969182636421a52c5a889","unresolved":true,"context_lines":[{"line_number":735,"context_line":"    cfg.BoolOpt(\u0027os_glance_reserved\u0027,"},{"line_number":736,"context_line":"                default\u003dFalse,"},{"line_number":737,"context_line":"                help\u003d\"Should we check that os_glance namespace is reserved\"),"},{"line_number":738,"context_line":"    cfg.BoolOpt(\u0027manage_locations\u0027,"},{"line_number":739,"context_line":"                default\u003dFalse,"},{"line_number":740,"context_line":"                help\u003d(\u0027Is show_multiple_locations enabled in glance. \u0027"},{"line_number":741,"context_line":"                      \u0027Note that at least one http store must be enabled as \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"adfe8b24_fe66dcb9","line":738,"range":{"start_line":738,"start_character":17,"end_line":738,"end_character":33},"in_reply_to":"ce1a7b59_1bf5e8f4","updated":"2022-10-14 13:33:38.000000000","message":"Well, I guess it\u0027s up for discussion. I named it this as in \"try to ... manage locations\" or \"the locations management stuff is enabled.\" There\u0027s more than just show_multiple_locations, as direct_url is tested as well, which isn\u0027t covered by the show_multiple_locations flag...","commit_id":"6471c0687bbf090f9e190dec25740d0618b63e6c"}]}