#762 #764 google ecommerce tests #767
Conversation
ArtemijRodionov
force-pushed
the
762-google-ecommerce-tests
branch
from
7e523bb to
101d124
Compare
ArtemijRodionov
force-pushed
the
762-google-ecommerce-tests
branch
from
2b0e597 to
71df91f
Compare
ArtemijRodionov
force-pushed
the
762-google-ecommerce-tests
branch
3 times, most recently
from
6bcbcab to
2d7e10b
Compare
ArtemijRodionov
changed the title
ArtemijRodionov
force-pushed
the
762-google-ecommerce-tests
branch
from
2d7e10b to
baf3c25
Compare
shopelectro/selenium/pages/page.py
Outdated
| from shopelectro.selenium import SiteDriver | ||
|
|
||
| from selenium.webdriver.common.by import By | ||
| from selenium.webdriver.support import expected_conditions as EC | ||
|
|
||
|
|
||
| def matched_url(fn): |
There was a problem hiding this comment.
decorator requires self: Page object with driver and path fields inside. So, it has not clear interface.
Here we have function, that contains self: Page and can be used only inside Page class. This is pretty the definition of method.
We should turn this decorator to Page's method, i think.
After putting it to Page class we can clarify it's semantic. The purpose of method is to wait until a page is loaded. Url waiting is just realization, it should not be visible throw the method interface.
Smth like Page.wait_loaded is good name
There was a problem hiding this comment.
The responsibility of this decorator isn't to wait for a page to load
Right now we have to synchronize the browser location and Page abstraction manually to prevent an inconsistent state issue, so i have created this decorator to forbid usage of wrong Page instance at a specific browser location
If we will make a method instead of the decorator, it maybe will be more clear from semantic point of view, but we still have to manually call it
I want to propose Navigation class instead that will responsible for this synchronization.
I will create todo for it and edit the decorator's comment to clarify its purpose
There was a problem hiding this comment.
@artemiy312 , i thought about separated class too, but it seems to me too much to create separated class for one method. The url sync mech is good related to Page class.
have to synchronize the browser location and Page abstraction manually to prevent an inconsistent state issue
but this sync is just common waiting by it's semantic. Waiting text in input has the same nature, but we use term "wait", not sync.
Anyway Page.sync method (or sync_page deco) looks much good then matched_urls
shopelectro/tests/tests_analytics.py
Outdated
| @@ -0,0 +1,36 @@ | |||
| from django.test import override_settings, tag | |||
There was a problem hiding this comment.
test_counters is more wide name. We'll move here all Ya.Metrica tests in the future, as i see.
By the way, if we should move, create pls pdd task to do it
There was a problem hiding this comment.
counters are part of web analytics and js eCommerce is not a counter, so i'd prefer tests_js_analytics.py
shopelectro/tests/tests_analytics.py
Outdated
|
|
||
| fixtures = ['dump.json'] | ||
|
|
||
| order: Order |
There was a problem hiding this comment.
this field looks strange: we don't use it anywhere
shopelectro/tests/tests_analytics.py
Outdated
| order = Order.objects.order_by('-created').first() | ||
| reached = self.browser.execute_script('return gaObject.results;') | ||
|
|
||
| # @todo #762:30m Implement assertion of an order and reached targets for test_google_ecommerce_purchase. |
There was a problem hiding this comment.
the header is too long for task, i think.
# @todo ... Assert if counter object results are successful for order.
GA counter object should contain a correct order and reached targets data.
Up to you
| def add_to_cart(browser, live_server_url): | ||
| browser.get(live_server_url + Product.objects.first().url) | ||
| def add_to_cart(browser): | ||
| browser.get(Product.objects.first().url) | ||
| browser.find_element_by_class_name('btn-to-basket').click() |
There was a problem hiding this comment.
seems some waiting should be good here. Maybe do pdd for it?
There was a problem hiding this comment.
I believe until it doesn't raise issues we shouldn't wait it, but if you will insist i create a todo
Closes #762 Closes #764