[py] Enhance PrintOptions to support default, predefined, and custom page sizes (#15052)

[yvsvarma]

User description

Description:

• Added support for default page size (A4) in PrintOptions.
• Introduced predefined page sizes (A4, LEGAL, LETTER, TABLOID)
• Enhanced set_page_size to handle predefined and custom sizes with validation.
• Updated test cases to validate default, predefined, and custom page size functionalities.
• Note: This implementation aligns with and extends the functionality introduced in Java PR #15052, ensuring consistency across both Python and Java libraries.

---

PR Type

Enhancement, Tests

---

Description

• Added predefined page sizes (A4, LEGAL, LETTER, TABLOID) in PrintOptions.

• Set default page size to A4 in PrintOptions.

• Introduced set_page_size method for predefined and custom sizes.

• Added unit tests for default, predefined, and custom page sizes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	print_page_options.py Add predefined and custom page size handling                          py/selenium/webdriver/common/print_page_options.py Added predefined page sizes (A4, LEGAL, LETTER, TABLOID). Set default page size to A4. Introduced set_page_size method for handling page sizes. Validated page size dimensions in set_page_size. +28/-1
Tests	print_page_options_tests.py Add tests for page size functionalities                                    py/test/unit/selenium/webdriver/common/print_page_options_tests.py Added tests for default A4 page size. Tested predefined page sizes (e.g., LEGAL). Verified custom page size functionality. +16/-0

---

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

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Input Validation The set_page_size method should validate that both 'height' and 'width' keys exist in the input dictionary before accessing them to avoid potential KeyError exceptions self._validate_num_property("height", page_size["height"]) self._validate_num_property("width", page_size["width"]) self._page["height"] = page_size["height"] self._page["width"] = page_size["width"] Documentation Gap The docstring for set_page_size method should mention the units (centimeters) and any constraints on valid dimensions def set_page_size(self, page_size: dict) -> None: """ Sets the page size to predefined or custom dimensions. Parameters ---------- page_size: dict A dictionary containing `height` and `width` as keys with respective values. Example ------- self.set_page_size(PageSize.A4) # A4 predefined size self.set_page_size({"height": 15.0, "width": 20.0}) # Custom size in cm """

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add input validation to prevent KeyError exceptions when accessing dictionary values Add validation for the page_size dictionary to ensure it contains required 'height' and 'width' keys before accessing them. py/selenium/webdriver/common/print_page_options.py [420-437] def set_page_size(self, page_size: dict) -> None: + if not isinstance(page_size, dict) or not all(key in page_size for key in ['height', 'width']): + raise ValueError("page_size must be a dictionary containing 'height' and 'width' keys") self._validate_num_property("height", page_size["height"]) self._validate_num_property("width", page_size["width"]) Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion addresses a critical defensive programming issue by preventing potential KeyError exceptions when invalid dictionary input is provided. This type of validation is essential for robust error handling.	9
	Add validation to ensure numeric values are positive to prevent invalid page dimensions Add validation to ensure page dimensions are positive numbers. Currently negative values would be accepted which could cause rendering issues. py/selenium/webdriver/common/print_page_options.py [420-439] def set_page_size(self, page_size: dict) -> None: self._validate_num_property("height", page_size["height"]) self._validate_num_property("width", page_size["width"]) + if page_size["height"] <= 0 or page_size["width"] <= 0: + raise ValueError("Page dimensions must be positive numbers") self._page["height"] = page_size["height"] self._page["width"] = page_size["width"] Apply this suggestion Suggestion importance[1-10]: 8 Why: This is a critical validation check that prevents potential rendering issues or errors by ensuring page dimensions are positive numbers. The suggestion addresses a real gap in input validation that could lead to runtime problems.	8
General	Add test coverage for error handling of invalid page size inputs Add test cases for invalid page size inputs to verify error handling behavior. py/test/unit/selenium/webdriver/common/print_page_options_tests.py [47-51] def test_set_page_size(print_options): # Example of setting a default (A4) assert print_options.page_width == PrintOptions.A4["width"] assert print_options.page_height == PrintOptions.A4["height"] + + # Test invalid inputs + with pytest.raises(ValueError): + print_options.set_page_size({"height": -1, "width": 10}) + with pytest.raises(ValueError): + print_options.set_page_size({"width": 10}) # Missing height Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves test coverage by adding important edge cases for error handling, ensuring the validation logic works as expected. This helps maintain code quality and prevents future regressions.	7

[shbenzer]

Thank you @yvsvarma

[shbenzer]

Please close PR #15052

[yvsvarma]

Please close PR #15052

Hi Simon, Thanks for reviewing. This PR is only for Python, independent of #15052 (which is only for Java).

[shbenzer]

You're right - today's been hectic for me, my bad.

LGTM

[nvborisenko]

Looks bad to me. Magic numbers, applied especially for python? Can we create a feature request.. and implement the feature across all bindings (if needed)? Thanks all for contribution.

[shbenzer]

Looks bad to me. Magic numbers, applied especially for python?

I disagree with this. He's not using magic numbers, but setting the default value to constants (e.g. A4) that are standard. It could be enhanced, though. For example, adding better/more detailed exception handling.

Can we create a feature request.. and implement the feature across all bindings (if needed)?

I agree - it would be much better to standardize this feature.

[yvsvarma]

Looks bad to me. Magic numbers, applied especially for python?

I disagree with this. He's not using magic numbers, but setting the default value to constants (e.g. A4) that are standard. It could be enhanced, though. For example, adding better/more detailed exception handling.

Can we create a feature request.. and implement the feature across all bindings (if needed)?

I agree - it would be much better to standardize this feature.

Thanks @nvborisenko and @shbenzer for reviewing this.
We have another PR created for this already exclusively for Java - Java PR . Since it has couple of commits already based on the review comments, I have created this new PR for Python. Please let me know if you want me to create single PR for all languages, or any other best approach for this.

[yvsvarma]

Update: A feature request was opened for this by @shbenzer to this for all bindings.

[yvsvarma]

@pujagani , Can you please take a look? I have addressed all comments in this PR.

[yvsvarma]

@AutomatedTester , @pujagani can you please review this PR?

[VietND96]

CI RBE is failed

//py:unit-test/unit/selenium/webdriver/common/print_page_options_tests.py FAILED in 2 out of 2 in 2.4s