[rb] Add URLs constant to update error messages

[aguspe]

User description

Description

This PR adds a URLs constant on the error module to be able to obtain the URL based on a symbol for each WebDriver error subclass

Motivation and Context

Based on #11512 the goal is to provide more information in the error message by linking to the selenium documentation

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Added a url method to the WebDriverError class to generate URLs dynamically based on the class name.
• Modified error messages to include the dynamically generated URLs.
• Removed hardcoded URLs from specific error classes such as NoSuchElementError, StaleElementReferenceError, InvalidSelectorError, and NoSuchDriverError.
• Added tests to verify the correct URL generation for error messages.
• Updated type signatures to reflect the new methods in the WebDriverError class.

---

Changes walkthrough 📝

	Relevant files
Enhancement	error.rb Add dynamic URL generation for error messages.                      rb/lib/selenium/webdriver/common/error.rb Added url method to WebDriverError class to generate URLs based on class names. Modified error messages to include generated URLs. Removed hardcoded URLs from specific error classes. +22/-21  error.rbs Update type signatures for new methods in `WebDriverError`. rb/sig/lib/selenium/webdriver/common/error.rbs Updated type signatures to include new methods in WebDriverError. +5/-0
Tests	error_spec.rb Add tests for dynamic URL generation in error messages.    rb/spec/integration/selenium/webdriver/error_spec.rb Added tests for self-generated URLs in error messages. Included test for NoSuchElementError to verify URL generation. +11/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	3
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The method url in WebDriverError class manipulates class names to generate URLs. This assumes a specific naming convention and structure for error class names which might not always hold true or could lead to incorrect URLs if the class names are not perfectly aligned with the expected format.
	Refactoring Impact: The removal of hardcoded URLs and reliance on dynamically generated URLs could impact existing documentation or external references that expect static URLs. This needs careful review to ensure it doesn't break existing user flows or documentation links.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Test coverage	Add a test case to verify URL generation for StaleElementReferenceError Add a test case to verify the URL generation for another error type, such as StaleElementReferenceError, to ensure the URL generation logic is robust. rb/spec/integration/selenium/webdriver/error_spec.rb [35-42] context 'with self generated url' do it 'provides the right url for NoSuchElementError' do driver.navigate.to url_for('xhtmlTest.html') driver.find_element(id: 'nonexistent') rescue WebDriver::Error::NoSuchElementError => e expect(e.message).to include("#{base_url}#no-such-element-exception") end + + it 'provides the right url for StaleElementReferenceError' do + driver.navigate.to url_for('xhtmlTest.html') + element = driver.find_element(id: 'some-element') + driver.execute_script('arguments[0].remove();', element) + element.click + rescue WebDriver::Error::StaleElementReferenceError => e + expect(e.message).to include("#{base_url}#stale-element-reference-exception") + end end Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding a test case for another error type is crucial for ensuring the robustness of the URL generation logic. This suggestion correctly identifies a gap in test coverage.	8
Enhancement	Simplify the gsub method call by using a block to insert hyphens before uppercase letters The rest_with_hyphens variable can be simplified by using gsub with a block to insert hyphens before uppercase letters, making the code more concise and readable. rb/lib/selenium/webdriver/common/error.rb [48] -rest_with_hyphens = rest.gsub(/([A-Z])/, '-\1') +rest_with_hyphens = rest.gsub(/([A-Z])/) { |match| "-#{match}" } Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies a potential improvement in the gsub usage for clarity and conciseness, which enhances readability.	7
Performance	Memoize the url method to improve performance by avoiding repeated calculations Consider memoizing the url method to avoid recalculating the URL every time it is accessed, which can improve performance if the method is called multiple times. rb/lib/selenium/webdriver/common/error.rb [46-50] def url - first_word, rest = parsed_class_name.split(/(?=[A-Z])/, 2) - rest_with_hyphens = rest.gsub(/([A-Z])/, '-\1') - "#{ERROR_URL}##{first_word.downcase}#{rest_with_hyphens.downcase.sub('error', 'exception')}" + @url ||= begin + first_word, rest = parsed_class_name.split(/(?=[A-Z])/, 2) + rest_with_hyphens = rest.gsub(/([A-Z])/) { |match| "-#{match}" } + "#{ERROR_URL}##{first_word.downcase}#{rest_with_hyphens.downcase.sub('error', 'exception')}" + end end Apply this suggestion Suggestion importance[1-10]: 7 Why: Memoization is a valuable optimization technique, especially for methods that might be called frequently with the same result. This suggestion is relevant and could improve performance.	7
Best practice	Freeze the string constants to ensure they remain immutable Use freeze on the SUPPORT_MSG and ERROR_URL constants to prevent them from being modified, ensuring they remain immutable. rb/lib/selenium/webdriver/common/error.rb [37-38] -SUPPORT_MSG = 'For documentation on this error, please visit:' -ERROR_URL = 'https://www.selenium.dev/documentation/webdriver/troubleshooting/errors' +SUPPORT_MSG = 'For documentation on this error, please visit:'.freeze +ERROR_URL = 'https://www.selenium.dev/documentation/webdriver/troubleshooting/errors'.freeze Apply this suggestion Suggestion importance[1-10]: 6 Why: Freezing constants is a good practice in Ruby to prevent accidental modifications. The suggestion is correct and applies to the constants defined in the PR.	6

[p0deje]

I see that you dynamically generate URLs from a class name of the error. I have a feeling it might be fragile and prone to subtle bugs if somebody makes a typo in a URL while working on #1276. It also make it really hard to refactor the website and make sure URLs are used correctly in all bindings. The problem is that this can't be grepped anymore.

I'd suggest we instead have a hash of error => URL mapping that will explicitly allow us to mark which errors already have URLs and add them as we go.

URLS = {
  NoSuchElementError => '#no-such-element-exception',
  ...
}

Looking forward to what @titusfortner thinks on that?

[aguspe]

I see that you dynamically generate URLs from a class name of the error. I have a feeling it might be fragile and prone to subtle bugs if somebody makes a typo in a URL while working on #1276. It also make it really hard to refactor the website and make sure URLs are used correctly in all bindings. The problem is that this can't be grepped anymore.

I'd suggest we instead have a hash of error => URL mapping that will explicitly allow us to mark which errors already have URLs and add them as we go.

URLS = {

  NoSuchElementError => '#no-such-element-exception',

  ...

}

Looking forward to what @titusfortner thinks on that?

That sounds like a really nice idea, and it will definitely be better than my approach of just having urls that redirect to the error page

If @titusfortner agrees with this, I can update this PR tomorrow since there are only a couple of urls and then keep making small PRs for the rest of them

[aguspe]

@p0deje I changed my implementation to match your suggestion and I fixed the rubocop issues, also I simplified the tests so I hope now this PR is on the right track :)

[p0deje]

Thank you for the contribution!