[rb] Add PrintOptions Implementation for Ruby WebDriver#15158
[rb] Add PrintOptions Implementation for Ruby WebDriver#15158p0deje merged 22 commits intoSeleniumHQ:trunkfrom
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
aguspe
left a comment
There was a problem hiding this comment.
It would be great to also have integration tests so we can call print_options from the driver and to run the formatting command from the Selenium folder sh ./scripts/format.sh so there are no formatting errors
Thank you so much for your help!
| @@ -0,0 +1,78 @@ | |||
| # <copyright file="print_options.rb" company="Selenium Committers"> | |||
There was a problem hiding this comment.
Please run sh ./scripts/format.sh to fix the formatting
rb/spec/spec_helper.rb
Outdated
| @@ -0,0 +1,15 @@ | |||
| require 'spec_helper' | |||
There was a problem hiding this comment.
Why is a new spec_helper file needed?
There was a problem hiding this comment.
Instead of a new spec_helper, you should:
- Move
print_options.rbtorb/lib/selenium/webdriver/common/ requirethe file inrb/lib/selenium/webdriver/common.rb
There was a problem hiding this comment.
Thanks for reviewing @aguspe, @p0deje
All review comments addressed:
- Moved
PrintOptionstocommon/, and required it incommon.rb - Cleaned up and created correct
spec_helper.rb - Added complete unit test coverage for page size variants
- Included license headers
- Ran
./scripts/format.sh
Please let me know if anything else is needed — thank you!
|
|
||
| # frozen_string_literal: true | ||
|
|
||
| require 'selenium/webdriver/common/print_options' |
There was a problem hiding this comment.
- Can you move
frozen_string_literalto the top? It has to be the very first line in the file. - Since the code itself is in
commonfolder, can you move this spec torb/spec/unit/selenium/webdriver/common/print_options_spec.rb? - Once moved, you need to require a default spec helper by adding this line before describe:
require File.expand_path('../spec_helper', __dir__)- Please run
./scripts/format.shand re-push.
There was a problem hiding this comment.
Addressed Review Comments
- Moved
# frozen_string_literal: trueto the top of the file. - Relocated the spec file under
webdriver/common/. - Updated the
requirepath to use the sharedspec_helper. - Ran
./scripts/format.shand re-pushed to ensure all formatting is correct.
Thank you for reviewing @p0deje
|
It is good to be merged, but there is a copyright notice issue on CI, can you please fix it? |
There are no errors on local. On running format.sh, I see this message (Attached the log)
|
|
@yvsvarma can you try running |
|
@yvsvarma Thank you for the contribution! |
User description
Description
This PR introduces the
PrintOptionsimplementation for Ruby WebDriver, aligning it with similar functionality available in Java, .NET, and Python bindings. ThePrintOptionsclass (print_options.rb) provides an interface to define printing configurations, such as page size, orientation, scale, and margins, and converts these configurations into a hash suitable for WebDriver commands.Key Changes
1.
PrintOptionsClass (lib/selenium/webdriver/print_options.rb):Added a class to manage print configurations with default values:
portrait1.0falseLetter(21.59 cm x 27.94 cm).2. Supports predefined page sizes:
to_hmethod generates a hash representation of the options for WebDriver.Unit Tests (
spec/unit/selenium/print_options_spec.rb):Added tests to ensure:
Testing
Verified functionality through the rspec test suite:
rspec spec/unit/selenium/print_options_spec.rb- Result: All tests pass successfully.Reference Implementations
This implementation follows the structure and functionality of the
PrintOptionsfeature in other languages:PR Type
Enhancement, Tests
Description
Introduced
PrintOptionsclass for Ruby WebDriver to manage print configurations.Added support for default, predefined, and custom page sizes.
Implemented
to_hmethod to convert configurations into WebDriver-compatible hash.Added comprehensive unit tests for
PrintOptionsfunctionality.Changes walkthrough 📝
print_options.rb
Introduced `PrintOptions` class for print configurationsrb/lib/selenium/webdriver/print_options.rb
PrintOptionsclass to manage print configurations.to_hmethod for hash conversion.spec_helper.rb
Added RSpec configuration for testsrb/spec/spec_helper.rb
PrintOptionstesting.print_options_spec.rb
Added unit tests for `PrintOptions` functionalityrb/spec/unit/selenium/print_options_spec.rb
PrintOptionsclass.