Fix select_top_level_subject() method #273
Conversation
| Update Reviewer's Actions - Path to run the test as below |
There was a problem hiding this comment.
Mostly small stuff, and some other things we'll want to go over at Forest Fire. It might all be good, but I'm not familiar enough with the problem to say. It's also been awhile since I worked on selenium code, so my memory may not be working well. We will see!
Cheers,
@felliott
pages/preprints.py Outdated
| @@ -1,9 +1,13 @@ | |||
| import time | |||
pages/preprints.py Outdated
| import time | ||
| from urllib.parse import urljoin | ||
| | ||
| import ipdb |
| ) | ||
| | ||
| def select_top_level_subject(self, selection): | ||
| subject_selector = 'div[data-analytics-scope="Browse"] > ul > li' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I haven't found the method like this one in GroupLocator()
pages/preprints.py Outdated
| | ||
| def select_top_level_subject(self, selection): | ||
| subject_selector = 'div[data-analytics-scope="Browse"] > ul > li' | ||
| wait = WebDriverWait(self.driver, 20) |
There was a problem hiding this comment.
question: 20 seconds sounds like a long time. Do we need that long?
There was a problem hiding this comment.
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.
pages/preprints.py Outdated
| def select_top_level_subject(self, selection): | ||
| subject_selector = 'div[data-analytics-scope="Browse"] > ul > li' | ||
| wait = WebDriverWait(self.driver, 20) | ||
| wait.until(text_to_be_present_in_elements((By.CSS_SELECTOR, subject_selector), selection)) |
There was a problem hiding this comment.
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.
| 'input.ember-checkbox.ember-view' | ||
| ) | ||
| checkbox.click() | ||
| subject.click() |
There was a problem hiding this comment.
Q: does this work? I always remember checkboxes being difficult to trigger a click on. If this works, awesome!
base/expected_conditions.py Outdated
| 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]) |
There was a problem hiding this comment.
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.
Shtogryn force-pushed the pshtohryn/ENG-6071/fix/select_top_level_subject branch from 4a025e6 to f637a4c Compare 
Purpose
The purpose of update is to fix failed test step of test_edit_preprint test
Summary of Changes
Updated select_top_level_subject() method
Reviewer's Actions
git fetch <remote> pull/273/head:pshtohryn/ENG-6071/fix/select_top_level_subjectRun this test using
MacOS - pytest tests/test_preprints.py::TestPreprintWorkflow::test_edit_preprint -sv
Windows - pytest .\tests\test_preprints.py::TestPreprintWorkflow::test_edit_preprint -sv
Testing Changes Moving Forward
N/A
Ticket
https://openscience.atlassian.net/browse/ENG-6071