-
Notifications
You must be signed in to change notification settings - Fork 103
7 screen play #5
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
base: main
Are you sure you want to change the base?
Changes from all commits
12ead22
99bbc30
d156631
9cd8e2d
1ffda75
0f82e73
45a0c4e
8f3b325
d807560
7f8d93e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| """ | ||
| This module contains DuckDuckGo pages. | ||
| """ | ||
|
|
||
| from playwright.sync_api import Page | ||
|
|
||
|
|
||
| class SearchPage: | ||
|
|
||
| URL = 'https://www.duckduckgo.com' | ||
|
|
||
| def __init__(self, page: Page) -> None: | ||
| self.page = page | ||
| self.search_button = page.locator('#search_button_homepage') | ||
| self.search_input = page.locator('#search_form_input_homepage') | ||
|
|
||
|
|
||
| class ResultPage: | ||
| def __init__(self, page: Page) -> None: | ||
| self.page = page | ||
| self.result_links = page.locator('a[data-testid="result-title-a"]') | ||
| self.search_input = page.locator('#search_form_input') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| """ | ||
| This module contains DuckDuckGo Questions. | ||
| """ | ||
|
|
||
| from abc import ABC, abstractmethod | ||
| from interactions.duckduckgo.pages import ResultPage | ||
| from playwright.sync_api import Page | ||
| from screenplay.pattern import Actor, Question, Answer | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing screenplay.pattern module will cause import failure 🚨 Critical This file imports from 'screenplay.pattern' which doesn't exist in the repository. This is a critical blocker - the tests will fail immediately on import with a ModuleNotFoundError. The entire PR depends on this missing module, which needs to be either added to the repository or installed as a dependency in requirements.txt. Without this module defining Actor, Question, Answer, and Task classes, none of the refactored code will work. Suggested fix: # Option 1: Create screenplay/pattern.py in the repository
# Option 2: Add to requirements.txt if it's an external package:
screenplay-pattern>=1.0.0 |
||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # DuckDuckGo Question parent class | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class DuckDuckGoQuestion(Question[Answer], ABC): | ||
|
|
||
| @abstractmethod | ||
| def request_on_page(self, actor: Actor, page: Page) -> Answer: | ||
| pass | ||
|
|
||
| def request_as(self, actor: Actor) -> Answer: | ||
| page: Page = actor.using('page') | ||
| return self.request_on_page(actor, page) | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # DuckDuckGo Questions | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class result_link_titles(DuckDuckGoQuestion[list[str]]): | ||
|
|
||
| def request_on_page(self, _, page: Page) -> list[str]: | ||
| result_page = ResultPage(page) | ||
| result_page.result_links.nth(4).wait_for() | ||
| return result_page.result_links.all_text_contents() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| """ | ||
| This module contains DuckDuckGo Tasks. | ||
| """ | ||
|
|
||
| from abc import ABC, abstractmethod | ||
| from interactions.duckduckgo.pages import ResultPage, SearchPage | ||
| from interactions.duckduckgo.questions import result_link_titles | ||
| from playwright.sync_api import Page, expect | ||
| from screenplay.pattern import Actor, Task | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # DuckDuckGo Task parent class | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class DuckDuckGoTask(Task, ABC): | ||
|
|
||
| @abstractmethod | ||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
| pass | ||
|
|
||
| def perform_as(self, actor: Actor) -> None: | ||
| page: Page = actor.using('page') | ||
| self.perform_on_page(actor, page) | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # DuckDuckGo Tasks | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class load_duckduckgo(DuckDuckGoTask): | ||
|
|
||
| def perform_on_page(self, _, page: Page) -> None: | ||
| page.goto(SearchPage.URL) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded wait for 5th element may cause flaky tests Waiting for exactly the 5th result (index 4) to appear is fragile and could cause test failures if search results return fewer items. This hardcoded wait assumes DuckDuckGo will always return at least 5 results, which may not be true for all searches or if the page structure changes. This can lead to timeout errors and flaky test behavior. Suggested fix: result_page.result_links.first.wait_for()
# Or wait for any results to be visible
# result_page.result_links.nth(0).wait_for()References: |
||
|
|
||
|
|
||
| class search_duckduckgo_for(DuckDuckGoTask): | ||
|
|
||
| def __init__(self, phrase: str) -> None: | ||
| super().__init__() | ||
| self.phrase = phrase | ||
|
|
||
| def perform_on_page(self, _, page: Page) -> None: | ||
| search_page = SearchPage(page) | ||
| search_page.search_input.fill(self.phrase) | ||
| search_page.search_button.click() | ||
|
|
||
|
|
||
| class verify_page_title_is(DuckDuckGoTask): | ||
|
|
||
| def __init__(self, title: str) -> None: | ||
| super().__init__() | ||
| self.title = title | ||
|
|
||
| def perform_on_page(self, _, page: Page) -> None: | ||
| expect(page).to_have_title(self.title) | ||
|
|
||
|
|
||
| class verify_result_link_titles_contain(Task): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent class inheritance pattern The verify_result_link_titles_contain class inherits directly from Task instead of DuckDuckGoTask like the other classes in this module. This breaks the established pattern and means it doesn't get the perform_on_page abstraction. This inconsistency can confuse maintainers and makes the codebase harder to understand. It appears to be intentional since it implements perform_as differently, but the reasoning isn't documented. Suggested fix: # This task needs direct Task inheritance because it uses asks_for() instead of page operations
class verify_result_link_titles_contain(Task):
# ... rest of implementation |
||
|
|
||
| def __init__(self, phrase: str, minimum: int = 1) -> None: | ||
| super().__init__() | ||
| self.phrase = phrase | ||
| self.minimum = minimum | ||
|
|
||
| def perform_as(self, actor: Actor) -> None: | ||
| titles = actor.asks_for(result_link_titles()) | ||
| matches = [t for t in titles if self.phrase.lower() in t.lower()] | ||
| assert len(matches) >= self.minimum | ||
|
|
||
|
|
||
| class verify_search_result_query_is(DuckDuckGoTask): | ||
|
|
||
| def __init__(self, phrase: str) -> None: | ||
| super().__init__() | ||
| self.phrase = phrase | ||
|
|
||
| def perform_on_page(self, _, page: Page) -> None: | ||
| result_page = ResultPage(page) | ||
| expect(result_page.search_input).to_have_value(self.phrase) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| """ | ||
| This module contains REST API calls for GitHub Projects. | ||
| """ | ||
|
|
||
| from abc import ABC, abstractmethod | ||
| from playwright.sync_api import APIRequestContext, APIResponse, expect | ||
| from screenplay.pattern import Actor, Question | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # GitHub Project Call parent class | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class GitHubProjectCall(Question[APIResponse], ABC): | ||
|
|
||
| @abstractmethod | ||
| def call_as(self, actor: Actor, context: APIRequestContext) -> APIResponse: | ||
| pass | ||
|
|
||
| def request_as(self, actor: Actor) -> APIResponse: | ||
| context: APIRequestContext = actor.using('gh_context') | ||
| return self.call_as(actor, context) | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # GitHub Project Calls | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class create_card(GitHubProjectCall): | ||
|
|
||
| def __init__(self, column_id: str, note: str | None = None) -> None: | ||
| self.column_id = column_id | ||
| self.note = note | ||
|
|
||
| def call_as(self, actor: Actor, context: APIRequestContext) -> APIResponse: | ||
| response = context.post( | ||
| f'/projects/columns/{self.column_id}/cards', | ||
| data={'note': self.note}) | ||
| expect(response).to_be_ok() | ||
| assert response.json()['note'] == self.note | ||
| return response | ||
|
|
||
|
|
||
| class retrieve_card(GitHubProjectCall): | ||
|
|
||
| def __init__(self, card_id: str) -> None: | ||
| self.card_id = card_id | ||
|
|
||
| def call_as(self, actor: Actor, context: APIRequestContext) -> APIResponse: | ||
| response = context.get(f'/projects/columns/cards/{self.card_id}') | ||
| expect(response).to_be_ok() | ||
| return response |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| """ | ||
| This module contains GitHub Project Tasks. | ||
| """ | ||
|
|
||
| from abc import ABC, abstractmethod | ||
| from playwright.sync_api import Page, expect | ||
| from screenplay.pattern import Actor, Task | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # GitHub Project Task parent class | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class GitHubProjectTask(Task, ABC): | ||
|
|
||
| @abstractmethod | ||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
| pass | ||
|
|
||
| def perform_as(self, actor: Actor) -> None: | ||
| page: Page = actor.using('page') | ||
| self.perform_on_page(actor, page) | ||
|
|
||
|
|
||
| # ------------------------------------------------------------ | ||
| # GitHub Project Tasks | ||
| # ------------------------------------------------------------ | ||
|
|
||
| class log_into_github_as(GitHubProjectTask): | ||
|
|
||
| def __init__(self, username: str, password: str) -> None: | ||
| self.username = username | ||
| self.password = password | ||
|
|
||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for login flow The login task doesn't verify that login was successful before proceeding. If authentication fails (wrong credentials, CAPTCHA, 2FA required, etc.), the test will continue and fail later with confusing error messages. This makes debugging failed tests much harder because the root cause (failed login) is obscured by subsequent failures. Production GitHub accounts often have 2FA enabled, which would cause this login flow to fail silently. Suggested fix: def perform_on_page(self, actor: Actor, page: Page) -> None:
page.goto(f'https://github.com/login')
page.locator('id=login_field').fill(self.username)
page.locator('id=password').fill(self.password)
page.locator('input[name="commit"]').click()
# Verify login succeeded by checking for user avatar or dashboard
page.locator('[data-login="{}"]'.format(self.username)).wait_for(timeout=10000)References: |
||
| page.goto(f'https://github.com/login') | ||
| page.locator('id=login_field').fill(self.username) | ||
| page.locator('id=password').fill(self.password) | ||
| page.locator('input[name="commit"]').click() | ||
|
|
||
|
|
||
| class load_github_project_for(GitHubProjectTask): | ||
|
|
||
| def __init__(self, username: str, project_number: str) -> None: | ||
| self.username = username | ||
| self.project_number = project_number | ||
|
|
||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
| page.goto(f'https://github.com/users/{self.username}/projects/{self.project_number}') | ||
|
|
||
|
|
||
| class verify_card_appears_with(GitHubProjectTask): | ||
|
|
||
| def __init__(self, column_id: str, note: str) -> None: | ||
| self.column_id = column_id | ||
| self.note = note | ||
|
|
||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
| card_xpath = f'//div[@id="column-cards-{self.column_id}"]//p[contains(text(), "{self.note}")]' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. XPath injection vulnerability in dynamic locator construction User-controlled input (column_id and note) is directly interpolated into XPath expressions without sanitization. If the 'note' parameter contains special XPath characters like quotes, parentheses, or brackets, it could break the XPath query or potentially be exploited for XPath injection. While this is a test framework and not production code, it can still cause test failures or unexpected behavior if notes contain special characters. Suggested fix: column_locator = page.locator(f'#column-cards-{self.column_id}')
card_locator = column_locator.locator('p', has_text=self.note)
expect(card_locator).to_be_visible()References: |
||
| expect(page.locator(card_xpath)).to_be_visible() | ||
|
|
||
|
|
||
| class move_card_to(GitHubProjectTask): | ||
|
|
||
| def __init__(self, column_id: str, note: str) -> None: | ||
| self.column_id = column_id | ||
| self.note = note | ||
|
|
||
| def perform_on_page(self, actor: Actor, page: Page) -> None: | ||
| page.drag_and_drop(f'text="{self.note}"', f'id=column-cards-{self.column_id}') | ||
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline at end of file⚠️ Medium
This file is missing a newline character at the end. While this won't cause runtime errors, it violates POSIX standards and can cause issues with certain text processing tools and version control systems. Most Python linters and formatters will flag this as a code style issue.
Suggested fix:
References: