Skip to content

Add ExpectedConditions.urlFulfills #16285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Add ExpectedConditions.urlFulfills #16285

wants to merge 2 commits into from

Conversation

@UncleOwen
Copy link

@UncleOwen UncleOwen commented Sep 2, 2025 •
edited by qodo-merge-pro bot
Loading

User description

💥 What does this PR do?

This PR adds a new method ExpectedConditions.urlFulfills(). This allows code like

wait.until(ExpectedConditions.urlFulfills(url -> url.getHost().equals(PORTAL_HOST)));

which is more specific than

wait.until(ExpectedConditions.urlContains(PORTAL_HOST))

and more readable than whatever you'd do with ExpectedContitions.urlMatches()

🔧 Implementation Notes

Predicates usually don't have a good .toString(), that's why I left it out from .toString()

💡 Additional Considerations

I'm bad at coming up with good names. Feed free to suggest a better name.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

  • Add urlFulfills() method to ExpectedConditions class

  • Accept Predicate<URI> for flexible URL validation

  • Include comprehensive test coverage for new functionality

  • Handle malformed URLs gracefully with exception handling

Diagram Walkthrough

flowchart LR
  A["WebDriver getCurrentUrl()"] --> B["Parse URL to URI"]
  B --> C["Apply Predicate<URI>"]
  C --> D["Return Boolean result"]
  E["URISyntaxException"] --> F["Return false"]
  B -.-> E
Loading

File Walkthrough
Relevant files
Enhancement
ExpectedConditions.java
Add urlFulfills method with predicate support                       

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java

  • Add urlFulfills() method accepting Predicate
  • Import URI, URISyntaxException, and Predicate classes
  • Implement URL parsing with exception handling for malformed URLs
  • Return descriptive toString() method for debugging
 +32/-0   
Tests
ExpectedConditionsTest.java
Add comprehensive tests for urlFulfills method                     

java/test/org/openqa/selenium/support/ui/ExpectedConditionsTest.java

  • Add static import for urlFulfills method
  • Import URI class for test implementations
  • Add four comprehensive test methods covering positive and negative
    scenarios
  • Test URL components (scheme, host, port, path) and malformed URL
    handling
 +39/-0   

@CLAassistant
Copy link

CLAassistant commented Sep 2, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@selenium-ci selenium-ci added C-java Java Bindings B-support Issue or PR related to support classes labels Sep 2, 2025
@selenium-ci
Copy link
Member

selenium-ci commented Sep 2, 2025

Thank you, @UncleOwen for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Sep 2, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

NPE Risk

urlFulfills does not defensively handle a null Predicate, which would throw a NullPointerException at runtime; consider adding a null-check or documenting the requirement.

 
public static ExpectedCondition<Boolean> urlFulfills(Predicate<URI> predicate) {
  return new ExpectedCondition<Boolean>() {
    private String currentUrl = "";

    @Override
    public Boolean apply(WebDriver driver) {
      currentUrl = driver.getCurrentUrl();
      try {
        URI uri = new URI(currentUrl);
        return predicate.test(uri);
      } catch (URISyntaxException e) {
        return false;
      }
    }

    @Override
    public String toString() {
      return String.format(
          "url to apply a predicate. Current url: \"%s\"", currentUrl);
    }
  };
}
toString Usefulness

The toString omits predicate details entirely; consider including a brief marker when the current URL is empty or first invocation to aid debugging, or document why predicate info is intentionally excluded.

 
  @Override
  public String toString() {
    return String.format(
        "url to apply a predicate. Current url: \"%s\"", currentUrl);
  }
};

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Sep 2, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Guard null URL and predicate failures

urlFulfills will throw a NullPointerException if driver.getCurrentUrl() returns
null, causing the wait to fail unexpectedly (unlike urlContains/urlToBe which
guard nulls). Add a null check before constructing the URI and consider catching
RuntimeException from predicate.test to return false so polling continues,
aligning behavior with other ExpectedConditions. This makes the new condition
robust across drivers that transiently return null or user predicates that may
NPE on missing URI parts.

Examples:

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java [174-182] 
      public Boolean apply(WebDriver driver) {
        currentUrl = driver.getCurrentUrl();
        try {
          URI uri = new URI(currentUrl);
          return predicate.test(uri);
        } catch (URISyntaxException e) {
          return false;
        }
      }

Solution Walkthrough:

Before:

public static ExpectedCondition<Boolean> urlFulfills(Predicate<URI> predicate) {
  return new ExpectedCondition<Boolean>() {
    @Override
    public Boolean apply(WebDriver driver) {
      String currentUrl = driver.getCurrentUrl();
      try {
        // Throws NullPointerException if currentUrl is null
        URI uri = new URI(currentUrl);
        // Can throw exceptions if predicate is not robust
        return predicate.test(uri);
      } catch (URISyntaxException e) {
        return false;
      }
    }
    // ...
  };
}

After:

public static ExpectedCondition<Boolean> urlFulfills(Predicate<URI> predicate) {
  return new ExpectedCondition<Boolean>() {
    @Override
    public Boolean apply(WebDriver driver) {
      String currentUrl = driver.getCurrentUrl();
      if (currentUrl == null) {
        return false;
      }
      try {
        URI uri = new URI(currentUrl);
        return predicate.test(uri);
      } catch (URISyntaxException | RuntimeException e) {
        // Also handles exceptions from the predicate
        return false;
      }
    }
    // ...
  };
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical NullPointerException when the URL is null and proposes a robust fix, aligning the new method's behavior with existing URL conditions.

High
Possible issue
Handle null current URL safely

Guard against a null current URL before constructing a URI. If
driver.getCurrentUrl() returns null, new URI(null) will throw a
NullPointerException and break the wait. Return false when the URL is null to
keep polling safely.

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java [173-182]

 @Override
 public Boolean apply(WebDriver driver) {
   currentUrl = driver.getCurrentUrl();
+  if (currentUrl == null) {
+    return false;
+  }
   try {
     URI uri = new URI(currentUrl);
     return predicate.test(uri);
   } catch (URISyntaxException e) {
     return false;
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential NullPointerException if driver.getCurrentUrl() returns null, which would crash the wait. Adding a null check improves the robustness of the new method, aligning it with other URL-based conditions in the class.

Medium
Learned
best practice
✅ Validate predicate non-null
Suggestion Impact:The commit added Objects.requireNonNull(predicate, ...) at the start of urlFulfills, implementing the suggested null validation.

code diff:

   public static ExpectedCondition<Boolean> urlFulfills(Predicate<URI> predicate) {
+    Objects.requireNonNull(predicate, "predicate cannot be null");
+
     return new ExpectedCondition<Boolean>() {

Validate that predicate is non-null at method entry to avoid NPEs from
predicate.test.

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java [169-170]

 public static ExpectedCondition<Boolean> urlFulfills(Predicate<URI> predicate) {
+  java.util.Objects.requireNonNull(predicate, "predicate");
   return new ExpectedCondition<Boolean>() {

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Initialize and validate inputs early to avoid null or uninitialized states.

Low
  • Update

@cgoldberg
Copy link
Member

cgoldberg commented Sep 2, 2025

Thanks, but I am closing this for the reasons described in: #16285 (comment)

Also, pretty much every conceivable expected condition for a URL can already be handled with urlMatches.

@cgoldberg cgoldberg closed this Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

B-support Issue or PR related to support classes C-java Java Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@UncleOwen @CLAassistant @selenium-ci @cgoldberg