Issue #87. Test for MozTrap #5935 #94

Merged
merged 6 commits into from Jan 18, 2013

Projects

None yet

5 participants

@sashakruglov
Contributor
@AndreiH
Contributor
AndreiH commented Jan 14, 2013

Lgtm 👍

@sashakruglov sashakruglov commented on the diff Jan 14, 2013
pages/manage_runs_page.py
@@ -5,6 +5,7 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
from selenium.webdriver.common.by import By
+from selenium.webdriver.support.color import Color
@sashakruglov
sashakruglov Jan 14, 2013 Contributor

warning! that is relatively new feature in webdriver, so it will cause ImportError with obsolete versions of selenium

@AlinT
AlinT Jan 15, 2013 Contributor

I don't think we are going to have this problem, due to the fact that we are using latest selenium version(2.28.0)

@AlinT
Contributor
AlinT commented Jan 15, 2013

Looks good to me!

@sashakruglov sashakruglov commented on an outdated diff Jan 15, 2013
tests/test_pinning_filters.py
+ @pytest.mark.nondestructive
+ def test_that_pinned_product_version_is_defaulted_on_test_run_creation(self, mozwebqa_logged_in):
+ product = self.create_product(mozwebqa_logged_in)
+ product_version = u'%s %s' % (product['name'], product['version']['name'])
+
+ manage_runs_pg = MozTrapManageRunsPage(mozwebqa_logged_in)
+ manage_runs_pg.go_to_manage_runs_page()
+ manage_runs_pg.filter_runs_by_product_version(product_version)
+ manage_runs_pg.pin_product_version()
+
+ Assert.equal(
+ manage_runs_pg.pinned_filter_color.upper(),
+ u'#DFB081')
+
+ create_run_pg = MozTrapCreateRunPage(mozwebqa_logged_in)
+ create_run_pg.go_to_create_run_page()
@sashakruglov
sashakruglov Jan 15, 2013 Contributor

will change this part to make test closer to steps in test case.
click create run button and go_to_create_run_page() doesn't click button, but opens page by direct URL

@sashakruglov
Contributor

so, would you merge it into master? i would like to write some more tests for pinning filters and i need to know to what branch commit.

@sashakruglov
Contributor

@AlinT , @AndreiH what do you think about moving filters into separate module?
I think I managed to migrate without braking any tests - https://github.com/sashakruglov/moztrap-tests/tree/filter_refactoring.

@AndreiH
Contributor
AndreiH commented Jan 16, 2013

Looked over the code. It should be more organized that way, but are you sure the tests weren't altered because "test_that_user_can_filter_product_by_name_without_mouse" is failing for me. What do you think @AlinT ?

@sashakruglov
Contributor

@AndreiH will look into it.

@AlinT
Contributor
AlinT commented Jan 16, 2013

@sashakruglov the reorganization is the best practice here.
However in branch filter_refactoring you have some remaining code from pull #93 , in several files:
https://github.com/sashakruglov/moztrap-tests/blob/filter_refactoring/pages/base_test.py#L21
My suggestion is wait until we merge pull #93 (this won't take much longer) and then submit a new pull for code review with filter_refactoring.

@sashakruglov
Contributor

@AlinT it's true, i checked out this branch from the other one, not from master. it would be better to merge two my previous pull requests, so i can resolve all conflicts and update this pull request. apart from that, do you have any other suggestions?

@AlinT
Contributor
AlinT commented Jan 16, 2013

@sashakruglov I don't have any suggestions for now, I'll wait until you have a pull request up for this :)

@sashakruglov
Contributor

@AndreiH could you please provide traceback and in what browser this test fails? I guess that is not firefox?

@sashakruglov
Contributor

@AndreiH bunch of tests are failing for me in chomium (on ubuntu).
so problem existed before, will try to fix at least issue with filters in chromium.

well, to be honest, there are so issues introduced by me into filters, and i think i've resolved them.

@AndreiH
Contributor
AndreiH commented Jan 17, 2013

We should investigate further after we merge this one and you request a pull for that. Please fix merge conflicts.

@sashakruglov
Contributor

@AndreiH fixed merge conflict

@AlinT
Contributor
AlinT commented Jan 18, 2013

LGTM

@teodosia
Contributor

Lgtm too.

@m8ttyB
Collaborator
m8ttyB commented Jan 18, 2013

thx @sashakruglov looks good to me. @AlinT and @teodosia thanks for your excellent reviews.

One nit for future pulls please run a pep8 checker against the code. pep8 flaged a few issues in this pull request but I'll let them slip through so we can get this coverage.

Merged
pew_pew_pew

@m8ttyB m8ttyB merged commit d91483b into mozilla:master Jan 18, 2013
@sashakruglov sashakruglov deleted the sashakruglov:moztrap_5935 branch Jan 18, 2013
@sashakruglov
Contributor

@m8ttyB thanks. Could you recommend some pep8 checker?

@m8ttyB
Collaborator
m8ttyB commented Jan 18, 2013

@sashakruglov more than happy to -- it depends on which env you're using and which setup feels best to you.
One option is to install pep8 with pip and then run the checker directly from the command-line:
pip install pep8
pep8 name_of_file_or_directory

A second option is to setup a pre-commit hook for pep8:
https://wiki.mozilla.org/QA/Execution/Web_Testing/Automation/PEP8_Pre_Commit_Hook

If you have any questions stop into our #mozwebqa irc channel and we'll help you sort it out.

@sashakruglov
Contributor

@m8ttyB great! I'll try pep8.

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