Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

address #73 #80

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
9 participants
Contributor

klrmn commented Oct 24, 2012

Please tell me what you think of this pull, before i get in any deeper.

  • fix wait_for_ajax
  • MozTrapEditRunPage
  • treat runs in manage runs list as sub-objects
  • treat suits in create/edit run as sub-objects
  • allow for both addition and removal of suites in runs

TODO:
figure out how to drag-n-drop

UPDATE:
pull is now fully ready. note that the test requires native.

@klrmn klrmn partially address #73
* fix wait_for_ajax
* MozTrapEditRunPage
* treat runs in manage runs list as sub-objects
* treat suits in create/edit run as sub-objects
* allow for both addition and removal of suites in runs

TODO:
figure out how to drag-n-drop
7a01058

@zacc zacc commented on an outdated diff Oct 24, 2012

pages/create_run_page.py
_submit_locator = (By.CSS_SELECTOR, '#run-add-form .form-actions > button')
+
+ # Note on __init__: it fails with 'ReferenceError: jQuery is not defined'
@zacc

zacc Oct 24, 2012

Contributor

Incidentally a dev for Socorro recently updated this method in socorro-tests to check for jQuery's existence and that .active == 0 to work around problems where on pages where jQuery might not exist.

@zacc zacc commented on the diff Oct 24, 2012

pages/manage_runs_page.py
+ @property
+ def get_runs(self):
+ runs = self.selenium.find_elements(*self._run_item_locator)
+ return [self.Run(self.testsetup, run) for run in runs]
+
+ class Run(Page):
+ _delete_run_locator = (By.CSS_SELECTOR, '.action-delete')
+ _run_activate_locator = (By.CSS_SELECTOR, '.status-action.active')
+ _run_status_locator = (By.CSS_SELECTOR, '.status-title')
+ _edit_run_locator = (By.CSS_SELECTOR, 'a.edit-link')
+
+ def __init__(self, testsetup, webelement):
+ Page.__init__(self, testsetup)
+ self.webelement = webelement
+
+ def delete(self):
@zacc

zacc Oct 24, 2012

Contributor

These would be better as click_delete, click_edit etc as that's the action on the element.

@klrmn

klrmn Oct 24, 2012

Contributor

after seeing how many times i have to filter the runs by name, get the list of one run, and do something to it, i am thinking that the manage page needs delete_run(name), activate_run(name) and edit_run(name) after all. that being said, i still think Runs should be sub-objects rather than having to wiggle the locators each time.

@klrmn klrmn and 1 other commented on an outdated diff Oct 24, 2012

pages/create_run_page.py
+ for key in kwargs.keys():
+ if not key in self._known_kwargs:
+ raise Exception("%s unrecognized, use only recognized kwargs:\n%s" %
+ (key, "\n".join(self._known_kwargs)))
+
+ if kwargs.has_key('name'):
+ name_field = self.selenium.find_element(*self._name_locator)
+ name_field.send_keys(kwargs['name'])
+ if kwargs.has_key('desc'):
+ self.selenium.find_element(*self._description_locator).send_keys(kwargs['desc'])
+ if kwargs.has_key('start_date'):
+ self.type_in_element(self._start_date_locator, kwargs['start_date'])
+ if kwargs.has_key('end_date'):
+ self.selenium.find_element(*self._end_date_locator).send_keys(kwargs['end_date'])
+
+ if kwargs.has_key('suite_list') and kwargs['suite_list']:
@klrmn

klrmn Oct 24, 2012

Contributor

