)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"762ede914ec8100e65fe7fc3d24b5b77f228b8e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3b457cc4_5382f9e2","updated":"2026-02-17 14:29:49.000000000","message":"recheck","commit_id":"73b1b155af552a666731f3f2fe460ef82763a974"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c179c45b8a19e69120b6fcf0cca1a1e91e703366","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"fb9c2259_8490df3d","updated":"2026-03-26 18:05:47.000000000","message":"recheck  is horizong fixed yet?\ni assume yes","commit_id":"7769d925122da32cbd2aa92a73bbf12b521aa829"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7a51a2e603f696a4b6c63440996a88f279cfe051","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"fdf5cc71_290e95d5","updated":"2026-04-07 15:24:39.000000000","message":"you need to rebase thsi on teh latest version of its parent and likely resove teh merge conflcit as i think you have duplciated code in both.","commit_id":"b06fc8ef0d9df01a501267c4a424d86dfa6d81bd"}],"watcher_dashboard/test/integration/playwright_base.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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":286,"context_line":"            self.fail(\"Could not extract UUID from href: %s\" % href)"},{"line_number":287,"context_line":"        return match.group(1)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def wait_for_audit_terminal_state("},{"line_number":290,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":291,"context_line":"    ):"},{"line_number":292,"context_line":"        \"\"\"Wait for audit to reach any terminal state on current page.\"\"\""}],"source_content_type":"text/x-python","patch_set":32,"id":"0f5da6fc_ad7f1540","line":289,"updated":"2026-06-23 05:10:22.000000000","message":"Both wait_for_audit_terminal_state and wait_for_action_plan_terminal_state share identical default terminal_states tuples (\u0027SUCCEEDED\u0027, \u0027FAILED\u0027, \u0027CANCELLED\u0027, \u0027DELETED\u0027) and identical poll loop structure, differing only in the scoping logic.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Extracting a shared _wait_for_terminal_state(method, timeout, poll_interval, terminal_states) helper would eliminate duplication and ensure consistent timeout/poll behavior across both wait methods.\n\n**Recommendation**:\nRefactor to a common polling loop that accepts a callable for the state-checking logic, reducing code duplication from ~55 combined lines to ~30.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":286,"context_line":"            self.fail(\"Could not extract UUID from href: %s\" % href)"},{"line_number":287,"context_line":"        return match.group(1)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def wait_for_audit_terminal_state("},{"line_number":290,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":291,"context_line":"    ):"},{"line_number":292,"context_line":"        \"\"\"Wait for audit to reach any terminal state on current page.\"\"\""}],"source_content_type":"text/x-python","patch_set":32,"id":"defa4f73_b52caa69","line":289,"updated":"2026-06-23 05:10:22.000000000","message":"wait_for_audit_terminal_state uses page-wide get_by_text(state, exact\u003dTrue).count() \u003e 0, which can match unrelated text on the page. The sibling wait_for_action_plan_terminal_state already scopes within vs outside tables; this audit method lacks similar scoping.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: The wait helper may return prematurely if a terminal state string appears elsewhere on the page before the audit row itself transitions, producing a false positive pass or a confusing failure downstream.\n\n**Suggestion**:\nScope the text search to the audit detail region or the specific status field element rather than the entire page body, similar to the approach used in wait_for_action_plan_terminal_state.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        while time.time() - start_time \u003c timeout:"},{"line_number":325,"context_line":"            self.page.reload()"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                for state in states:"},{"line_number":328,"context_line":"                    total \u003d self.page.get_by_text(state, exact\u003dTrue).count()"},{"line_number":329,"context_line":"                    if total \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":32,"id":"f20af73c_c5561979","line":326,"updated":"2026-06-23 05:10:22.000000000","message":"wait_for_action_plan_terminal_state silently swallows all sync_api.Error and AttributeError exceptions in the polling loop with a bare \u0027pass\u0027. If the page is in a transient error state, the method polls until timeout then raises a generic TimeoutError that obscures the underlying Playwright error.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Debugging test timeouts becomes harder because the actual Playwright error (e.g., element detached, navigation interrupted) is swallowed and never surfaced in the failure output.\n\n**Suggestion**:\nAt minimum, log the caught exception at DEBUG level inside the except block before continuing, so the underlying error is visible in verbose test output when diagnosing flaky failures.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":531,"context_line":"        )"},{"line_number":532,"context_line":"        sync_api.expect(success_msg).to_be_visible(timeout\u003dtimeout)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"    def wait_for_audit_status(self, status, timeout\u003d120, poll_interval\u003d5):"},{"line_number":535,"context_line":"        \"\"\"Wait for audit status with polling."},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"        :param status: Expected status string"}],"source_content_type":"text/x-python","patch_set":32,"id":"230ad898_b1bfd88c","line":534,"updated":"2026-06-23 05:10:22.000000000","message":"The new wait_for_audit_terminal_state method coexists with the older wait_for_audit_status method (line 534) in the same base class, both polling audit state via page reloads. This creates two parallel mechanisms for the same concept.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Consolidating into a single well-documented wait method reduces maintenance burden and prevents divergence in polling behavior between tests that use the old vs new helper.\n\n**Recommendation**:\nIn a follow-up patch, consider deprecating or merging wait_for_audit_status into wait_for_audit_terminal_state (or vice versa), updating existing callers in test_playwright_audit_workflow.py which currently calls wait_for_audit_status(\u0027Succeeded\u0027).","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":286,"context_line":"            self.fail(\"Could not extract UUID from href: %s\" % href)"},{"line_number":287,"context_line":"        return match.group(1)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def wait_for_audit_terminal_state("},{"line_number":290,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":291,"context_line":"    ):"},{"line_number":292,"context_line":"        \"\"\"Wait for audit to reach any terminal state on current page.\"\"\""}],"source_content_type":"text/x-python","patch_set":33,"id":"d6fbbf7b_606dc6f9","line":289,"updated":"2026-06-30 08:33:03.000000000","message":"The new wait_for_audit_terminal_state overlaps the older wait_for_audit_status (playwright_base.py:534): both poll the page for an audit status, but one takes a single status and the other a tuple, with different default timeouts (120 vs 300).\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Consolidating onto one helper avoids divergence where one poll loop gets improved (e.g. table exclusion) and the other does not, and removes the SUCCEEDED-vs-Succeeded casing inconsistency between the two tests.\n\n**Recommendation**:\nIn a follow-up, make wait_for_audit_status delegate to wait_for_audit_terminal_state (accepting a single status as a one-element terminal set) and standardize the casing used across both tests.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":286,"context_line":"            self.fail(\"Could not extract UUID from href: %s\" % href)"},{"line_number":287,"context_line":"        return match.group(1)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def wait_for_audit_terminal_state("},{"line_number":290,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":291,"context_line":"    ):"},{"line_number":292,"context_line":"        \"\"\"Wait for audit to reach any terminal state on current page.\"\"\""}],"source_content_type":"text/x-python","patch_set":33,"id":"26d77698_0a365394","line":289,"updated":"2026-06-30 08:33:03.000000000","message":"wait_for_audit_terminal_state scans the whole page via get_by_text(state, exact\u003dTrue) with no table-exclusion logic. The audit detail view renders related action plans/actions in nested tables, so a SUCCEEDED/FAILED string in a child table can satisfy the check before the audit reaches that state.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: The skip test asserts SUCCEEDED on this return value. A stale/unrelated SUCCEEDED from a nested table could let the test proceed to skip an action before the decision engine finished, causing intermittent failures or false greens rather than deterministic results.\n\n**Suggestion**:\nScope the search to the audit status element the way wait_for_action_plan_terminal_state does (page count minus in-table count), or locate the specific status cell on the audit detail panel by role/label and assert against it directly instead of a full-page scan.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            \"Audit did not reach terminal states %s in %ss\" % (states, timeout)"},{"line_number":310,"context_line":"        )"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    def wait_for_action_plan_terminal_state("},{"line_number":313,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":314,"context_line":"    ):"},{"line_number":315,"context_line":"        \"\"\"Wait for action plan terminal state, excluding in-table matches.\"\"\""}],"source_content_type":"text/x-python","patch_set":33,"id":"dc236efc_29a9e41c","line":312,"updated":"2026-06-30 08:33:03.000000000","message":"wait_for_audit_terminal_state and wait_for_action_plan_terminal_state share an almost identical poll loop (reload, count states, sleep, raise TimeoutError). The action-plan variant only adds the in-table subtraction, yet both copy the identical terminal_states default tuple.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: A single private _poll_for_terminal_state(exclude_tables\u003dFalse) helper removes ~30 lines of duplication, keeps the default terminal-state tuple in sync, and ensures future fixes (logging, better reload handling) apply to both call sites consistently.\n\n**Recommendation**:\nExtract a shared _poll_for_terminal_state(exclude_tables\u003dFalse) method that both public methods delegate to, passing the flag to control in-table subtraction. Keep the two public names as thin wrappers for readability.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        while time.time() - start_time \u003c timeout:"},{"line_number":325,"context_line":"            self.page.reload()"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                for state in states:"},{"line_number":328,"context_line":"                    total \u003d self.page.get_by_text(state, exact\u003dTrue).count()"},{"line_number":329,"context_line":"                    if total \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":33,"id":"47371693_4fa42f80","line":326,"updated":"2026-06-30 08:33:03.000000000","message":"wait_for_action_plan_terminal_state wraps the per-iteration scan in except (sync_api.Error, AttributeError): pass with no logging, swallowing all Playwright API errors and AttributeError for the whole 300s timeout window.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: When the loop fails in CI the only signal is a TimeoutError after up to 5 minutes with no sign that every reload raised. This makes flaky integration failures expensive to diagnose and masks AttributeError selector typos during development.\n\n**Suggestion**:\nLog the swallowed exception at debug level (LOG.debug(\u0027terminal-state poll error: %s\u0027, exc)) so CI artifacts show whether the loop was genuinely polling or erroring every iteration. Keep the retry but make it observable.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            user_domain_name\u003dconfig.get_user_domain_name(),"},{"line_number":159,"context_line":"            project_domain_name\u003dconfig.get_project_domain_name(),"},{"line_number":160,"context_line":"        )"},{"line_number":161,"context_line":"        session \u003d keystone_session.Session("},{"line_number":162,"context_line":"            auth\u003dauth, verify\u003dnot config.is_insecure()"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        self.client \u003d watcher_client.Client(\u00271\u0027, session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":34,"id":"b2905412_4bcca602","line":161,"updated":"2026-07-02 09:13:03.000000000","message":"WatcherClientFixture disables TLS verification when config.is_insecure() is True, and credentials are read from playwright_config. While this is test code, insecure TLS defaults in shared config can cause tests to silently skip certificate validation against real endpoints.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: In a CI or shared dev cloud, disabled verification masks MITM and cert-misconfiguration issues, and the password-based credential flow has no obvious secret management.\n\n**Suggestion**:\nDefault is_insecure() to False and require it to be explicitly opted into via an env var named for the purpose (e.g. PLAYWRIGHT_INSECURE\u003d1). Document that credentials should come from env vars or a secrets backend, not plaintext config.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":286,"context_line":"            self.fail(\"Could not extract UUID from href: %s\" % href)"},{"line_number":287,"context_line":"        return match.group(1)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def wait_for_audit_terminal_state("},{"line_number":290,"context_line":"        self, timeout\u003d300, poll_interval\u003d5, terminal_states\u003dNone"},{"line_number":291,"context_line":"    ):"},{"line_number":292,"context_line":"        \"\"\"Wait for audit to reach any terminal state on current page.\"\"\""}],"source_content_type":"text/x-python","patch_set":34,"id":"51804f47_3653a3fa","line":289,"updated":"2026-07-02 09:13:03.000000000","message":"wait_for_audit_terminal_state and wait_for_action_plan_terminal_state duplicate the same reload/poll/time.sleep loop with only the match predicate differing. The audit variant lacks the table-exclusion guard the action-plan variant has, making them inconsistent.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Duplicated polling logic is a maintenance burden: a timing fix must be applied in two places, and the inconsistency (audit does not exclude in-table matches) invites the false-positive bug noted separately.\n\n**Suggestion**:\nExtract a single _poll_for_state(predicate, timeout, poll_interval) helper and have both methods call it with their specific match function. This also makes it easy to add jitter or exponential backoff in one place.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        )"},{"line_number":299,"context_line":"        start_time \u003d time.time()"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        while time.time() - start_time \u003c timeout:"},{"line_number":302,"context_line":"            self.page.reload()"},{"line_number":303,"context_line":"            for state in states:"},{"line_number":304,"context_line":"                if self.page.get_by_text(state, exact\u003dTrue).count() \u003e 0:"}],"source_content_type":"text/x-python","patch_set":34,"id":"dc441838_73b2cf9e","line":301,"updated":"2026-07-02 09:13:03.000000000","message":"wait_for_audit_terminal_state returns the first matching status string found anywhere on the page via get_by_text(state, exact\u003dTrue). On an audit detail page, a terminal state from a Related Action Plan or a stale row could be matched, causing a false positive.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The test could wrongly conclude the audit succeeded when the matched text belongs to a related action plan or another table row, producing flaky or incorrect pass results in CI.\n\n**Suggestion**:\nScope the text search to the audit detail region/table rather than the whole page, e.g. locate the specific status cell by its label or within the audit overview section, mirroring the table-exclusion logic already used in wait_for_action_plan_terminal_state.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        self.take_screenshot(\"audits_page_loaded\")"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"        # The button to open modal - prefer user-visible text over classes."},{"line_number":402,"context_line":"        try:"},{"line_number":403,"context_line":"            create_link \u003d self.page.get_by_role(\"link\", name\u003d\"Create Audit\")"},{"line_number":404,"context_line":"            create_link.click()"},{"line_number":405,"context_line":"        except sync_api.TimeoutError:"}],"source_content_type":"text/x-python","patch_set":34,"id":"ba0d15ce_f0953ae9","line":402,"updated":"2026-07-02 09:13:03.000000000","message":"create_audit catches sync_api.TimeoutError to fall back from \u0027Create Audit\u0027 to \u0027New Audit\u0027 link text. The fallback link is clicked outside the try, so a second TimeoutError is uncaught and surfaces an opaque error.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Clearer failure messages speed up CI triage when the create-audit button label changes between Horizon versions.\n\n**Recommendation**:\nWrap both attempts or use Playwright\u0027s locator.or_() to try both selectors in one resolution, and on final failure raise a descriptive AssertionError naming the expected button labels.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        self.wait_for_success_message()"},{"line_number":430,"context_line":"        self.take_screenshot(\"audit_created\")"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        audit_uuid \u003d self._get_audit_uuid_via_api(audit_name)"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"        self.addCleanup(self._delete_audit_via_api, audit_uuid)"},{"line_number":435,"context_line":"        self.addCleanup("}],"source_content_type":"text/x-python","patch_set":34,"id":"efee5d8c_47ee1d4c","line":432,"updated":"2026-07-02 09:13:03.000000000","message":"create_audit passes an unguarded potentially-None audit_uuid to addCleanup. _get_audit_uuid_via_api returns None on lookup failure, but the result feeds directly into _delete_action_plans_for_audit_via_api, which calls action_plan.list(audit\u003dNone).\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: If the audit UUID lookup fails, cleanup calls action_plan.list(audit\u003dNone) which lists ALL action plans in the deployment and then deletes each one, destroying unrelated test or tenant data.\n\n**Priority**: Before merge\n**Why This Matters**: A cleanup path that can delete every action plan in the cloud when a single UUID lookup fails is a data-loss hazard in any shared CI or dev environment. The broad exception swallowing in the cleanup methods masks the underlying error.\n\n**Recommendation**:\nGuard the UUID before registering cleanups: if audit_uuid is None, call self.fail(...) or raise immediately so the test fails loudly instead of running destructive cleanup. Also have _delete_action_plans_for_audit_via_api skip early when audit_uuid is falsy rather than calling action_plan.list(audit\u003dNone).","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        if timeout is None:"},{"line_number":524,"context_line":"            timeout \u003d config.get_timeout()"},{"line_number":525,"context_line":"        # Try multiple selectors for success message"},{"line_number":526,"context_line":"        success_msg \u003d ("},{"line_number":527,"context_line":"            self.page.get_by_role(\"alert\")"},{"line_number":528,"context_line":"            .or_(self.page.get_by_text(\"Success\"))"},{"line_number":529,"context_line":"            .or_(self.page.get_by_text(\"Successfully\"))"}],"source_content_type":"text/x-python","patch_set":34,"id":"7ccdad19_34fb3678","line":526,"updated":"2026-07-02 09:13:03.000000000","message":"wait_for_success_message chains .or_() locators across alert role and generic text. Playwright .or_() resolves to the first available match, which can be a stale alert from a prior step.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: A more precise success-message locator reduces false-positive waits and improves test determinism across the create_audit and skip-action flows.\n\n**Recommendation**:\nTarget the Horizon success alert container specifically (e.g. .messages .alert-success or a role\u003dalert with a success class) rather than broad text matching, so stale or unrelated alerts do not satisfy the wait.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"5da37f84c1995bf0816dd5859cf7025e4f7863f3","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":2,"context_line":"# not use this file except in compliance with the License. You may obtain"},{"line_number":3,"context_line":"# a copy of the License at"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":35,"id":"4c350397_62633995","line":1,"updated":"2026-07-02 15:54:05.000000000","message":"License header in playwright_base.py uses \u0027# Licensed\u0027 (single leading space) while the two test files use \u0027#    Licensed\u0027 (four-space indent). The four-space form is the canonical OpenStack Apache header used across the watcher-dashboard tree.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Inconsistent license header formatting within the same new module group. It is cosmetic and does not affect licensing, but breaks the otherwise uniform header convention in the repository.\n\n**Suggestion**:\nAlign playwright_base.py header to the four-space \u0027#    Licensed\u0027 form used by test_playwright_audit_workflow.py and test_playwright_skip_action_workflow.py for consistency.","commit_id":"208b5f46e1a8ab8edfa799eaf5e8c7fc353473ec"},{"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":"5da37f84c1995bf0816dd5859cf7025e4f7863f3","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            \u0027CANCELLED\u0027,"},{"line_number":297,"context_line":"            \u0027DELETED\u0027,"},{"line_number":298,"context_line":"        )"},{"line_number":299,"context_line":"        start_time \u003d time.time()"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        while time.time() - start_time \u003c timeout:"},{"line_number":302,"context_line":"            self.page.reload()"}],"source_content_type":"text/x-python","patch_set":35,"id":"01b83cc2_022ce3e3","line":299,"updated":"2026-07-02 15:54:05.000000000","message":"time.sleep-based reload polling loops are used in wait_for_audit_status, wait_for_audit_terminal_state, and wait_for_action_plan_terminal_state. Fixed sleeps with hard reloads are a primary source of integration-test flakiness, yet the commit advertises Playwright auto-waiting.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Tests either wait longer than necessary (fixed 5s sleep even when state changed immediately) or hammer the page with reloads under load. The reload-then-count pattern can also race with background XHR updates, producing intermittent false negatives.\n\n**Suggestion**:\nPrefer Playwright built-in auto-waiting: use expect(locator).to_be_visible()/to_have_text() with a generous timeout on a scoped state locator rather than a manual reload+sleep loop. If polling is unavoidable, drive it off page.wait_for_load_state() and the configured timeout rather than a separate poll_interval.","commit_id":"208b5f46e1a8ab8edfa799eaf5e8c7fc353473ec"},{"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":"5da37f84c1995bf0816dd5859cf7025e4f7863f3","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        if timeout is None:"},{"line_number":524,"context_line":"            timeout \u003d config.get_timeout()"},{"line_number":525,"context_line":"        # Try multiple selectors for success message"},{"line_number":526,"context_line":"        success_msg \u003d ("},{"line_number":527,"context_line":"            self.page.get_by_role(\"alert\")"},{"line_number":528,"context_line":"            .or_(self.page.get_by_text(\"Success\"))"},{"line_number":529,"context_line":"            .or_(self.page.get_by_text(\"Successfully\"))"}],"source_content_type":"text/x-python","patch_set":35,"id":"f875a566_653f2588","line":526,"updated":"2026-07-02 15:54:05.000000000","message":"wait_for_success_message uses an alert.or_(get_by_text(\u0027Success\u0027)).or_(get_by_text(\u0027Successfully\u0027)).first chain. While valid Playwright, .first on a union locator means that if a matching element exists but is hidden (e.g. a stale alert), to_be_visible can resolve to the wrong element.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: A more robust locator scoped to the active toast/alert container reduces the chance of matching a stale or off-screen success message, improving reliability of every test that calls this helper.\n\n**Recommendation**:\nScope the locator to the visible message container, e.g. self.page.locator(\u0027.alert-success, [role\u003dalert]\u0027).first, or use page.wait_for_selector with a state\u003d\u0027visible\u0027 filter. Document the exact success-message DOM contract in the docstring so future UI changes update in one place.","commit_id":"208b5f46e1a8ab8edfa799eaf5e8c7fc353473ec"},{"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":"5da37f84c1995bf0816dd5859cf7025e4f7863f3","unresolved":false,"context_lines":[{"line_number":531,"context_line":"        )"},{"line_number":532,"context_line":"        sync_api.expect(success_msg).to_be_visible(timeout\u003dtimeout)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"    def wait_for_audit_status(self, status, timeout\u003d120, poll_interval\u003d5):"},{"line_number":535,"context_line":"        \"\"\"Wait for audit status with polling."},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"        :param status: Expected status string"}],"source_content_type":"text/x-python","patch_set":35,"id":"4c9c89e8_1f29acd2","line":534,"updated":"2026-07-02 15:54:05.000000000","message":"Casing inconsistency: wait_for_audit_status(\"Succeeded\") uses get_by_text(status, exact\u003dTrue). Playwright exact text matching is case-sensitive, but Watcher audit states are uppercase (\u0027SUCCEEDED\u0027). The sibling wait_for_audit_terminal_state checks \u0027SUCCEEDED\u0027. The mismatched call can fail to match.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: test_launch_audit_and_verify_action_plan can never satisfy the wait condition if the page renders the state as \u0027SUCCEEDED\u0027, causing it to loop until the 120s timeout and then raise TimeoutError, producing a consistently failing/flaky integration test.\n\n**Priority**: Before merge\n**Why This Matters**: A wait helper that compares against the wrong case for an exact text match is a latent correctness bug that turns into a hard test failure depending on how Horizon renders the state field. It also leaves two divergent helpers doing the same job with different casing contracts.\n\n**Recommendation**:\nStandardize on canonical uppercase Watcher state strings. Either update the caller in test_playwright_audit_workflow.py to pass \"SUCCEEDED\" and use wait_for_audit_terminal_state, or drop the legacy wait_for_audit_status method entirely. If case-insensitive matching is desired, use get_by_text(re.compile(re.escape(status), re.I)) instead of exact\u003dTrue.","commit_id":"208b5f46e1a8ab8edfa799eaf5e8c7fc353473ec"}],"watcher_dashboard/test/integration/test_playwright_audit_workflow.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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.create_audit(template_name, audit_name)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        # Wait for audit to complete"},{"line_number":65,"context_line":"        self.wait_for_audit_status(\"Succeeded\")"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # Verify action plan exists"},{"line_number":68,"context_line":"        self.page.goto(f\"{self.dashboard_url}/admin/action_plans/\")"}],"source_content_type":"text/x-python","patch_set":33,"id":"3650069d_08bb5909","line":65,"updated":"2026-06-30 08:33:03.000000000","message":"test_launch_audit_and_verify_action_plan still calls self.wait_for_audit_status(\u0027Succeeded\u0027) (capitalized) while the new terminal-state helpers and skip test use all-caps \u0027SUCCEEDED\u0027. This change touches the audit workflow file and would have been a natural place to align them.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: A single canonical status casing across integration tests reduces confusion about what the dashboard renders and prevents a future refactor from breaking one test while the other passes.\n\n**Recommendation**:\nVerify the exact casing the dashboard renders for audit status and use it consistently in both tests; if the page shows SUCCEEDED switch the older test to the terminal-state helper, otherwise align the helper\u0027s default tuple.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"}],"watcher_dashboard/test/integration/test_playwright_skip_action_workflow.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e8250a790843abe7315e5d333cec603e0979a09e","unresolved":true,"context_lines":[{"line_number":150,"context_line":"            \"SUCCEEDED\","},{"line_number":151,"context_line":"            final_plan_state,"},{"line_number":152,"context_line":"            \"Action plan did not succeed; state\u003d%s\" % final_plan_state,"},{"line_number":153,"context_line":"        )"},{"line_number":154,"context_line":"        self.take_screenshot(\"action_plan_succeeded\")"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # Status Message value (no role for dt/dd); assert by visible text"}],"source_content_type":"text/x-python","patch_set":1,"id":"4a2fc925_af3a2592","line":153,"updated":"2026-02-18 14:17:32.000000000","message":"In the screenshot for 031_action_plan_succeeded, https://4e653b44258ae0ef8e8c-431c4675dbf321a8fbd861f82b52f6e9.ssl.cf2.rackcdn.com/openstack/caf3bc578b1947e3a25e67101d8969ab/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_playwright_skip_action_workflow.SkipActionWorkflowTests.test_skip_action_workflow/031_action_plan_succeeded.png the action plan is still ongoing. I guess because it\u0027s taking the SUCCEEDED state of one of the actions.?\n\nWe should probably assert also in the Action Plan Status Message to have \"One or more actions were skipped.\" text","commit_id":"73b1b155af552a666731f3f2fe460ef82763a974"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"7e5b0b837068308faacde3741eeb32bddec821d9","unresolved":true,"context_lines":[{"line_number":150,"context_line":"            \"SUCCEEDED\","},{"line_number":151,"context_line":"            final_plan_state,"},{"line_number":152,"context_line":"            \"Action plan did not succeed; state\u003d%s\" % final_plan_state,"},{"line_number":153,"context_line":"        )"},{"line_number":154,"context_line":"        self.take_screenshot(\"action_plan_succeeded\")"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # Status Message value (no role for dt/dd); assert by visible text"}],"source_content_type":"text/x-python","patch_set":1,"id":"99d5cc48_403db15f","line":153,"in_reply_to":"4a2fc925_af3a2592","updated":"2026-02-22 07:47:21.000000000","message":"Done, Here is the last screenshot generated with above message https://06c425c005622414e7bc-fc8dbe66ba43ee6d3126c4b00a900b20.ssl.cf2.rackcdn.com/openstack/8b36c6c1a6754018af3e2c88d7611a9f/controller/logs/playwright/screenshots/watcher_dashboard.test.integration.test_playwright_skip_action_workflow.SkipActionWorkflowTests.test_skip_action_workflow/031_action_plan_succeeded.png and asseted in the code.","commit_id":"73b1b155af552a666731f3f2fe460ef82763a974"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class SkipActionWorkflowTests(playwright_base.PlaywrightTestCase):"},{"line_number":24,"context_line":"    \"\"\"Integration tests for skip action workflow.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def test_skip_action_workflow(self):"},{"line_number":27,"context_line":"        \"\"\"Test skipping an action and verifying plan succeeds with skip msg."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"        Steps:"}],"source_content_type":"text/x-python","patch_set":32,"id":"6c08962b_5ed82a91","line":26,"updated":"2026-06-23 05:10:22.000000000","message":"The test method test_skip_action_workflow is 135 lines long and covers five distinct workflow phases (create, wait, skip, verify, start-and-verify). This makes it difficult to diagnose which phase failed from the test name alone.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Breaking into smaller test methods or extracting named helper methods for each phase would improve failure diagnostics and make individual workflow steps reusable across future tests.\n\n**Recommendation**:\nExtract phase helpers (e.g., _skip_first_action, _verify_action_skipped, _start_and_verify_plan) to improve readability without changing test granularity, or split into parameterized sub-tests if the CI setup supports test ordering.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        self.take_screenshot(\"action_detail_loaded\")"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.expect(self.page.get_by_text(\"State\")).to_be_visible()"},{"line_number":107,"context_line":"        self.expect("},{"line_number":108,"context_line":"            self.page.get_by_text(\"SKIPPED\", exact\u003dTrue)"},{"line_number":109,"context_line":"        ).to_be_visible()"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"95fcb90f_b2c47d92","line":107,"updated":"2026-06-23 05:10:22.000000000","message":"The test asserts SKIPPED state using get_by_text(\u0027SKIPPED\u0027, exact\u003dTrue) on the action detail page. The ActionsTable display_choices maps raw \u0027SKIPPED\u0027 to user-facing \u0027Skipped\u0027. Coupling to the exact uppercase representation is fragile if the template or display_choices changes.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the detail view template is later updated to apply the display_choices filter (rendering \u0027Skipped\u0027 instead of \u0027SKIPPED\u0027), this assertion would fail with a non-obvious error despite the feature working correctly.\n\n**Suggestion**:\nConsider matching case-insensitively using re.compile(r\u0027skipped\u0027, re.IGNORECASE) or verifying the specific status field element rather than relying on page-wide exact text matching.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"25dcd5a52640ee40be5b9f141265e69607eb3612","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        ).to_be_visible()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.expect(self.page.get_by_text(\"Skip Reason\")).to_be_visible()"},{"line_number":112,"context_line":"        self.expect("},{"line_number":113,"context_line":"            self.page.get_by_text("},{"line_number":114,"context_line":"                re.compile(r\"(skipped.*CI testing|Action skipped by user\\.?)\")"},{"line_number":115,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":32,"id":"a9e06f9c_ed5d0d56","line":112,"updated":"2026-06-23 05:10:22.000000000","message":"The skip reason assertion uses a regex alternation r\u0027(skipped.*CI testing|Action skipped by user\\.?)\u0027 that couples the test to both the exact test-supplied reason text and an assumed backend message format. If the backend format or the test reason text changes, the assertion silently degrades.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: The two-branch regex masks failures: if the backend message changes, the test still passes on the first branch (the test\u0027s own reason text), hiding a real UI regression in how backend skip reasons are displayed.\n\n**Suggestion**:\nSplit into two independent assertions: first verify the user-supplied skip reason is displayed, then separately verify any backend-generated message if that is part of the expected behavior.","commit_id":"547527f14dcd1b15e6d25b8079aadff61b134cc9"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.expect(action_plan_row).to_be_visible()"},{"line_number":72,"context_line":"        action_plan_link \u003d action_plan_row.get_by_role(\"link\").first"},{"line_number":73,"context_line":"        self.expect(action_plan_link).to_be_visible()"},{"line_number":74,"context_line":"        action_plan_href \u003d action_plan_link.get_attribute(\"href\")"},{"line_number":75,"context_line":"        action_plan_uuid \u003d self._extract_uuid_from_href(action_plan_href)"},{"line_number":76,"context_line":"        action_plan_link.click()"},{"line_number":77,"context_line":"        self.take_screenshot(\"action_plan_detail_loaded\")"}],"source_content_type":"text/x-python","patch_set":33,"id":"20b5e329_3008976f","line":74,"updated":"2026-06-30 08:33:03.000000000","message":"The test extracts action_plan_uuid from a link href and later navigates/filters by that UUID. If _extract_uuid_from_href ever returns a partial or wrong segment, downstream steps silently use the bad value with no API-side sanity check.\n\n**Severity**: SUGGESTION | **Confidence**: 0.6\n\n**Benefit**: A cheap API-side sanity check on the extracted UUIDs would fail fast with a clear message instead of producing confusing Playwright timeouts deep in the workflow if the href format changes.\n\n**Recommendation**:\nOptionally validate extracted UUIDs against self.watcher_client (e.g. action_plan.get(action_plan_uuid)) right after extraction, or at minimum assert the href ends with the expected detail path pattern before parsing.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        )"},{"line_number":104,"context_line":"        self.take_screenshot(\"action_detail_loaded\")"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.expect(self.page.get_by_text(\"State\")).to_be_visible()"},{"line_number":107,"context_line":"        self.expect("},{"line_number":108,"context_line":"            self.page.get_by_text(\"SKIPPED\", exact\u003dTrue)"},{"line_number":109,"context_line":"        ).to_be_visible()"}],"source_content_type":"text/x-python","patch_set":33,"id":"c5e5850d_3b72eac9","line":106,"updated":"2026-06-30 08:33:03.000000000","message":"The action detail page is verified with get_by_text(\u0027State\u0027).to_be_visible() and get_by_text(\u0027SKIPPED\u0027, exact\u003dTrue). Both match any element anywhere on the page; this is the same broad-match pattern the base class works around with table exclusion.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the detail page renders the action state in more than one place (e.g. breadcrumb plus table), to_be_visible resolves on the first match and the test could pass against the wrong element, masking a regression, or fail intermittently when multiple nodes match.\n\n**Suggestion**:\nAnchor the assertion to the specific state value cell, e.g. scope get_by_text to the detail table/definition list, or assert via page.get_by_role(\u0027row\u0027, name\u003d\u0027State\u0027).locator(\u0027td\u0027).get_by_text(\u0027SKIPPED\u0027).","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            self.page.get_by_text(\"SKIPPED\", exact\u003dTrue)"},{"line_number":109,"context_line":"        ).to_be_visible()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.expect(self.page.get_by_text(\"Skip Reason\")).to_be_visible()"},{"line_number":112,"context_line":"        self.expect("},{"line_number":113,"context_line":"            self.page.get_by_text("},{"line_number":114,"context_line":"                re.compile(r\"(skipped.*CI testing|Action skipped by user\\.?)\")"}],"source_content_type":"text/x-python","patch_set":33,"id":"92776550_78e97414","line":111,"updated":"2026-06-30 08:33:03.000000000","message":"The skip-reason assertion uses re.compile(r\u0027(skipped.*CI testing|Action skipped by user\\.?)\u0027) with get_by_text, a substring/regex match across the whole page that relies on the dashboard echoing the submitted reason verbatim and can match incidental \u0027skipped\u0027 text.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the dashboard truncates, lowercases, or HTML-escapes the stored reason (common for user-supplied text), \u0027skipped.*CI testing\u0027 silently stops matching and the test fails for a presentation reason. The two-alternative regex also signals the expected message is not pinned down.\n\n**Suggestion**:\nAssert against the specific Skip Reason value cell with exact/normalized text. If the backend message varies, enumerate the known canonical strings and document why, rather than a permissive \u0027skipped.*CI testing\u0027 wildcard.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"a74fe355ee6041c8c19856d99bca90f8feb2d5ff","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            final_plan_state,"},{"line_number":158,"context_line":"            \"Action plan did not succeed; state\u003d%s\" % final_plan_state,"},{"line_number":159,"context_line":"        )"},{"line_number":160,"context_line":"        self.take_screenshot(\"action_plan_succeeded\")"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        # Backend may set the message slightly after plan reaches SUCCEEDED"},{"line_number":163,"context_line":"        self.expect("}],"source_content_type":"text/x-python","patch_set":33,"id":"a23f31fe_93c9a1c2","line":160,"updated":"2026-06-30 08:33:03.000000000","message":"The final success message is expected with get_by_text(re.compile(...), timeout\u003d30000) right after asserting SUCCEEDED. The inline comment says the backend may set the message after the state flips, but there is no reload/poll loop around the expectation.\n\n**Severity**: WARNING | **Confidence**: 0.6\n\n**Impact**: to_be_visible polls the DOM for the timeout, so this usually works, but it does not trigger a reload. If the status message requires a page refresh to render, the test becomes flaky. The comment implies a race the code does not actually handle.\n\n**Suggestion**:\nIf the message can require a refresh, wrap the assertion in the same reload-and-poll pattern used for terminal states. Otherwise drop the comment to avoid implying a handled race that is not actually handled.","commit_id":"0df54a2deef24f4aef24553f2cdfe313d6a53638"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        action_href \u003d action_link.get_attribute(\"href\")"},{"line_number":86,"context_line":"        action_uuid \u003d self._extract_uuid_from_href(action_href)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        skip_btn \u003d first_action_row.get_by_role(\"link\", name\u003d\"Skip Action\")"},{"line_number":89,"context_line":"        self.expect(skip_btn).to_be_visible()"},{"line_number":90,"context_line":"        skip_btn.click()"},{"line_number":91,"context_line":"        self.take_screenshot(\"skip_action_modal_open\")"}],"source_content_type":"text/x-python","patch_set":34,"id":"d0ce0e98_98bb8197","line":88,"updated":"2026-07-02 09:13:03.000000000","message":"The skip-action test relies on visible button/link text (\u0027Skip Action\u0027, \u0027Start Action Plan\u0027) for navigation. These selectors break silently if Horizon changes label casing or wording.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Anchoring selectors to stable attributes (data-testid, id, or aria-label) where Horizon provides them reduces test churn on cosmetic UI changes.\n\n**Recommendation**:\nWhere the dashboard supports data-testid or stable id attributes, prefer those over visible text. Where only text is available, centralize the label strings as named constants so a wording change is a single-point update.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"cc54dc79e0c510c32d0a7a944cfcdb100b07b1ac","unresolved":false,"context_lines":[{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self.expect(self.page.get_by_text(\"Skip Reason\")).to_be_visible()"},{"line_number":112,"context_line":"        self.expect("},{"line_number":113,"context_line":"            self.page.get_by_text("},{"line_number":114,"context_line":"                re.compile(r\"(skipped.*CI testing|Action skipped by user\\.?)\")"},{"line_number":115,"context_line":"            )"},{"line_number":116,"context_line":"        ).to_be_visible()"}],"source_content_type":"text/x-python","patch_set":34,"id":"a126ec71_1eed8587","line":113,"updated":"2026-07-02 09:13:03.000000000","message":"The test asserts the skip reason via a regex alternation (skipped.*CI testing|Action skipped by user.?) which matches two unrelated strings. If the dashboard changes the message format, the assertion may pass against the wrong text or fail confusingly.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: The assertion is coupled to an exact CI-specific reason on one branch and a generic backend string on the other, so it does not reliably verify that the user-entered skip_reason is actually displayed.\n\n**Suggestion**:\nAssert against the literal skip_reason value the test submitted (self.page.get_by_text(skip_reason, exact\u003dTrue)) to verify round-trip fidelity, and separately assert the State label shows SKIPPED. Drop the generic fallback branch unless it represents a known backend normalization.","commit_id":"ab5616a66944dccb18529b9b0d8e39a8f3816dfc"},{"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":"5da37f84c1995bf0816dd5859cf7025e4f7863f3","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        self.take_screenshot(\"action_plan_detail_loaded\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        actions_table \u003d self.page.get_by_role(\"table\", name\u003d\"Related Actions\")"},{"line_number":80,"context_line":"        first_action_row \u003d actions_table.locator(\"tbody tr\").first"},{"line_number":81,"context_line":"        self.expect(first_action_row).to_be_visible()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        action_link \u003d first_action_row.get_by_role(\"link\").first"}],"source_content_type":"text/x-python","patch_set":35,"id":"50c4ac04_def3ca9f","line":80,"updated":"2026-07-02 15:54:05.000000000","message":"The skip-action test uses order-dependent positional selectors (actions_table.locator(\u0027tbody tr\u0027).first and get_by_role(\u0027link\u0027).first) to locate the action. If the target action is in a non-first row, or the first link is not the Skip Action control, the test skips an unintended action.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Targeting the Skip Action control by its accessible name/role rather than by positional \u0027.first\u0027 makes the test resilient to column and row ordering changes and clarifies intent.\n\n**Recommendation**:\nPrefer get_by_role(\u0027link\u0027, name\u003d\u0027Skip Action\u0027) scoped to the actions table, then resolve that link\u0027s owning row, rather than assuming the first row and first link is the skip control. The test already locates skip_btn by name on line 88; consider driving the row lookup from that locator instead of from first_action_row.","commit_id":"208b5f46e1a8ab8edfa799eaf5e8c7fc353473ec"}]}