Skip to content

[java] guard tests broken in chrome 149#17426

Merged
titusfortner merged 2 commits intotrunkfrom
content_editable
May 8, 2026
Merged

[java] guard tests broken in chrome 149#17426
titusfortner merged 2 commits intotrunkfrom
content_editable

Conversation

@titusfortner
Copy link
Copy Markdown
Member

💥 What does this PR do?

Fix CI failures
These tests work in 148 but not in 149

🔧 Implementation Notes

I'd like it if Java had a better way to guard this, but this is what we have for now.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

Copilot AI review requested due to automatic review settings May 8, 2026 10:40
@selenium-ci selenium-ci added the C-java Java Bindings label May 8, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Guard TinyMCE tests against Chrome 149 failures

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Guard TinyMCE tests against Chrome 149 compatibility issues
• Add version checks to skip tests on Chrome 149+
• Import TestUtilities for Chrome version detection

Grey Divider

File Changes

1. java/test/org/openqa/selenium/ContentEditableTest.java 🐞 Bug fix +4/-0

Add Chrome 149 version guards to TinyMCE tests

• Added import for TestUtilities to access Chrome version detection
• Added assumeThat(TestUtilities.getChromeVersion(driver)).isLessThan(149) guard to
 testShouldBeAbleToTypeIntoTinyMCE() test
• Added same version guard to testShouldAppendToTinyMCE() test
• Tests will be skipped on Chrome 149+ due to known compatibility issues

java/test/org/openqa/selenium/ContentEditableTest.java


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 8, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. assumeThat not imported ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The newly added assumeThat(...) calls will not compile because there is no static import (or
qualified reference) for assumeThat. This breaks the test build and prevents the intended
Chrome-version gating from executing.
Code

java/test/org/openqa/selenium/ContentEditableTest.java[R103-106]

+    assumeThat(TestUtilities.getChromeVersion(driver)).isLessThan(149);
    driver.get(appServer.whereIs("tinymce.html"));
    driver.switchTo().frame("mce_0_ifr");
Evidence
PR Compliance ID 12 requires environment/feature gating for tests to avoid CI failures; here the
gating was added but is broken because assumeThat is referenced without being imported/qualified.
The file imports assertThat and assumeFalse but not
org.assertj.core.api.Assumptions.assumeThat, while adding new assumeThat(...) calls.

java/test/org/openqa/selenium/ContentEditableTest.java[20-33]
java/test/org/openqa/selenium/ContentEditableTest.java[101-106]
java/test/org/openqa/selenium/ContentEditableTest.java[115-123]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`assumeThat(...)` is used in `ContentEditableTest` but is not imported or qualified, causing compilation failures and preventing the intended Chrome-version test gating.

## Issue Context
Other tests in this repo use `import static org.assertj.core.api.Assumptions.assumeThat;` (or qualify the call via `Assumptions.assumeThat(...)`). This file currently only imports `assertThat` and JUnit's `assumeFalse`.

## Fix Focus Areas
- java/test/org/openqa/selenium/ContentEditableTest.java[20-33]
- java/test/org/openqa/selenium/ContentEditableTest.java[101-106]
- java/test/org/openqa/selenium/ContentEditableTest.java[115-123]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. getChromeVersion gating unreliable 📘 Rule violation ☼ Reliability
Description
The new skip condition relies on TestUtilities.getChromeVersion(driver), which returns 0 when
Chrome version cannot be determined; this makes the assumption pass (0 < 149) and can leave the
test running (and failing) on Chrome 149+ in some environments. This undermines the intended
environment gating for CI stability.
Code

java/test/org/openqa/selenium/ContentEditableTest.java[R103-106]

+    assumeThat(TestUtilities.getChromeVersion(driver)).isLessThan(149);
    driver.get(appServer.whereIs("tinymce.html"));
    driver.switchTo().frame("mce_0_ifr");
Evidence
PR Compliance ID 12 expects tests to be gated based on environment capability; the new gating uses
getChromeVersion(...) without ensuring Chrome detection or handling the method's 0 fallback.
getChromeVersion explicitly returns 0 when capabilities/version parsing is unavailable, which
will satisfy .isLessThan(149) and not skip the test even if running on a Chrome 149+ environment
where version detection is missing.

java/test/org/openqa/selenium/ContentEditableTest.java[101-106]
java/test/org/openqa/selenium/ContentEditableTest.java[119-122]
java/test/org/openqa/selenium/testing/TestUtilities.java[62-87]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new Chrome-version skip uses `TestUtilities.getChromeVersion(driver)` directly, but that helper returns `0` when the version cannot be determined. That can cause the assumption to pass and the test to run (and still fail) on Chrome 149+ in environments where the capability is missing.

## Issue Context
`TestUtilities.getChromeVersion(driver)` returns `0` if `driver` is not `HasCapabilities`, if the chromedriver version capability is missing, or if parsing fails. Treating `0` as a real version makes `.isLessThan(149)` always pass.

## Fix Focus Areas
- java/test/org/openqa/selenium/ContentEditableTest.java[101-106]
- java/test/org/openqa/selenium/ContentEditableTest.java[119-122]
- java/test/org/openqa/selenium/testing/TestUtilities.java[62-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread java/test/org/openqa/selenium/ContentEditableTest.java
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a temporary guard to skip the TinyMCE contenteditable tests when running against Chrome/ChromeDriver 149+, addressing CI failures seen in Chrome 149 but not 148.

Changes:

  • Introduce a Chrome/ChromeDriver major-version assumption to skip two TinyMCE-related tests for 149+.
  • Add TestUtilities usage to read the ChromeDriver major version from capabilities.

Comment thread java/test/org/openqa/selenium/ContentEditableTest.java
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread java/test/org/openqa/selenium/ContentEditableTest.java
Comment thread java/test/org/openqa/selenium/ContentEditableTest.java
@titusfortner titusfortner merged commit bc08914 into trunk May 8, 2026
32 checks passed
@titusfortner titusfortner deleted the content_editable branch May 8, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants