Add print page command to ruby binding - #8920
Conversation
fcb3d59 to
6cabffb
Compare
| def print_page(**options) | ||
| options[:page_ranges] &&= Array(options[:page_ranges]) | ||
|
|
||
| bridge.print_page(options) |
There was a problem hiding this comment.
the bridge here isn't defined as an accessor / reader to try mask it better. So you need to directly call the iVar @bridge here.
| def print_page(**options) | ||
| options[:page_ranges] &&= Array(options[:page_ranges]) | ||
|
|
||
| bridge.print_page(options) |
There was a problem hiding this comment.
Same here. and all others.
|
|
||
| module Selenium | ||
| module WebDriver | ||
| MAGIC_NUMBER = 'JVBER' |
There was a problem hiding this comment.
if you set this as a let it's probably syntatically better.
let(:magic_number) { 'JVBER' }
| end | ||
|
|
||
| it 'should return base64 for print command' do | ||
| expect(@driver.print_page().include?(MAGIC_NUMBER)).to be true |
There was a problem hiding this comment.
this can be re-written to expect(@driver.print_page).to include(magic_number)
Reason: Rspec includes most predicates built into ruby as rspec matchers
Use this style for the ones below
6cabffb to
5d3f892
Compare
@luke-hill Applied your review comments. Please take a look. Thanks 🙇 |
luke-hill
left a comment
There was a problem hiding this comment.
All good. There are also a few stylistic ruby specific things you could do. But I'll let you learn those yourself.
If inside /rb you run the command rubocop it will flag up minor tweaks. If you then run rubocop -A or maybe rubocop -a dependent on version it will auto fix many of them.
18fba7e to
36c72e0
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Motivation and Context
Types of changes
Checklist