[rb] Add URLs constant to update error messages#14174
[rb] Add URLs constant to update error messages#14174p0deje merged 12 commits intoSeleniumHQ:trunkfrom
Conversation
PR Reviewer Guide 🔍
|
PR Code Suggestions ✨
|
…uspe/selenium into rb_add_link_to_docs_in_error_messages
…uspe/selenium into rb_add_link_to_docs_in_error_messages
p0deje
left a comment
There was a problem hiding this comment.
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 |
|
@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 :) |
|
Thank you for the contribution! |
Co-authored-by: aguspe <agustin.pe94@gmail.com>
Co-authored-by: aguspe <agustin.pe94@gmail.com>
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
Checklist
PR Type
Enhancement, Tests
Description
urlmethod to theWebDriverErrorclass to generate URLs dynamically based on the class name.NoSuchElementError,StaleElementReferenceError,InvalidSelectorError, andNoSuchDriverError.WebDriverErrorclass.Changes walkthrough 📝
error.rb
Add dynamic URL generation for error messages.rb/lib/selenium/webdriver/common/error.rb
urlmethod toWebDriverErrorclass to generate URLs based onclass names.
error.rbs
Update type signatures for new methods in `WebDriverError`.rb/sig/lib/selenium/webdriver/common/error.rbs
WebDriverError.error_spec.rb
Add tests for dynamic URL generation in error messages.rb/spec/integration/selenium/webdriver/error_spec.rb
NoSuchElementErrorto verify URL generation.