Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions testsupport/devsandbox-dashboard/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,18 @@ func handleCookiesConsent(t *testing.T, page playwright.Page) {
contentFrame := iframe.ContentFrame()
require.NotNil(t, contentFrame)

// find the agree button
agreeButton := contentFrame.GetByRole("button", playwright.FrameLocatorGetByRoleOptions{
// TrustArc can show different modals (e.g. full consent vs. simplified); accept whichever button is present.
agreeProceed := contentFrame.GetByRole("button", playwright.FrameLocatorGetByRoleOptions{
Name: "Agree and proceed with",
})

err = agreeButton.Click()
// to some us states, like texas, it appears the "Cookie Preferences and Opt-Out Rights" modal
acceptDefault := contentFrame.GetByRole("button", playwright.FrameLocatorGetByRoleOptions{
Name: "Accept default",
})
consentButton := agreeProceed.Or(acceptDefault)

err = consentButton.Click()
require.NoError(t, err)
Comment on lines +134 to 146
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Playwright (especially playwright-go), if Locator.Or(otherLocator) matches two elements, can Locator.Click() fail due to strict mode? What is the recommended pattern to try locator A first and fall back to locator B?

💡 Result:

Yes. In Playwright, locators are strict: if you call Locator.click (including in playwright-go) and that locator resolves to more than one element, Playwright can throw a “strict mode violation”. Using locator.or (or Locator.Or in playwright-go) creates a locator that matches one or both alternatives; if both alternatives are present at the same time, the combined locator may match multiple elements and then click will fail due to strictness. Recommended pattern (try A, fall back to B): avoid calling click on an OR-locator that may match multiple elements. Instead, explicitly choose which one to click by checking visibility (or count) on each alternative and then clicking the single matching locator. Example (Node/Playwright JS pattern, same idea applies to playwright-go): const a = page.getByRole('button', { name: 'A' }); const b = page.getByRole('button', { name: 'B' }); if (await a.isVisible) await a.click; else await b.click; If you specifically use an OR locator, Playwright’s own docs show using .first to prevent the strict mode issue when the OR matches multiple elements, but they caution that .first is not recommended because page changes can make it click an unintended element. Example with OR: await expect(a.or(b).first).toBeVisible; // then decide/click based on which one is visible, as shown in the docs.

Citations:


🏁 Script executed:

#!/bin/bash
# Verify the code snippet exists at the specified lines
if [ -f "testsupport/devsandbox-dashboard/setup.go" ]; then
  sed -n '130,150p' testsupport/devsandbox-dashboard/setup.go | cat -n
else
  echo "File not found"
fi

Repository: codeready-toolchain/toolchain-e2e

Length of output: 1065


Make consent-button fallback deterministic to avoid flaky modal handling.

The agreeProceed.Or(acceptDefault).Click() pattern at line 145 can fail in strict mode if both buttons are present simultaneously, causing test flakiness. Playwright does not guarantee which locator in an OR combination is clicked, and when multiple elements match, Click() throws a strict mode violation. Use explicit try-then-fallback instead.

🔧 Proposed fix
-	consentButton := agreeProceed.Or(acceptDefault)
-
-	err = consentButton.Click()
+	err = agreeProceed.Click(playwright.LocatorClickOptions{
+		Timeout: playwright.Float(2000),
+	})
+	if err != nil {
+		err = acceptDefault.Click(playwright.LocatorClickOptions{
+			Timeout: playwright.Float(2000),
+		})
+	}
 	require.NoError(t, err)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@testsupport/devsandbox-dashboard/setup.go` around lines 134 - 146, The OR
locator is non-deterministic and can throw a strict-mode error if both buttons
exist; replace consentButton := agreeProceed.Or(acceptDefault).Click() with an
explicit try-then-fallback: first attempt agreeProceed.Click(), if that returns
an error (or a strict-mode/multiple-matches error), then attempt
acceptDefault.Click(), and assert no error for whichever click succeeds (use
require.NoError on each attempted click). Use the existing agreeProceed,
acceptDefault, contentFrame and require.NoError symbols to locate and implement
the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As far as I know, either appears one or another

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@rsoaresd, that's a good clarification — if the two TrustArc modal variants are mutually exclusive (only one appears at a time, never both), then the .Or() locator will always resolve to exactly one element, and the strict mode concern doesn't apply. The current code is safe as-is. Thanks for the context!


📝 Learnings were identified and not saved (knowledge base disabled). Enable


// wait for iframe to disappear (indicates cookie acceptance complete)
Expand Down
Loading