-
Notifications
You must be signed in to change notification settings - Fork 12
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: updating the selenium selectors for the updated site. added a wa… #15
base: master
Are you sure you want to change the base?
fix: updating the selenium selectors for the updated site. added a wa… #15
Conversation
…it at the end of the order script for the user to manually login
|
||
print("login success!") | ||
return True | ||
except TimeoutException: | ||
print("login failed!") | ||
# close the browser and stop here | ||
self.browser.quit() | ||
# self.browser.quit() |
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.
This will result in open, dangling windows to stay around. Do we want this?
@@ -416,12 +451,16 @@ def __process_select_lego_set(self, lego_set): | |||
lego_set=lego_set)) | |||
setno_field = self.wait.until( | |||
EC.element_to_be_clickable( | |||
(By.CSS_SELECTOR, '.product-search input[ng-model=productNumber]')) | |||
(By.XPATH, "//input[@data-cy = 'enter-set-number-search']")) |
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.
It looks like you prefer XPath over CSS selectors. I feel this is less readable, harder to understand, harder to maintain. Would you be willing to change all calls to CSS selectors?
return | ||
# The site now fails these login attempts as they are too fast. | ||
# if not self._process_login(): | ||
# return |
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.
Hmmm. Should we remove this code? Or can we do something about it? (Print some log output that explains what happens?)
.until(EC.element_to_be_clickable((By.XPATH, "//button[@data-test = 'age-gate-grown-up-cta']")))\ | ||
.click() | ||
except NoSuchElementException: | ||
print("!!! Something's wrong with the age gate button") |
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.
Would it make sense to suggest a solution? (e.g. "Verify that ... foo ... bar.")
.until(EC.element_to_be_clickable((By.XPATH, "//button[@data-cy = 'choose-country-button']")))\ | ||
.click() | ||
except NoSuchElementException: | ||
print("!!! Something's wrong with the age gate button") |
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.
Same here. We want to be helpful, don't we?
# Since the new site fails automated logins, we will wait for the user to login manually at the end | ||
user_choice = input('Please click ENTER button to close application') | ||
if not user_choice: | ||
quit() |
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.
What's the point of the if
here? Should that better be a while
instead?
…it at the end of the order script for the user to manually login