-
Notifications
You must be signed in to change notification settings - Fork 44
Added methods to sign in and out of django admin panel, as well as toggle waffle flags Everyone attribute #90
Conversation
_django_login_button_locator = (By.CSS_SELECTOR, 'input.grp-button.grp-default') | ||
_django_signed_in_locator = (By.CSS_SELECTOR, '#grp-admin-title') | ||
_waffle_module_locator = (By.CSS_SELECTOR, '#module-waffle') | ||
_change_waffle_flag_locator = (By.CSS_SELECTOR, '#module-waffle li:nth-of-type(2) a') |
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 locator returns a list of elements.
Maybe use something like this:
_change_waffle_flag_locator = (By.CSS_SELECTOR, '#module-waffle .grp-change-link a[href*="flag"]')
I think it would be cleaner to move this code to a different file, one that handles all admin controls. It would be easier to manage if we want to add additional methods. @stephendonner It this kind of functionality safe? Here we are accessing the admin controls so if anything fails we might affect the behaviour of the site. Are we sure we want to do this? |
Thanks @chirarobert , while we wait for others I'll look into some other options! I agree that it definitely is unsafe if any of the methods fail. I also realized I left my import of time by accident! |
@dchunwong please move anything related to the django admin to a separate file. For tests that depend on a waffle flag being set, I suggest we have check for success on setting the flag before we proceed with the test else return a quick failure. |
For the separate file @retornam would it be appropriate to treat it like page? i.e. it subclasses BasePage and in tests someone could create a django_admin object passing in mozwebqa and use its methods to sign in etc. as set up and teardown. |
Added methods to sign in and out of django admin panel, as well as toggle flags to 'Yes', 'No', 'Unknown'
I've updated it such that it is like my previous comment. one would instantiate a DjangoAdmin object by passing in mozwebqa just like a page and then using its methods to change settings. I also added a check to see that flags are successfully changed as well as maximize the screen when checking for the flag. |
You all are way past my selenium testing skills, but I'm happy to cheer for this pull request. It's a great idea and technique that will likely be used by other Mozilla websites in the future. |
r+ |
I can't comment on the risk from a technical standpoint, but I like the idea and the ability it gives us -- will/could this be flexible enough of an approach to use on other sites, as well, as @groovecoder suggests? If so, and we can mitigate the safety risks, then perhaps it's worth merging and playing with. |
I'm exploring getting the waffle flags to toggle on setup and teardown on a class level so that it'll toggle flags, run tests, then untoggle flags. The benefit of this is if it fails to do so no other tests that require it would be run. What I've figured out so far is to make a new fixture in conftest.py which could be passed in to the functions that require the django admin settings to be toggled. I'll get an example up when I can. |
Not sure what to say on this, my attempts to incorporate the methods into a setup and teardown were mostly unfruitful due to a socket connection refused error on teardown, which may explain why the oauth_flag is set to No right now and why tests have been passing. I suppose the best solution would be to wait for oauth login to be finished and refactor the tests for that. |
@dchunwong If you want to share a branch with your work I can take a look. I have some experience using fixtures in conftest.py. |
Thanks @bobsilverberg , my current work can be found at https://github.com/dchunwong/mdn-tests/tree/togglewaffle-testing The changed files are conftest.py, pages/django_admin.py, and tests/test_user_account.py. It's currently setup that django_admin will return True when successfully changing the flags. It performs setup on the class scope by logging and and toggle the proper flag. Then in the fin() block it should simply login again and untoggle the flag, but I currently get a connection refused error. |
@bobsilverberg @dchunwong Any updates for this PR? |
Sorry not right now, :( I believe that tests are passing currently as the flag is just currently set to 'No'. |
Closing issue for now. |
Right now I've added a method that will take in a waffle flag name and the setting it is to be toggled to ('Yes', 'No', 'Unknown') It will log into the admin panel, change the flag, then log out. This is in relation to #86 and #87. Would this be sufficient? Any suggestions would be helpful. I know I added a lot of locators to the base file...but I've tested it with oauth_login flag and it consistently works.