i am trying to decide if this section is overly complicated (remove unwanted from include, add wanted from available, check to make sure there are no wanteds that aren't available). none of the tests actually test the removal of suites from runs.

@zacc

zacc Oct 25, 2012

Contributor

Yeah I agree it's probably not safe to assume that suites should be deselected if they're not in the included list.

@zacc

zacc Oct 25, 2012

Contributor

An API for inserting test data would be so beneficial for this project!

@teodosia teodosia and 1 other commented on an outdated diff Oct 25, 2012

pages/create_run_page.py
+ (key, "\n".join(self._known_kwargs)))
+
+ if kwargs.has_key('name'):
+ name_field = self.selenium.find_element(*self._name_locator)
+ name_field.send_keys(kwargs['name'])
+ if kwargs.has_key('desc'):
+ self.selenium.find_element(*self._description_locator).send_keys(kwargs['desc'])
+ if kwargs.has_key('start_date'):
+ self.type_in_element(self._start_date_locator, kwargs['start_date'])
+ if kwargs.has_key('end_date'):
+ self.selenium.find_element(*self._end_date_locator).send_keys(kwargs['end_date'])
+
+ if kwargs.has_key('suite_list') and kwargs['suite_list']:
+ # remove unwanted
+ for suite in self.included_suites:
+ print 'included has %s' % suite.name
@teodosia

teodosia Oct 25, 2012

Contributor

We should delete this print and the ones below.

@klrmn

klrmn Oct 25, 2012

Contributor

i think i've moved most of the prints, but i left one that was very very useful in debugging.

@AlinT AlinT commented on the diff Oct 25, 2012

pages/create_run_page.py
from pages.base_page import MozTrapBasePage
-
-class MozTrapCreateRunPage(MozTrapBasePage):
-
- _page_title = 'MozTrap'
-
+class MozTrapEditRunPage(MozTrapBasePage):
@AlinT

AlinT Oct 25, 2012

Contributor

expected 2 blank lines, found 1

@AlinT AlinT commented on an outdated diff Oct 25, 2012

pages/create_run_page.py
_submit_locator = (By.CSS_SELECTOR, '#run-add-form .form-actions > button')
+
+ def __init__(self, testsetup):
+ MozTrapBasePage.__init__(self, testsetup)
+ self.wait_for_ajax()
+
+ def fill_fields(self, **kwargs):
+ '''
+ ::keyword args::
+ name
+ desc
+ start_date
+ end_date
+ suite_list
+ '''
+
@AlinT

AlinT Oct 25, 2012

Contributor

blank line contains whitespace

@AlinT AlinT commented on an outdated diff Oct 25, 2012

pages/create_run_page.py
+ def fill_fields(self, **kwargs):
+ '''
+ ::keyword args::
+ name
+ desc
+ start_date
+ end_date
+ suite_list
+ '''
+
+ for key in kwargs.keys():
+ if not key in self._known_kwargs:
+ raise Exception("%s unrecognized, use only recognized kwargs:\n%s" %
+ (key, "\n".join(self._known_kwargs)))
+
+ if kwargs.has_key('name'):
@AlinT

AlinT Oct 25, 2012

Contributor

.has_key() is deprecated, use 'in'

@AlinT AlinT commented on an outdated diff Oct 25, 2012

