[Playwright] Date time and forms and some component widget migration#104
Merged
digitronik merged 2 commits intoOct 17, 2025
Merged
Conversation
Reviewer's GuideThis PR migrates core widget components and tests from Selenium-style interactions to Playwright-friendly APIs by refactoring locator expressions, replacing element actions and property accessors with browser helper methods, and standardizing text and visibility handling across the codebase. Sequence diagram for refactored form select fill and read methodssequenceDiagram
participant User
participant FormSelect
participant Browser
User->>FormSelect: fill(value)
FormSelect->>Browser: __element__().select_option(label=value)
User->>FormSelect: read()
FormSelect->>Browser: text("option:checked")
Browser-->>FormSelect: selected option text
Sequence diagram for calendar month year setter (Playwright migration)sequenceDiagram
participant User
participant BaseCalendarMonth
participant Browser
User->>BaseCalendarMonth: set year(value)
BaseCalendarMonth->>Browser: fill(str(value), YEAR_INPUT_LOCATOR)
BaseCalendarMonth->>Browser: click(".//body")
Sequence diagram for pagination go_to_page method (Playwright migration)sequenceDiagram
participant User
participant Pagination
participant Browser
User->>Pagination: go_to_page(value)
Pagination->>Pagination: _current_page.fill(value)
Pagination->>Browser: _current_page.__element__().press("Enter")
Class diagram for refactored widget components (Playwright migration)classDiagram
class BaseCalendarMonth {
+CALENDAR_HEADER
+MONTH_SELECT_LOCATOR
+_month_select_widget
+YEAR_INPUT_LOCATOR
+DATE_LOCATOR
+PREV_BUTTON_LOCATOR
+NEXT_BUTTON_LOCATOR
+TABLE
+SELECTED_DATE_LOCATOR
+year
+month
+day
}
class BaseDualListSelector {
+_available
+_chosen
+_left_list
+_right_list
+_left_title
+_right_title
+_left_elements
+_right_elements
+read(selected_only)
+reset_selected(left_items)
}
class BaseClipboardCopy {
+is_editable()
+is_inline
+text
}
class FormSelect {
+fill(value)
+read()
+__repr__()
+all_enabled_options
+_select_element
+__element__()
}
class Pagination {
+is_enabled()
+set_per_page(count)
+go_to_page(value)
+__iter__()
+_last
+_options
+_next
+_current_page
+current_page
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- I see many manual conversions from element.text or get_property to browser.text or text_content; consider centralizing text retrieval in a helper or in the base widget to ensure consistency instead of updating each usage individually.
- The workaround of clicking on the body after filling the year input feels brittle; it might be more robust to trigger a blur event or wait for the field’s change event rather than relying on an extra click.
- The CI workflow is now ignoring several test files—please confirm that skipping those tests is intentional and document the rationale to avoid masking potential regressions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- I see many manual conversions from element.text or get_property to browser.text or text_content; consider centralizing text retrieval in a helper or in the base widget to ensure consistency instead of updating each usage individually.
- The workaround of clicking on the body after filling the year input feels brittle; it might be more robust to trigger a blur event or wait for the field’s change event rather than relying on an extra click.
- The CI workflow is now ignoring several test files—please confirm that skipping those tests is intentional and document the rationale to avoid masking potential regressions.
## Individual Comments
### Comment 1
<location> `testing/components/test_dual_list_selector.py:27-28` </location>
<code_context>
def test_available(view):
- view.dual_list_selector._available.is_displayed()
+ view.dual_list_selector._available.is_visible()
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding assertions to verify the expected state after visibility checks.
The test should assert the result of is_visible() to confirm the component's state is as expected.
```suggestion
def test_available(view):
assert view.dual_list_selector._available.is_visible()
```
</issue_to_address>
### Comment 2
<location> `testing/components/test_popover.py:46` </location>
<code_context>
def test_popover_close(popover):
popover.close()
+ time.sleep(0.5)
assert not popover.is_displayed
</code_context>
<issue_to_address>
**suggestion (testing):** Consider replacing time.sleep with a more robust wait mechanism.
Explicitly wait for popover.is_displayed to become False instead of using time.sleep to reduce test flakiness.
</issue_to_address>
### Comment 3
<location> `src/widgetastic_patternfly5/components/clipboard_copy.py:23-26` </location>
<code_context>
@property
def is_editable(self):
if self.is_inline:
return False
if "readonly" in self.browser.attributes(self.text):
return False
else:
return True
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
- Simplify boolean if expression ([`boolean-if-exp-identity`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/boolean-if-exp-identity/))
- Remove unnecessary casts to int, str, float or bool ([`remove-unnecessary-cast`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-cast/))
```suggestion
return "readonly" not in self.browser.attributes(self.text)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
digitronik
force-pushed
the
date_time_forms
branch
from
b77e9f2 to
8619c17
Compare
mshriver
reviewed
Oct 17, 2025
mshriver
reviewed
Oct 17, 2025
mshriver
approved these changes
Oct 17, 2025
mshriver
left a comment
mshriver
left a comment
Collaborator
There was a problem hiding this comment.
Possible simplification of locator string, see individual comment that is non-blocking.
digitronik
force-pushed
the
date_time_forms
branch
from
8619c17 to
a4dcaa4
Compare
digitronik
force-pushed
the
date_time_forms
branch
from
a4dcaa4 to
bb943f5
Compare
digitronik
merged commit 88d8e93
into
RedHatQE:feature-playwright
10 of 11 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Migrate widgetastic_patternfly5 components and tests to Playwright’s current API by updating locators, element interactions, and assertions, adjust test fixtures and CI workflow to improve stability.
Enhancements:
CI:
Tests: