)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c2ae82b89e0c92eda266a7b0ee4afe4a7417b2fa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bb6b218c_7592d3e2","updated":"2024-09-05 16:43:55.000000000","message":"code lgtm but as these are lib function please add:\n\n- add the release notes about adding domain manager and project manager support in tempest\n- add unit test for both cred class, you can find their existing tests in below files\n  - https://github.com/openstack/tempest/blob/master/tempest/tests/lib/common/test_dynamic_creds.py\n  - https://github.com/openstack/tempest/blob/master/tempest/tests/lib/common/test_preprov_creds.py","commit_id":"63ad7b518875e5cdaf45b06a7b9e6885e4800cd2"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"86de67ac88bb509c7750f8b0d58615536a5ea9ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"23bc09a5_0124078b","updated":"2024-09-05 15:17:03.000000000","message":"makes sense, thanks","commit_id":"63ad7b518875e5cdaf45b06a7b9e6885e4800cd2"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"5284d177dd6dcac8583bec01d072f328008f79a8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2b165e48_50854718","updated":"2024-09-06 08:22:31.000000000","message":"Thank you Ghanshyam for having a look!\n\nAs requested, I added release notes and tests for the DynamicCredentialProvider.\n\nHowever, I was unable to figure out what kind of tests you expect me to add for the PreProvisionedCredentialProvider.\nI had a look at https://github.com/openstack/tempest/blob/master/tempest/tests/lib/common/test_preprov_creds.py but aside from tests of the general functionality of the PreProvisionedCredentialProvider class, I was unable to find any existing tests for the personas, such as project reader or domain member for example, which I could mimic for the manager personas.","commit_id":"953fb5cc5ea7f26dbad7502889250e44ae610772"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fa3bdc0f25f990869ce12da61c833e9eddf8003c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f1e830b1_0279b8ea","updated":"2024-09-06 17:26:35.000000000","message":"Thanks for updates, we have the test for pre-provisioned creds, please add them. I am -1 for that bcz we always miss to test the pre-provisioned creds and its job fail and no body notice.","commit_id":"953fb5cc5ea7f26dbad7502889250e44ae610772"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"60d42c234711e2f26a5a193c6f0f69488c2edaf0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a42c1892_ecb7ba33","updated":"2024-09-06 12:54:46.000000000","message":"lgtm, i\u0027ll wait for gmann to confirm his points were addressed","commit_id":"953fb5cc5ea7f26dbad7502889250e44ae610772"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"9d359573e1d434abe44cb4be4f9f59dbffb5a44e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"54ac93a4_788b7e49","in_reply_to":"006dd24b_d334c78b","updated":"2024-09-09 11:20:34.000000000","message":"Acknowledged","commit_id":"953fb5cc5ea7f26dbad7502889250e44ae610772"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fa3bdc0f25f990869ce12da61c833e9eddf8003c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"006dd24b_d334c78b","in_reply_to":"2b165e48_50854718","updated":"2024-09-06 17:26:35.000000000","message":"yeah, we are missing the reader personas tests but we have member, admin personas tests - https://github.com/openstack/tempest/blob/0a0e1070e573674332cb5126064b95f17099307e/tempest/tests/lib/common/test_preprov_creds.py#L352C1-L368C52\n\nYou can add 1. role[manager], 2. scope\u003ddomain, role[manager] accounts in _fake_accounts() of both class TestPreProvisionedCredentials and TestPreProvisionedCredentialsV3\n\nAnd write test like the one below (NOTE: I have executed them, please modify if any error)\n\n\n    def test_get_project_manager_creds(self):\n        test_accounts_class \u003d preprov_creds.PreProvisionedCredentialProvider(\n            **self.fixed_params)\n        p_manager_creds \u003d test_accounts_class.get_project_manager_creds()\n        self.assertNotIn(\u0027test_admin\u0027, p_manager_creds.username)\n        self.assertNotIn(\u0027test_user\u0027, p_manager_creds.username)\n        self.assertIn(\u0027test_project_manager\u0027, p_manager_creds.username)\n\n    def test_get_primary_creds_none_available(self):\n        admin_accounts \u003d [x for x in self.test_accounts if \u0027test_admin\u0027\n                          in x[\u0027username\u0027]]\n        self.useFixture(fixtures.MockPatch(\n            \u0027tempest.lib.common.preprov_creds.read_accounts_yaml\u0027,\n            return_value\u003dadmin_accounts))\n        test_accounts_class \u003d preprov_creds.PreProvisionedCredentialProvider(\n            **self.fixed_params)\n        with testtools.ExpectedException(lib_exc.InvalidCredentials):\n            # Get one more\n            test_accounts_class.get_project_manager_creds()\n            \n similarly for domain manager.","commit_id":"953fb5cc5ea7f26dbad7502889250e44ae610772"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"9d359573e1d434abe44cb4be4f9f59dbffb5a44e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f1c7683d_dd4ba4c0","updated":"2024-09-09 11:20:34.000000000","message":"Ghanshyam thank you very much for your guidance and the examples! I implemented tests for the pre-provisioned credentials as advised.","commit_id":"a0199bfcd0ab1feb9c7004848683cca6fcc8bebc"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"78d5982fbb64e0ce9fcfc2684cb1c8d871a4e75a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ae1a548d_f25f193e","updated":"2024-09-09 17:01:36.000000000","message":"perfect, thanks Markus.\n\nApproving it considering Martin earlier +2 and only unit tests were added after that.","commit_id":"a0199bfcd0ab1feb9c7004848683cca6fcc8bebc"}]}