Skip to content
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

Fix select_top_level_subject() method #273

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions base/expected_conditions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from selenium.common.exceptions import StaleElementReferenceException
from selenium.webdriver.remote.webdriver import WebDriver
from selenium.webdriver.support import expected_conditions as EC


Expand Down Expand Up @@ -25,3 +27,20 @@ def __init__(self, page_index):

def __call__(self, driver):
return len(driver.window_handles) > self.page_index


class text_to_be_present_in_elements(object):
""" An expectation for checking if the given text is present in the
specified element.
locator, text
"""
def __init__(self, locator, text_):
self.locator = locator
self.text = text_

def __call__(self, driver: WebDriver):
try:
element_texts = [element.text for element in driver.find_elements(*self.locator)]
return any([self.text in element_text for element_text in element_texts])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't use a "contains-text" selector, could these lines be combined to help the check short-circuit faster?. I'm not sure, but it might be worth investigating.

except StaleElementReferenceException:
return False
13 changes: 8 additions & 5 deletions pages/preprints.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove if unused

from urllib.parse import urljoin

import ipdb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove if unused

import pytest
from selenium.webdriver.common.by import By
from selenium.webdriver.support.wait import WebDriverWait

import settings
from base.expected_conditions import text_to_be_present_in_elements
from base.locators import (
ComponentLocator,
GroupLocator,
Expand Down Expand Up @@ -113,13 +117,12 @@ def select_from_dropdown_listbox(self, selection):
)

def select_top_level_subject(self, selection):
subject_selector = 'div[data-analytics-scope="Browse"] > ul > li'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could this be constructed from top_level_subjects, which has the same selector, but is a GroupLocator? I wonder if there's a .first method or something like that.

wait = WebDriverWait(self.driver, 20)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: 20 seconds sounds like a long time. Do we need that long?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this will bail early if the condition is met, correct? If so, 20s isn't the worst thing, but if it's taking that long, we might want a flag raised. This is probably an FF topic.

wait.until(text_to_be_present_in_elements((By.CSS_SELECTOR, subject_selector), selection))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be fine, but isn't there a selector to check text contents? I'm pretty sure there's one for xpath, not sure about css.

for subject in self.top_level_subjects:
if subject.text == selection:
# Find the checkbox element and click it to select the subject
checkbox = subject.find_element_by_css_selector(
'input.ember-checkbox.ember-view'
)
checkbox.click()
subject.click()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: does this work? I always remember checkboxes being difficult to trigger a click on. If this works, awesome!

break

first_selected_subject = Locator(By.CSS_SELECTOR, 'li[data-test-selected-subject]')
Expand Down