pages/create_run_page.py
+ name
+ product_version
+ desc
+ start_date
+ end_date
+ suite_list
+ '''
+
+ if kwargs.has_key('product_version'):
+ product_version_select = Select(self.selenium.find_element(*self._product_version_select_locator))
+ product_version_select.select_by_visible_text(kwargs['product_version'])
+
+ MozTrapEditRunPage.fill_fields(self, **kwargs)
+
+
+ def create_run(self,
@AlinT

AlinT Oct 25, 2012

Contributor

trailing whitespace & too many blank lines

@AlinT AlinT commented on an outdated diff Oct 25, 2012

pages/base_test.py
@@ -82,8 +82,9 @@ def create_run(self, mozwebqa, activate=False, product=None, version=None, suite
if activate:
manage_runs_pg = MozTrapManageRunsPage(mozwebqa)
manage_runs_pg.filter_runs_by_name(name=run['name'])
- manage_runs_pg.activate_run(name=run['name'])
-
+ runs = manage_runs_pg.get_runs
+ runs[0].activate()
+
@AlinT

AlinT Oct 25, 2012

Contributor

blank line contains whitespace

@AlinT AlinT commented on an outdated diff Oct 25, 2012

tests/test_manage_runs_page.py
self.delete_version(mozwebqa_logged_in, version=run['version'], delete_product=True)
+
+ @pytest.mark.moztrap(2929)
+ def test_edit_existing_run_that_includes_suites(self, mozwebqa_logged_in):
+ # setup
+ product = self.create_product(mozwebqa_logged_in)
+ version = self.create_version(mozwebqa_logged_in, product=product)
+ suite1 = self.create_suite(mozwebqa_logged_in, product=product)
+ suite2 = self.create_suite(mozwebqa_logged_in, product=product)
@AlinT

AlinT Oct 25, 2012

Contributor

trailing whitespace

@AlinT AlinT commented on an outdated diff Oct 25, 2012

tests/test_manage_runs_page.py
@@ -9,6 +9,7 @@
from pages.base_test import BaseTest
from pages.manage_runs_page import MozTrapManageRunsPage
+from pages.manage_suites_page import MozTrapManageSuitesPage
@AlinT

AlinT Oct 25, 2012

Contributor

Unused import

@klrmn klrmn review feedback and drag-and-drop. fixes #73
* pep8 fixes
* don't remove presumed unwanted suites in fill_fields, provide
additional methods for adding and removing suites to/from runs.
* implement drag-and-drop (note: this uses native)
fef251a

@teodosia teodosia commented on the diff Oct 29, 2012

pages/create_run_page.py
+ @property
+ def product_version(self):
+ return self.selenium.find_element(*self._product_version_readonly_locator).text
+
+ @property
+ def included_suites(self):
+ '''this method only works if run is in draft mode.'''
+ suites = self.selenium.find_elements(*self._included_suites_locator)
+ included_suites = [self.Suite(self.testsetup, loc) for loc in suites]
+ return included_suites
+
+ @property
+ def included_suite_names(self):
+ # XXX fix this so that it returns something when run is active
+ included_suite_names = [suite.name for suite in self.included_suites]
+ print 'included_suite_names:\n%s' % included_suite_names
@teodosia

teodosia Oct 29, 2012

Contributor

We should remove this print.

@klrmn

klrmn Oct 29, 2012

Contributor

hi @teodosia
this is the print i left behind for debugging purposes (if the list is empty when you expect it to be full, you probably have an issue with FF version vs selenium version).

Contributor

AlinT commented Oct 30, 2012

The test passes and the code looks good but it takes a huge amount of time to run(~520 seconds).

@m8ttyB m8ttyB and 4 others commented on an outdated diff Oct 30, 2012

tests/test_manage_runs_page.py
+ # add suite2 to the run
+ edit_run_pg.add_suite(suite2['name'])
+ # re-order suites to run suite2 first
+ edit_run_pg.drag_and_drop_suite(suite2['name'], suite1['name'])
+ # save
+ manage_runs_pg = edit_run_pg.click_save()
+ # click edit
+ manage_runs_pg.filter_runs_by_name(name=run['name'])
+ runs = manage_runs_pg.get_runs
+ edit_run_pg = runs[0].edit()
+ expected = [suite2['name'], suite1['name']]
+ actual = edit_run_pg.included_suite_names
+ assert actual == expected
+
+ # teardown
+ self.delete_run(mozwebqa_logged_in, run)
@m8ttyB

m8ttyB Oct 30, 2012

Collaborator

If an above assertion fails the tear down won't run. Is this a problem?

@bobsilverberg

bobsilverberg Oct 30, 2012

Collaborator

@m8ttyB, @klrmn: why not use py.test's teardown instead of a simulated teardown?

@klrmn

klrmn Oct 30, 2012

Contributor

because i don't know how to do setup() and teardown() methods that apply to just one out of many of the test cases in the class, and each of the test cases has a slightly different fixture set. if someone can suggest how, i'm happy to learn it. the same thing had been bothering me.

@bobsilverberg

bobsilverberg Oct 30, 2012

Collaborator

I don't believe that you can have setup or teardown run for a single test case (or selected test cases) in a class, but I would think that just moving the test into its own file would be an option.

You could look into using a global variable to control the behaviour of teardown, but that seems overly complicated to me.

@klrmn

klrmn Oct 30, 2012

Contributor

if the not-quite-mock objects returned by the create_X methods in the base
test class knew how to delete themselves, i could push them into a
self.fixtures list and tell teardown to run each object's delete method,
but the objects rerturned by create_X are just dictionaries and don't know
how to delete themselves.

while i think this would be a good problem to solve, i'm not sure it's a
good idea to try to solve it within the scope of this PR. unless i am
mistaken, all of the test cases in this project have the same issue, it's
just not quite as obvious because they are one-line unmarked teardown
rather than a whole section.

On Tue, Oct 30, 2012 at 9:30 AM, Bob Silverberg notifications@github.comwrote:

In tests/test_manage_runs_page.py:

  •    # add suite2 to the run
    
  •    edit_run_pg.add_suite(suite2['name'])
    
  •    # re-order suites to run suite2 first
    
  •    edit_run_pg.drag_and_drop_suite(suite2['name'], suite1['name'])
    
  •    # save
    
  •    manage_runs_pg = edit_run_pg.click_save()
    
  •    # click edit
    
  •    manage_runs_pg.filter_runs_by_name(name=run['name'])
    
  •    runs = manage_runs_pg.get_runs
    
  •    edit_run_pg = runs[0].edit()
    
  •    expected = [suite2['name'], suite1['name']]
    
  •    actual = edit_run_pg.included_suite_names
    
  •    assert actual == expected
    
  •    # teardown
    
  •    self.delete_run(mozwebqa_logged_in, run)
    

I don't believe that you can have setup or teardown run for a single test
case (or selected test cases) in a class, but I would think that just
moving the test into its own file would be an option.

You could look into using a global variable to control the behaviour of
teardown, but that seems overly complicated to me.


Reply to this email directly or view it on GitHubhttps://github.com/mozilla/moztrap-tests/pull/80/files#r1980850.

@m8ttyB

m8ttyB Oct 30, 2012

Collaborator

@klrmn good point -- I didn't realize the other tests for this product followed the same convention. I withdraw my criticism and will put it into a git issue :-)

@bebef1987

bebef1987 Oct 31, 2012

Contributor

don't know if this is the case but can't we use something like http://pytest.org/2.2.4/funcargs.html#_pytest.python.FuncargRequest.cached_setup
here?
I never used it but from the docs it looks like it would fit in our tests

@zacc

zacc Oct 31, 2012

Contributor

I have tried to do setup and teardown in py.test as we have a similar problem in flightdeck-tests. It's the one thing I really miss from junit/testng.

I managed to get a setup/teardown but couldn't get the mozwebqa/selenium session into it.

At the moment we just have to deal with it I think

@klrmn

klrmn Oct 31, 2012

Contributor

yeah...if we have the api, then we don't need mozwebqa in the setup/teardown, but it is otherwise problematic.

Contributor

klrmn commented Oct 30, 2012

@AlinT , yes, it does take forever to run, because of the setup and teardown being done via selenium rather than via an API that doesn't exist yet. I requested the API tho =).

Contributor

bebef1987 commented Oct 31, 2012

http://pastebin.mozilla.org/1894532

this takes too long to run...
and we really need a API for this site...
Can we talk with the devs to maybe check and see is there is something like this in place...

Contributor

bebef1987 commented Oct 31, 2012

@jgerigmeyer @rbillings @camd @carljm do we have a API for adding tests suites runs ?

@bebef1987 bebef1987 referenced this pull request Oct 31, 2012

Closed

Issue 74 #82

@zacc zacc and 1 other commented on an outdated diff Oct 31, 2012

tests/test_manage_runs_page.py
+ assert suite1['name'] in edit_run_pg.included_suite_names
+ assert suite2['name'] in edit_run_pg.available_suite_names
+
+ # add suite2 to the run
+ edit_run_pg.add_suite(suite2['name'])
+ # re-order suites to run suite2 first
+ edit_run_pg.drag_and_drop_suite(suite2['name'], suite1['name'])
+ # save
+ manage_runs_pg = edit_run_pg.click_save()
+ # click edit
+ manage_runs_pg.filter_runs_by_name(name=run['name'])
+ runs = manage_runs_pg.get_runs
+ edit_run_pg = runs[0].edit()
+ expected = [suite2['name'], suite1['name']]
+ actual = edit_run_pg.included_suite_names
+ assert actual == expected
@zacc

zacc Oct 31, 2012

Contributor

Any reason not using unittest-zero in this test?

@klrmn

klrmn Oct 31, 2012

Contributor

because i got out of the habit =)

Contributor

camd commented Oct 31, 2012

Bebe-- Unfortunately, we don't quite have this yet. Though much of the plumbing is there to make it possible. And I think this is definitely needed (klrmn had asked me for this earlier as well.)

I created a tracker story for this: https://www.pivotaltracker.com/story/show/38741433

Would you help clarify the needs here? I know creating test cases, suites and tags is what you're asking for here. Is there more? Could you prioritize which parts are more important than others?

Just as a heads up, there is some complex logic in some areas (such as activating a test run to be executed) which would be more effort in creating an api. But for just creating suites, test cases and tags, that shouldn't be bad. I'll see if I can do at least that in the short term.

If you would comment on that story in tracker, that'd be ideal.

Thanks,
-Cam

On Oct 31, 2012, at 3:33 AM, Florin Strugariu notifications@github.com wrote:

@jgerigmeyer @rbillings @camd @carljm do we have a API for adding tests suites runs ?


Reply to this email directly or view it on GitHub.

Contributor

AlinT commented Nov 1, 2012

The code looks good and the tests pass on Mac+FF14 but fail on Win7+FF14
I suspect the locators need to be strengthen, but any idea is welcomed!

Contributor

klrmn commented Nov 1, 2012

could you specify where it failed? i have run into a couple of cases where
a timeout of 60 was not enough for wait_for_ajax().

On Thu, Nov 1, 2012 at 7:53 AM, Trif Andrei-Alin
notifications@github.comwrote:

The code looks good and the tests pass on Mac+FF14 but fail on Win7+FF14
I suspect the locators need to be strengthen, but any idea is welcomed!


Reply to this email directly or view it on GitHubhttps://github.com/mozilla/moztrap-tests/pull/80#issuecomment-9982741.

Contributor

AlinT commented Nov 2, 2012

@klrmn Ran this again with Webdriver 2.26.0 on Win7 + FF16&15, and Webdriver 2.25.0 on Win7 + FF 14.
On all cases the test failed with the same log http://pastebin.mozilla.org/1902059.
Hope this helps you, and I'd really appreciate if other people would run this on Windows platforms and post a comment with their findings/conclusions so that we can rule out the problem of the environment.
Thanks in advance!

Contributor

klrmn commented Nov 2, 2012

@AlinT

This particular failure means that drag-and-drop isn't working. So far as i know, this is the first mozwebqa repo to try to use drag-and-drop.

@AutomatedTester, after you suggested i had a version mismatch, i got the drag and drop to work on:
Mac+FF13+Selenium 2.25,
Mac+FF15+Selenium 2.25,
XP+FF15+Selenium 2.25
(probably some other combos)

but @AlinT is seeing failures listed above.

any ideas? i would suggest sending it to sauce so we can watch a recording, but Win7 doesn't appear to be an option there. (I think the selenium tests are being run on sauce, so this might mean it's not being tested before selenium release?)

-Leah

Contributor

bebef1987 commented Nov 20, 2012

(moztrap)florinstrugariu@P4298:~/work/moztrap-tests-bebe$ py.test --baseurl=http://moztrap-dev.allizom.org/ --browserver=16 --browsername=firefox --platform=mac --credentials=/home/florinstrugariu/work/webqa-credentials/moztrap.yaml --destructive --host=webqa.local tests/test_manage_runs_page.py
================================================================================================ test session starts ================================================================================================
platform linux2 -- Python 2.7.3 -- pytest-2.2.4
collected 2 items

tests/test_manage_runs_page.py X.

====================================================================================== 1 passed, 1 xpassed in 1073.96 seconds =======================================================================================

Contributor

bebef1987 commented Nov 20, 2012

this looks OK but... this is not a test that could be merged...
2 test in 1073 (17 min) jenkins will throw it out without looking at it...

In my opinion without the API to create test data fast this task is blocked

Contributor

camd commented Nov 20, 2012

Hey there. We are working to prioritize MozTrap work and evaluating whether we can get these APIs added in now or need to wait till after the new year due to higher priorities. We'll keep you posted. Sorry for your pain, but thanks for your hard work!

Contributor

bebef1987 commented Nov 20, 2012

Thanks for the info @camd let's hope we get a cool api for Christmas Ninja cat was a really good cat 😄

@m8ttyB should we merge this?

Collaborator

m8ttyB commented Nov 21, 2012

I need some time to understand what's going on, what's being tested, and if there is an alternate path we can take to setting up and running the test -- let's take the discussion to the mailing list. I'm hopeful we can find a sneaky way to get this test automated without an API.

As @camd noted we're really not likely to get an API for some time unless of course someone from the community wants to reach out to @camd for mentorship and give it shot themselves (which would be quite cool).

Collaborator

m8ttyB commented Nov 21, 2012

Just for reference here is the testcase that is being automated -- correct me if I'm wrong :)

https://moztrap.mozilla.org/manage/cases/?filter-id=2929#caseversion-id-10687

And because I love tiny #squee goats

Contributor

sashakruglov commented Jan 19, 2013

It is possible to reduce time for test cases creation by using bulk create.

Collaborator

m8ttyB commented Jan 19, 2013

Hi @sashakruglov I don't think I understand what your question is? May I suggest you start a thread on our mailing list so the team can respond?

mozwebqa@mozilla.org

Collaborator

m8ttyB commented Jan 19, 2013

@klrmn should we close this pull request and then reissue it once an API is in place? This pull has been open for quite some time.

Contributor

sashakruglov commented Jan 20, 2013

@m8ttyB I was talking about this part of test https://github.com/mozilla/moztrap-tests/pull/80/files#L4R41 and was trying to say that it's possible to create 4 test cases at once by bulk create option, so it will reduce execution time of test for maybe a minute or a little less than a minute.

Another option is to use direct POST requests to create test items.
I tried and it works, there are some difficulties though:

  1. it requires sessionid and maybe anoncsrf token (so, it will require log in action in browser to obtain session)
  2. it requires another csrf token (which can be obtained by driver.find_element(By.NAME, 'csrfmiddlewaretoken').get_attribute('value')
  3. and most difficult one is that some parameters that are sent to server are IDs of items (like ID of profile).

We can try this approach, I think we can win some time by it. What do you think?

Also, I might look into creating requested API, but I'm not so sure that I'm capable enough to implement it, but I would like to give it a shot.

Collaborator

bobsilverberg commented Feb 5, 2013

Can we either resolve the merge conflicts on this or close it?

Collaborator

bobsilverberg commented Feb 5, 2013

Closing due to age. Yes, when it comes to pull requests I am ageist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment