Skip to content
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.

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

[rb] Add missing RBS methods #14621

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented Oct 18, 2024

Description

This PR adds an array of RBS missing methods and reduces the RBS errors from 73 to 28

Screenshot 2024-10-24 at 21 41 42 Screenshot 2024-10-24 at 21 41 22

Motivation and Context

To add full RBS support for Selenium and allow us to eventually add an RBS check on the pipeline.
The reference feature is #10943

The goal is also to be able to have no steep errors to start adding the right type on the classes that have untyped to have the right type enforces

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


Description

  • Updated method signatures and return types across multiple modules to enhance functionality and type safety.
  • Added new methods and instance variables to improve class initialization and method operations.
  • Enhanced the handling of callbacks and message handlers in WebSocket and BiDi modules.
  • Improved the structure and capabilities of the DriverFinder, SearchContext, and Service modules.

Changes walkthrough 📝

Relevant files
Enhancement
11 files
bidi.rbs
Update `remove_callback` method signature in BiDi module 

rb/sig/lib/selenium/webdriver/bidi.rbs

  • Modified remove_callback method signature to accept parameters.
+1/-1     
log_handler.rbs
Update return types in LogHandler module                                 

rb/sig/lib/selenium/webdriver/bidi/log_handler.rbs

  • Changed return type of remove_message_handler to bool.
  • Changed return type of subscribe_log_entry to bool.
  • Changed return type of unsubscribe_log_entry to bool.
  • +3/-3     
    driver_finder.rbs
    Add initialization and path methods to DriverFinder           

    rb/sig/lib/selenium/webdriver/common/driver_finder.rbs

  • Added instance variables @options, @paths, and @service.
  • Added initialize method with parameters.
  • Added new methods browser_path, browser_path?, and driver_path.
  • +17/-0   
    search_context.rbs
    Add accessors and methods to SearchContext module               

    rb/sig/lib/selenium/webdriver/common/search_context.rbs

  • Added attr_accessor for self.extra_finders.
  • Added self.finders method.
  • +4/-0     
    service.rbs
    Update Service module with new method and type change       

    rb/sig/lib/selenium/webdriver/common/service.rbs

  • Changed return type of env_path to String?.
  • Added find_driver_path method.
  • +3/-1     
    websocket_connection.rbs
    Add and update callback methods in WebSocketConnection     

    rb/sig/lib/selenium/webdriver/common/websocket_connection.rbs

  • Added add_callback method with parameters.
  • Modified remove_callback method signature to accept parameters.
  • +4/-0     
    dialog.rbs
    Add initialization method to FedCM Dialog class                   

    rb/sig/lib/selenium/webdriver/fedcm/dialog.rbs

    • Added initialize method with Remote::Bridge parameter.
    +2/-0     
    options.rbs
    Add options instance variable to Firefox Options                 

    rb/sig/lib/selenium/webdriver/firefox/options.rbs

    • Added instance variable @options.
    +2/-0     
    common.rbs
    Enhance HTTP Common with headers and user agent methods   

    rb/sig/lib/selenium/webdriver/remote/http/common.rbs

  • Added @common_headers instance variable.
  • Added attr_accessor for self.extra_headers.
  • Added attr_writer and method for self.user_agent.
  • Added common_headers method.
  • +10/-0   
    guard.rbs
    Add tracker attribute reader to Guard class                           

    rb/sig/lib/selenium/webdriver/support/guards/guard.rbs

    • Added attr_reader for tracker.
    +1/-0     
    script.rbs
    Add initialization and update handler method in Script class

    rb/sig/selenium/web_driver/script.rbs

  • Added initialize method with BiDi parameter.
  • Modified remove_console_message_handler to accept an id parameter.
  • +3/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @aguspe aguspe marked this pull request as ready for review October 24, 2024 08:23
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Method Signature Change
    The remove_callback method signature has been changed to accept parameters. Ensure this change is consistent with the implementation and doesn't break existing code.

    Return Type Changes
    The return types for remove_message_handler, subscribe_log_entry, and unsubscribe_log_entry have been changed from false to bool. Verify if this change is intentional and consistent with the actual implementation.

    New Methods
    Several new methods and instance variables have been added to the DriverFinder class. Ensure these additions are properly implemented and documented in the actual code.

    Method Signature Changes
    The add_callback and remove_callback methods have been modified to accept parameters. Verify that these changes are reflected in the implementation and don't break existing functionality.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 24, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    ✅ Specify more precise types for method parameters to improve type safety and clarity
    Suggestion Impact:The commit implemented more precise types for the parameters in the remove_callback method, changing 'event' to 'String' and 'id' to 'Integer', which aligns with the suggestion's goal of improving type safety.

    code diff:

    -      def remove_callback: (untyped event, untyped id) -> untyped
    +      def remove_callback: (String event, Integer id) -> Error::WebDriverError?

    Consider specifying more precise types for the event and id parameters in the
    remove_callback method signature. This will improve type safety and make the
    method's usage clearer.

    rb/sig/lib/selenium/webdriver/bidi.rbs [16]

    -def remove_callback: (untyped event, untyped id) -> untyped
    +def remove_callback: (Symbol event, Integer id) -> untyped
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion to specify more precise types for the event and id parameters in the remove_callback method enhances type safety and clarity, which is beneficial for maintainability and reducing potential runtime errors.

    7
    Specify more precise types for method parameters to improve type information and clarity

    Consider specifying more precise types for the options and service parameters in the
    initialize method. This will provide better type information and improve code
    clarity.

    rb/sig/lib/selenium/webdriver/common/driver_finder.rbs [9]

    -def initialize: (untyped options,untyped  service) -> void
    +def initialize: (Hash[Symbol, untyped] options, Service service) -> void
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Providing more precise types for the options and service parameters in the initialize method improves type information and code clarity, which can help prevent type-related errors and make the code easier to understand.

    7
    Specify a more precise type for a class attribute to improve type information and clarity

    Consider specifying a more precise type for extra_finders instead of using untyped.
    This will provide better type information and improve code clarity.

    rb/sig/lib/selenium/webdriver/common/search_context.rbs [8]

    -attr_accessor self.extra_finders: untyped
    +attr_accessor self.extra_finders: Hash[Symbol, Proc]
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using a more precise type for extra_finders instead of untyped enhances type information and clarity, which aids in understanding the code and catching type-related issues early.

    7

    💡 Need additional feedback ? start a PR chat

    @aguspe aguspe changed the title Add missing methods [rb] Add missing RBS methods Oct 24, 2024
    @p0deje p0deje merged commit 798f3f9 into SeleniumHQ:trunk Oct 25, 2024
    22 of 23 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants