)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"4282fbe077912fb67a99a72346cd41e810886cf7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"52261ab1_a6c24684","updated":"2026-07-07 07:49:26.000000000","message":"recheck","commit_id":"77139e25454f7f695d18b2dba8a241a26e80eaae"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"a7ff9f46aa46d8e5bc82c3d3d4fdcd7e99247327","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"3041c7bc_09fa58f0","updated":"2026-07-14 06:37:50.000000000","message":"test_existing_goals.py test screenshot: https://dcdef67f370bf151c86e-9e1e29a469f8f669289667ecce245a85.ssl.cf1.rackcdn.com/openstack/67c0871e85f2480fbe906ec623883649/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_goals.GoalsTests.test_goals_match_watcher_api/011_all_goals_verified.png\n\ntest_existing_strategies.py test screenshot: https://dcdef67f370bf151c86e-9e1e29a469f8f669289667ecce245a85.ssl.cf1.rackcdn.com/openstack/67c0871e85f2480fbe906ec623883649/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_strategies.StrategiesTests.test_strategies_match_watcher_api/011_all_strategies_verified.png","commit_id":"d2168fc4129e5adb9c74e3594f397df7d606d7dc"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"84c8cfab31990538f7ed8e9e9df8ac830e0e6358","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"5a970f8f_6c07075a","updated":"2026-07-16 17:13:49.000000000","message":"lgtm, thanks","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"017ef3c4f0961f4fe890245beb1e6d6f59155b6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"e6bb98a4_33c5d8e0","updated":"2026-07-30 16:54:08.000000000","message":"You can work in improvement on a follow patch, for now we can continue with this one. Thanks","commit_id":"99c57210a1461a12f7ec541c9f10fc5d5d911211"}],"watcher_dashboard/test/integration/test_existing_goals.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a55ada967b06ebabd7d813fc34dd663921f402b6","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\"\"\"Integration tests for Goals using Playwright.\"\"\""},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from django import test"},{"line_number":16,"context_line":"from playwright import sync_api as playwright_sync"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from watcher_dashboard.test.integration import playwright_base"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"76f12bd2_2f53f263","line":16,"updated":"2026-07-06 10:22:31.000000000","message":"Imports `playwright_sync` solely to call `playwright_sync.expect(...)`, but the base class already exposes the same helper as `self.expect`, which the sibling audit-workflow test uses consistently.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Consistency with the sibling integration test and with the base class design, plus removal of an import that is no longer needed.\n\n**Recommendation**:\nDrop the `from playwright import sync_api as playwright_sync` import and replace each `playwright_sync.expect(...)` call with `self.expect(...)`, matching how PlaywrightTestCase exposes `expect \u003d sync_api.expect` and how test_playwright_audit_workflow.py uses it.","commit_id":"0d38af2430e64d25b915052f7177c6a03c17db56"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a55ada967b06ebabd7d813fc34dd663921f402b6","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            display_name \u003d api_goal.display_name"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"            # Locate the row by goal name"},{"line_number":66,"context_line":"            row \u003d self.page.locator(f\"tbody tr[data-display\u003d\u0027{goal_name}\u0027]\")"},{"line_number":67,"context_line":"            playwright_sync.expect(row).to_be_visible("},{"line_number":68,"context_line":"                timeout\u003d5000"},{"line_number":69,"context_line":"            ), f\"Goal \u0027{goal_name}\u0027 not found in UI\""}],"source_content_type":"text/x-python","patch_set":5,"id":"4a80acd5_92976142","line":66,"updated":"2026-07-06 10:22:31.000000000","message":"Locates table rows with the CSS selector `tbody tr[data-display\u003d\u0027{goal_name}\u0027]`, which couples the test to Horizon\u0027s internal `data-display` rendering and breaks if a goal name contains an apostrophe.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Role-based locators survive markup churn and special characters, and they align this test with the existing sibling-test convention.\n\n**Recommendation**:\nPrefer the role-based pattern already used in test_playwright_audit_workflow.py: `self.page.get_by_role(\"row\").filter(has_text\u003dgoal_name)`. If the attribute selector must stay, at least escape single quotes/apostrophes in goal_name before interpolating it into the selector.","commit_id":"0d38af2430e64d25b915052f7177c6a03c17db56"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"6639a945e29f6a6c81fa50c25d7bf9cb9e29b453","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\"\"\"Integration tests for Goals using Playwright.\"\"\""},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from django import test"},{"line_number":16,"context_line":"from playwright import sync_api as playwright_sync"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from watcher_dashboard.test.integration import playwright_base"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5383b08b_d08d87e1","line":16,"updated":"2026-07-07 08:10:12.000000000","message":"Both new test files import \u0027from playwright import sync_api as playwright_sync\u0027 and call playwright_sync.expect(...) throughout. The base class PlaywrightTestCase already provides \u0027expect \u003d sync_api.expect\u0027 (playwright_base.py line 177), and the sibling test test_playwright_audit_workflow.py cons...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Minor maintainability concern: future contributors may be confused by two different patterns for the same operation across sibling test files. The base class alias was specifically designed to avoid importing playwright internals in test subclasses.\n\n**Recommendation**:\nRemove the \u0027from playwright import sync_api as playwright_sync\u0027 import and replace all \u0027playwright_sync.expect(...)\u0027 calls with \u0027self.expect(...)\u0027 to match the convention used in test_playwright_audit_workflow.py and the design intent of the base class.","commit_id":"77139e25454f7f695d18b2dba8a241a26e80eaae"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4fb85e43eb65176253e13f78d7d50012b259b4a4","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"            # Locate the row by goal name"},{"line_number":66,"context_line":"            row \u003d self.page.locator(f\"tbody tr[data-display\u003d\u0027{goal_name}\u0027]\")"},{"line_number":67,"context_line":"            ("},{"line_number":68,"context_line":"                playwright_sync.expect(row).to_be_visible(timeout\u003d5000),"},{"line_number":69,"context_line":"                f\"Goal \u0027{goal_name}\u0027 not found in UI\","},{"line_number":70,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":8,"id":"2e70a278_5c92fedc","line":67,"updated":"2026-07-07 09:10:39.000000000","message":"The pattern `(playwright_sync.expect(row).to_be_visible(timeout\u003d5000), f\"Goal \u0027...\u0027 not found in UI\")` creates a bare tuple that is immediately discarded. The expect() call does execute and will raise on failure, but the error message string is the second element of a discarded tuple and is never...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: When a goal is missing from the UI, the test fails with Playwright\u0027s generic timeout message instead of the intended descriptive message (e.g., \"Goal \u0027X\u0027 not found in UI\"). Developers debugging test failures lose the context the author clearly intended to provide. The tuple syntax is also mislead...\n\n**Suggestion**:\nRemove the tuple wrapper and either (a) call `playwright_sync.expect(row).to_be_visible(timeout\u003d5000)` directly without a custom message, or (b) if custom messages are desired, use a try/except around the expect() call that re-raises with the descriptive message. Option (a) aligns with the existing pattern in test_playwright_audit_workflow.py.","commit_id":"b801385945f4710b95b5e72cbeb8049237a7413e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"d62c9021d0a542c333491106430c349c474f5009","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from django import test"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from playwright import sync_api as playwright_sync"},{"line_number":18,"context_line":"from watcher_dashboard.test.integration import playwright_base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"03554d57_6d29d90c","line":17,"updated":"2026-07-08 09:01:42.000000000","message":"Both new test files import playwright_sync from playwright.sync_api and use playwright_sync.expect(...), while the PlaywrightTestCase base class already exposes expect as a class attribute (self.expect). The existing sibling test_playwright_audit_workflow.py follows the established pattern of usi...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: The inconsistency creates a maintenance pattern where future test files may copy the redundant import rather than using the base class facility. It also adds an unnecessary module-level import that could confuse readers about which expect API is in use.\n\n**Recommendation**:\nRemove the \u0027from playwright import sync_api as playwright_sync\u0027 import from both files and replace all playwright_sync.expect(...) calls with self.expect(...), matching the pattern used in test_playwright_audit_workflow.py.","commit_id":"5aa6ef85d6d6acea5183286294f91f9b3d18bea1"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"84d080b7f4615a5f937746a5d63242f4aa2b0f1b","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"            # Locate the row by goal name"},{"line_number":66,"context_line":"            row \u003d self.page.locator(f\"tbody tr[data-display\u003d\u0027{goal_name}\u0027]\")"},{"line_number":67,"context_line":"            playwright_sync.expect(row).to_be_visible(timeout\u003d5000)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            # Verify the display name is visible in the row"},{"line_number":70,"context_line":"            playwright_sync.expect("}],"source_content_type":"text/x-python","patch_set":11,"id":"a0fd951c_ef276c5f","line":67,"updated":"2026-07-09 08:09:33.000000000","message":"Both new test files use a hardcoded timeout\u003d5000 for to_be_visible() calls instead of the configurable config.get_timeout() used throughout the rest of the test framework.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: If the configured PLAYWRIGHT_TIMEOUT is higher than 5000ms (common in slow CI environments), these tests will use a shorter timeout than intended, potentially causing flaky failures when the UI takes longer than 5 seconds to render individual rows.\n\n**Recommendation**:\nReplace `timeout\u003d5000` with `timeout\u003dconfig.get_timeout()` in both test files, or simply omit the timeout parameter to inherit the default already set on the browser context via set_default_timeout().","commit_id":"effec6618f058202220d4f7079d0fd6c674224e6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"f6353070596ff70851306e4fa2b206b9d0941d70","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from django import test"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from playwright import sync_api as playwright_sync"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from watcher_dashboard.test.integration import playwright_base"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"72d444c3_fd2814f2","line":17,"updated":"2026-07-09 09:24:43.000000000","message":"Both new test files import \u0027from playwright import sync_api as playwright_sync\u0027 and call playwright_sync.expect() throughout. However, the base class PlaywrightTestCase already defines \u0027expect \u003d sync_api.expect\u0027 as a class attribute (playwright_base.py line 177), and the existing sibling test tes...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Minor maintainability concern: future test authors may copy either pattern, leading to mixed conventions. The redundant import also adds unnecessary cognitive overhead when reading the test suite.\n\n**Recommendation**:\nReplace \u0027playwright_sync.expect(...)\u0027 calls with \u0027self.expect(...)\u0027 and remove the \u0027from playwright import sync_api as playwright_sync\u0027 import from both files, matching the pattern used in test_playwright_audit_workflow.py and the base class.","commit_id":"2913f3169ce55c3ab04f019317d6d332cbb519c3"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"84c8cfab31990538f7ed8e9e9df8ac830e0e6358","unresolved":true,"context_lines":[{"line_number":21,"context_line":"class GoalsTests(playwright_base.PlaywrightTestCase):"},{"line_number":22,"context_line":"    \"\"\"Integration tests for Goals using Playwright.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def test_goals_match_watcher_api(self):"},{"line_number":25,"context_line":"        \"\"\"Test that goals displayed in UI match those from Watcher API."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":15,"id":"9477af73_55b0ebde","line":24,"range":{"start_line":24,"start_character":8,"end_line":24,"end_character":36},"updated":"2026-07-16 17:13:49.000000000","message":"https://b3463c59c8f87072d3e9-87141d8cdbf10530595552debffaf82b.ssl.cf1.rackcdn.com/openstack/d9661a56cf4247dcae457ab6860ff481/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_goals.GoalsTests.test_goals_match_watcher_api/index.html\nlgtm","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d79f00de5b45bd79b4c151ff96232edd78acbe0e","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class GoalsTests(playwright_base.PlaywrightTestCase):"},{"line_number":22,"context_line":"    \"\"\"Integration tests for Goals using Playwright.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def test_goals_match_watcher_api(self):"},{"line_number":25,"context_line":"        \"\"\"Test that goals displayed in UI match those from Watcher API."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":15,"id":"7b33479c_ad14ad18","line":24,"range":{"start_line":24,"start_character":8,"end_line":24,"end_character":36},"in_reply_to":"9477af73_55b0ebde","updated":"2026-07-30 16:52:40.000000000","message":"Done","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9a93a142b41d28a771051d1a2024b7b1f8f85afc","unresolved":true,"context_lines":[{"line_number":69,"context_line":"            self.expect("},{"line_number":70,"context_line":"                row.get_by_text(display_name, exact\u003dTrue)"},{"line_number":71,"context_line":"            ).to_be_visible()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.take_screenshot(\"all_goals_verified\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"8140a9b2_541cee4d","line":72,"updated":"2026-07-16 13:27:46.000000000","message":"Same comment as for strategies.","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"9e0d381b6d2470a4bcbfbf867c7892e93f30c4f7","unresolved":true,"context_lines":[{"line_number":69,"context_line":"            self.expect("},{"line_number":70,"context_line":"                row.get_by_text(display_name, exact\u003dTrue)"},{"line_number":71,"context_line":"            ).to_be_visible()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.take_screenshot(\"all_goals_verified\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"2a1a7e5b_312e26d2","line":72,"in_reply_to":"8140a9b2_541cee4d","updated":"2026-07-20 13:11:34.000000000","message":"extended to check on each goal, and compare information from horizon with the information from the api","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"3bd0b1c3fbd80afd4e7037467ee29999f6d15c4c","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        if not api_specs:"},{"line_number":100,"context_line":"            # No efficacy specifications - verify UI shows this"},{"line_number":101,"context_line":"            # Look for \"No items to display\" or similar message"},{"line_number":102,"context_line":"            no_data_msg \u003d self.page.get_by_text(\"No items to display\")"},{"line_number":103,"context_line":"            self.assertGreater("},{"line_number":104,"context_line":"                no_data_msg.count(),"},{"line_number":105,"context_line":"                0,"}],"source_content_type":"text/x-python","patch_set":22,"id":"a3d5f3b0_0c49cf5d","line":102,"updated":"2026-07-21 10:01:50.000000000","message":"In _verify_efficacy_specifications, when a goal has no efficacy specs, the test checks for the \u0027No items to display\u0027 message using no_data_msg.count() which is a synchronous snapshot without auto-waiting. Every other assertion in the test uses self.expect(...).to_be_visible(timeout\u003d...) which aut...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Intermittent false test failures in CI when the detail page is slow to render, undermining the reliability of the entire test suite.\n\n**Suggestion**:\nReplace the assertGreater(no_data_msg.count(), 0, ...) pattern with self.expect(no_data_msg).to_be_visible(timeout\u003d5000) to enable auto-waiting, consistent with the rest of the test.","commit_id":"55a86142f037a0ade4cb052774eb665fe1bcead4"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d79f00de5b45bd79b4c151ff96232edd78acbe0e","unresolved":true,"context_lines":[{"line_number":22,"context_line":"class GoalsTests(playwright_base.PlaywrightTestCase):"},{"line_number":23,"context_line":"    \"\"\"Integration tests for Goals using Playwright.\"\"\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def test_goals_match_watcher_api(self):"},{"line_number":26,"context_line":"        \"\"\"Test that goals displayed in UI match those from Watcher API."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":24,"id":"154eeb31_4fddac78","line":25,"range":{"start_line":25,"start_character":8,"end_line":25,"end_character":36},"updated":"2026-07-30 16:52:40.000000000","message":"looks good: https://c7b5eb1ea7a5841f9463-ee688885bbf3a755ead0851cb0c23f69.ssl.cf5.rackcdn.com/openstack/c6cb515e2a8046c089a8997de69169df/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_goals.GoalsTests.test_goals_match_watcher_api/index.html\n\nThe page also have Related Strategies that could be evaluated, but i think this have a good coverage.","commit_id":"03805aef23f1f5f3726582a483b1433314373363"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"932e9c39ed44d0e7d9b22aefd4ecd2e4f317d44e","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        self.take_screenshot(f\"goal_{idx}_{goal_name}_efficacy_specs_count_ok\")"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        # Verify each specification"},{"line_number":120,"context_line":"        for spec_idx, api_spec in enumerate(api_specs):"},{"line_number":121,"context_line":"            spec_name \u003d api_spec[\u0027name\u0027]"},{"line_number":122,"context_line":"            spec_description \u003d api_spec[\u0027description\u0027]"},{"line_number":123,"context_line":"            spec_unit \u003d api_spec.get(\u0027unit\u0027) or \u0027-\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"3ef3d8ec_5875e51e","line":120,"updated":"2026-07-30 13:27:18.000000000","message":"The inner loop in _verify_efficacy_specifications uses enumerate(api_specs) and unpacks into spec_idx and api_spec, but spec_idx is never referenced anywhere in the loop body. The variable adds visual noise and could confuse future readers.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Minor code clarity issue. No functional impact, but unused variables reduce readability.\n\n**Recommendation**:\nReplace \u0027for spec_idx, api_spec in enumerate(api_specs):\u0027 with \u0027for api_spec in api_specs:\u0027 since the index is not needed.","commit_id":"99c57210a1461a12f7ec541c9f10fc5d5d911211"}],"watcher_dashboard/test/integration/test_existing_strategies.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a55ada967b06ebabd7d813fc34dd663921f402b6","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\"\"\"Integration tests for Strategies using Playwright.\"\"\""},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from django import test"},{"line_number":16,"context_line":"from playwright import sync_api as playwright_sync"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from watcher_dashboard.test.integration import playwright_base"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1d4df97c_2456c204","line":16,"updated":"2026-07-06 10:22:31.000000000","message":"Imports `playwright_sync` solely to call `playwright_sync.expect(...)`, but the base class already exposes the same helper as `self.expect`, which the sibling audit-workflow test uses consistently.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Consistency with the sibling integration test and with the base class design, plus removal of an import that is no longer needed.\n\n**Recommendation**:\nDrop the `from playwright import sync_api as playwright_sync` import and replace each `playwright_sync.expect(...)` call with `self.expect(...)`, matching how PlaywrightTestCase exposes `expect \u003d sync_api.expect` and how test_playwright_audit_workflow.py uses it.","commit_id":"0d38af2430e64d25b915052f7177c6a03c17db56"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a55ada967b06ebabd7d813fc34dd663921f402b6","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            display_name \u003d api_strategy.display_name"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            # Locate the row by strategy name"},{"line_number":70,"context_line":"            row \u003d self.page.locator("},{"line_number":71,"context_line":"                f\"tbody tr[data-display\u003d\u0027{strategy_name}\u0027]\""},{"line_number":72,"context_line":"            )"},{"line_number":73,"context_line":"            playwright_sync.expect(row).to_be_visible("}],"source_content_type":"text/x-python","patch_set":5,"id":"0a003dd8_05462ce3","line":70,"updated":"2026-07-06 10:22:31.000000000","message":"Locates rows with `tbody tr[data-display\u003d\u0027{strategy_name}\u0027]`, coupling the test to Horizon internals and breaking on names that contain apostrophes or other quote characters.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Role-based locators survive markup churn and special characters, and they align this test with the existing sibling-test convention.\n\n**Recommendation**:\nPrefer the role-based pattern already used in test_playwright_audit_workflow.py: `self.page.get_by_role(\"row\").filter(has_text\u003dstrategy_name)`. If the attribute selector must stay, at least escape single quotes/apostrophes in strategy_name before interpolating it into the selector.","commit_id":"0d38af2430e64d25b915052f7177c6a03c17db56"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4fb85e43eb65176253e13f78d7d50012b259b4a4","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            row \u003d self.page.locator("},{"line_number":71,"context_line":"                f\"tbody tr[data-display\u003d\u0027{strategy_name}\u0027]\""},{"line_number":72,"context_line":"            )"},{"line_number":73,"context_line":"            ("},{"line_number":74,"context_line":"                playwright_sync.expect(row).to_be_visible(timeout\u003d5000),"},{"line_number":75,"context_line":"                f\"Strategy \u0027{strategy_name}\u0027 not found in UI\","},{"line_number":76,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":8,"id":"72d26d76_df1e8c3f","line":73,"updated":"2026-07-07 09:10:39.000000000","message":"The pattern `(playwright_sync.expect(row).to_be_visible(timeout\u003d5000), f\"Strategy \u0027...\u0027 not found in UI\")` creates a bare tuple that is immediately discarded. The expect() call does execute and will raise on failure, but the error message string is never surfaced. This is the identical pattern as...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: When a strategy is missing from the UI, the test fails with Playwright\u0027s generic timeout message instead of the intended descriptive message. Developers debugging test failures lose the context the author intended to provide.\n\n**Suggestion**:\nRemove the tuple wrapper and call `playwright_sync.expect(row).to_be_visible(timeout\u003d5000)` directly, or use a try/except to re-raise with the custom message. Align with the existing pattern in test_playwright_audit_workflow.py.","commit_id":"b801385945f4710b95b5e72cbeb8049237a7413e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"84c8cfab31990538f7ed8e9e9df8ac830e0e6358","unresolved":true,"context_lines":[{"line_number":21,"context_line":"class StrategiesTests(playwright_base.PlaywrightTestCase):"},{"line_number":22,"context_line":"    \"\"\"Integration tests for Strategies using Playwright.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def test_strategies_match_watcher_api(self):"},{"line_number":25,"context_line":"        \"\"\"Test that strategies displayed in UI match those from Watcher API."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":15,"id":"13bbcf46_8b79804e","line":24,"range":{"start_line":24,"start_character":8,"end_line":24,"end_character":41},"updated":"2026-07-16 17:13:49.000000000","message":"https://b3463c59c8f87072d3e9-87141d8cdbf10530595552debffaf82b.ssl.cf1.rackcdn.com/openstack/d9661a56cf4247dcae457ab6860ff481/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_strategies.StrategiesTests.test_strategies_match_watcher_api/index.html\nlooks good","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d79f00de5b45bd79b4c151ff96232edd78acbe0e","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class StrategiesTests(playwright_base.PlaywrightTestCase):"},{"line_number":22,"context_line":"    \"\"\"Integration tests for Strategies using Playwright.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def test_strategies_match_watcher_api(self):"},{"line_number":25,"context_line":"        \"\"\"Test that strategies displayed in UI match those from Watcher API."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f0e5007_94fc5e4a","line":24,"range":{"start_line":24,"start_character":8,"end_line":24,"end_character":41},"in_reply_to":"13bbcf46_8b79804e","updated":"2026-07-30 16:52:40.000000000","message":"Done","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9a93a142b41d28a771051d1a2024b7b1f8f85afc","unresolved":true,"context_lines":[{"line_number":73,"context_line":"            self.expect("},{"line_number":74,"context_line":"                row.get_by_text(display_name, exact\u003dTrue)"},{"line_number":75,"context_line":"            ).to_be_visible()"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        self.take_screenshot(\"all_strategies_verified\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"f7e26b57_de9151c9","line":76,"updated":"2026-07-16 13:27:46.000000000","message":"I think this is not testing the details view of strategies, right?. IMO, it\u0027d be good to extend this test to click on one of the strategies and check that the strategy details page is properly displayed.","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"9e0d381b6d2470a4bcbfbf867c7892e93f30c4f7","unresolved":true,"context_lines":[{"line_number":73,"context_line":"            self.expect("},{"line_number":74,"context_line":"                row.get_by_text(display_name, exact\u003dTrue)"},{"line_number":75,"context_line":"            ).to_be_visible()"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        self.take_screenshot(\"all_strategies_verified\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"08498f28_e3985092","line":76,"in_reply_to":"81500f22_e82acc56","updated":"2026-07-20 13:11:34.000000000","message":"I have added code for verifying extended information for each strategy, entering on each of them and comparing the information on horizon to the information from the api","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"66846610dbe608a25920f4dad3984c2df861eb06","unresolved":true,"context_lines":[{"line_number":73,"context_line":"            self.expect("},{"line_number":74,"context_line":"                row.get_by_text(display_name, exact\u003dTrue)"},{"line_number":75,"context_line":"            ).to_be_visible()"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        self.take_screenshot(\"all_strategies_verified\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"81500f22_e82acc56","line":76,"in_reply_to":"f7e26b57_de9151c9","updated":"2026-07-16 17:17:43.000000000","message":"right, it is only checking the display_name, not strategy details. The test could be extended to check for details on each strategy..","commit_id":"741f180af3387307929aad5bc72d1ac3c9319169"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"73dcead36e865f3e63a01db23340f1e65f2142ed","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        :param strategy_name: Name of the strategy for screenshot naming"},{"line_number":101,"context_line":"        :param idx: Index for screenshot naming"},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        import json"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        import yaml"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"5576238a_b7054743","line":103,"updated":"2026-07-20 15:20:40.000000000","message":"The method _verify_parameter_specs contains `import json` and `import yaml` inside the function body (lines 103-105) instead of at the top of the module. All other test files in the integration directory place imports at module level following PEP 8 and OpenStack conventions.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Function-level imports that have no justification deviate from the established project pattern and PEP 8, reducing code consistency and readability. This is not caught by flake8/hacking since E402 only covers module-level import placement.\n\n**Recommendation**:\nMove `import json` to the stdlib import group and `import yaml` to the third-party import group at the top of the file, alongside the existing `from django import test` and `from watcher_dashboard.test.integration import playwright_base` imports.","commit_id":"08f3be590fbe63d2aed8e3e2aaec1fc228b35a88"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"70d712e67c99f0944c8461a1de9bd2f85f03e26e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        :param strategy_name: Name of the strategy for screenshot naming"},{"line_number":101,"context_line":"        :param idx: Index for screenshot naming"},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        import json"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        import yaml"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"4973e28c_4fd2eb13","line":103,"updated":"2026-07-21 09:40:36.000000000","message":"The `import json` and `import yaml` statements are placed inside the `_verify_parameter_specs` method instead of at the top of the module. This violates PEP 8 import placement conventions and is inconsistent with the rest of the codebase where yaml is imported at module level.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: No functional impact since Python caches imports, but it degrades code clarity, hides dependencies, and creates inconsistency with the rest of the codebase. A reviewer reading the module header cannot see that yaml is a dependency.\n\n**Recommendation**:\nMove `import json` and `import yaml` to the top of the file, following stdlib-first then third-party import ordering: `import json` (stdlib) before `from django import test` (third-party), and `import yaml` in the third-party group.","commit_id":"18ce5f9971a47f25b53d0a096c64660e510bd580"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d79f00de5b45bd79b4c151ff96232edd78acbe0e","unresolved":false,"context_lines":[{"line_number":26,"context_line":"class StrategiesTests(playwright_base.PlaywrightTestCase):"},{"line_number":27,"context_line":"    \"\"\"Integration tests for Strategies using Playwright.\"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_strategies_match_watcher_api(self):"},{"line_number":30,"context_line":"        \"\"\"Test that strategies displayed in UI match those from Watcher API."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        This test verifies that the Horizon dashboard correctly displays all"}],"source_content_type":"text/x-python","patch_set":25,"id":"2f769da7_d07a23b0","line":29,"range":{"start_line":29,"start_character":8,"end_line":29,"end_character":41},"updated":"2026-07-30 16:52:40.000000000","message":"looks good\nhttps://855f4dca0fefe19148c1-90a9b723857c85e2536fe5d9751b99c8.ssl.cf1.rackcdn.com/openstack/7e9e2e9fd16c4c9ea4bf6edf087939f6/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_existing_strategies.StrategiesTests.test_strategies_match_watcher_api/index.html","commit_id":"99c57210a1461a12f7ec541c9f10fc5d5d911211"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8dfee5e06c0a72c50a4057cf337691b84357923d","unresolved":true,"context_lines":[{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        # Get the text content from the UI"},{"line_number":124,"context_line":"        ui_spec_text \u003d params_spec_dd.inner_text().strip()"},{"line_number":125,"context_line":"        self.take_screenshot("},{"line_number":126,"context_line":"            f\"strategy_{idx}_{strategy_name}_parameter_specs_visible\""},{"line_number":127,"context_line":"        )"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"17bc1fb3_1fc5bec7","line":125,"updated":"2026-07-30 16:07:00.000000000","message":"We have three times the same screenshot, as we are not moving to any different page between:\n\nf\"strategy_{idx}_{strategy_name}_details_opened\"\nf\"strategy_{idx}_{strategy_name}_parameter_specs_visible\"\nf\"strategy_{idx}_{strategy_name}_parameter_specs_verified\"\nf\"strategy_{idx}_{strategy_name}_no_parameter_specs\"\n\nNot a big problem but posible optimization to avoid wasting space. Can be fixed in follow-up","commit_id":"99c57210a1461a12f7ec541c9f10fc5d5d911211"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d79f00de5b45bd79b4c151ff96232edd78acbe0e","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        # API specs is already a dict"},{"line_number":145,"context_line":"        api_spec_dict \u003d api_specs"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # UI shows it as YAML format, parse it"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            ui_spec_dict \u003d yaml.safe_load(ui_spec_text)"},{"line_number":150,"context_line":"        except yaml.YAMLError:"}],"source_content_type":"text/x-python","patch_set":25,"id":"669fb747_74ac7618","line":147,"range":{"start_line":147,"start_character":0,"end_line":147,"end_character":46},"updated":"2026-07-30 16:52:40.000000000","message":"The api returns as json, but the view print as yaml, but when the view fails to dump as yaml it tries to dump as json... so yeah\n we could expect any of both...","commit_id":"99c57210a1461a12f7ec541c9f10fc5d5d911211"}]}
