Skip to content

chore(e2e): rhdhbugs-2483 Disabling rbac.spec.ts which is failing consistently #3994

Merged
nickboldt merged 2 commits intoredhat-developer:mainfrom
gustavolira:RHDHBUGS-2483
Jan 13, 2026
Merged

chore(e2e): rhdhbugs-2483 Disabling rbac.spec.ts which is failing consistently #3994
nickboldt merged 2 commits intoredhat-developer:mainfrom
gustavolira:RHDHBUGS-2483

Conversation

@gustavolira
Copy link
Copy Markdown
Member

Description

Disabling rbac.spec.ts which is failing consistently

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Test Coverage

Two E2E tests are now skipped, which reduces RBAC coverage and can hide regressions. Validate that skipping is the intended temporary action, that there is a clear plan to re-enable them, and that CI/reporting will still surface the underlying failure elsewhere (or that the failure is tracked adequately).

//FIXME RHDHBUGS-2483
test.skip("Create and edit a role from the roles list page", async ({
  page,
}) => {
  const uiHelper = new UIhelper(page);
Consistency

The PR mixes different FIXME formats (plain ticket id vs full URL). Consider standardizing the comment format and using Playwright’s test.fixme() (optionally with a reason) for better reporting, so skipped tests are clearly attributed in test output.

//FIXME https://issues.redhat.com/browse/RHDHBUGS-2483
test("Edit users and groups and update policies of a role from the overview page", async ({
  page,
}) => {
Skip Scope

The skips are applied at individual test level; confirm this is the minimal necessary scope (vs skipping only in specific environments/configurations) and that other RBAC tests remain reliable to avoid masking broader instability.

//FIXME https://issues.redhat.com/browse/RHDHBUGS-2483
test.skip("Test that user with `IsOwner` condition can access the RBAC page, create a role, edit a role, and delete the role", async ({
  page,
}) => {
  const common = new Common(page);
📄 References
  1. No matching references available

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Jan 13, 2026

PR Type

Tests


Description

  • Skip two failing RBAC test cases with FIXME comments

  • Mark "Create and edit a role from the roles list page" test as skipped

  • Mark "Test that user with IsOwner condition..." test as skipped

  • Add issue reference RHDHBUGS-2483 to failing tests


File Walkthrough

Relevant files
Tests
rbac.spec.ts
Skip two failing RBAC test cases                                                 

e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts

  • Skip two failing test cases using test.skip() instead of test()
  • Add FIXME comments referencing [RHDHBUGS-2483](https://issues.redhat.com//browse/RHDHBUGS-2483) issue for both skipped
    tests
  • First skipped test: "Create and edit a role from the roles list page"
  • Second skipped test: "Test that user with IsOwner condition can access
    the RBAC page..."
+5/-2     

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Jan 13, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Skip a test to prevent failures
Suggestion Impact:The test at the referenced location was updated to use `test.skip`, disabling it to prevent failures.

code diff:

     //FIXME https://issues.redhat.com/browse/RHDHBUGS-2483
-    test("Edit users and groups and update policies of a role from the overview page", async ({
+    test.skip("Edit users and groups and update policies of a role from the overview page", async ({
       page,
     }) => {

Add test.skip to the test at line 407 to align with the PR's goal of disabling
failing tests.

e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts [406-409]

 //FIXME https://issues.redhat.com/browse/RHDHBUGS-2483
-test("Edit users and groups and update policies of a role from the overview page", async ({
+test.skip("Edit users and groups and update policies of a role from the overview page", async ({
   page,
 }) => {

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that a test was marked with a FIXME comment but not skipped, which is inconsistent with other changes in the PR and likely defeats its purpose for this test.

Medium
Mark test as expected failure

Replace test.skip with test.fixme to better indicate that the test is skipped
due to a known bug.

e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts [323-324]

-//FIXME RHDHBUGS-2483
-test.skip("Create and edit a role from the roles list page", async ({
+test.fixme("Create and edit a role from the roles list page", async ({
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion proposes using test.fixme instead of test.skip, which is a better practice for temporarily disabled tests linked to a bug, improving test reporting and maintainability.

Low
Convert skip to fixme

Replace test.skip with test.fixme to better indicate that the test is skipped
due to a known bug.

e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts [752-753]

-//FIXME https://issues.redhat.com/browse/RHDHBUGS-2483
-test.skip("Test that user with `IsOwner` condition can access the RBAC page, create a role, edit a role, and delete the role", async ({
+test.fixme("Test that user with `IsOwner` condition can access the RBAC page, create a role, edit a role, and delete the role", async ({
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion proposes using test.fixme instead of test.skip, which is a better practice for temporarily disabled tests linked to a bug, improving test reporting and maintainability.

Low
  • Update

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems silly that we need to get approvals to disable tests that don't work but here we are

/lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jan 13, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nickboldt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nickboldt nickboldt merged commit 2c5ec57 into redhat-developer:main Jan 13, 2026
12 of 14 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

The image is available at:

/test e2e-ocp-helm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants