Jonahstanley/fix alert tests #77
Conversation
This is done with the following steps: 'I confirm all alerts' means that all alert and confirm windows are returned and returned true respectively 'I dismiss all alerts' means that all confirm windows are returned false 'I answer all prompts with "([^"]*)"' means that all prompts are returned with the given string Please note that these settings are on a PER PAGE basis. This means that for best results, the step must be given right before the alert is generated.
There was a problem hiding this comment.
Looks like some indenting issues. Perhaps the file needs to be reindented to remove tabs?
There was a problem hiding this comment.
Thats weird... the file looks fine on my local version and git is telling me that I don't have any more changes to commit so I am not sure how to fix that
There was a problem hiding this comment.
Have you verified the file does not have a mix of tabs and spaces? We don't want tabs.
There was a problem hiding this comment.
For some reason it loaded as tabs. Changing that now and recommitting
|
It's a little awkward to read, with the confirming happening before you actually do the action that prompts the alert. Perhaps change the steps to read "I will confirm all alerts"? |
|
👍 (thought tests have not finished running). Note that I did NOT check out the branch and try running the tests myself. |
common/djangoapps/terrain/steps.py
Outdated
There was a problem hiding this comment.
Since the wording of this is necessarily a bit awkward because of the way it needs to be used, can you please include docstrings for these three "I will" methods? Something about how you need to put the statements about the behavior wanted when dismissing alerts before the statement that will trigger an alert to be popped up?
|
Please include the in-code documentation noted above, then 👍 |
|
👍 Looks good to me |
Jonahstanley/fix alert tests
…eapiview-fix Mattdrayer/api secureapiview fix Tested - seemed to fix the right stuff for us!
Release Candidate rc/2016-03-22
(cherry picked from commit 7fc92dd)
Remove code added for Debugging purpose Approved-by: nyein.eizaw
Suggested Reviewers @wedaly @cahrens @jzoldak
In this pull request, I have fixed the issue of chrome hanging on alerts.
This was done by changing how alerts work on the javascript level:
Alerts are created through window.alert(), window.confirm(), window.prompt()
These methods have some functionality that creates the popup box and upon the user clicking/typing either return (alert), return true or false (confirm) or return with a string (prompt)
Selenium can inject javascript through world.browser.execute_script()
Now, there are steps to cut out the middleman so to speak.
"I confirm all alerts" will redefine
window.alert = function(){return;}
window.confirm = function(){return true;}
"I dismiss all alerts" will redefine
window.confirm = function(){return false;}
'I answer all prompts with "([^"]*)"' will redefine
window.prompt = function(){return %s} %prompt
The window variable only persists on a given page so changing these functions will only affect the page they are on. While this does make it safer to modify these functions, this also means that these steps must be called right before the alert is generated.
In following this, I have also fixed the features that required this functionality and they all now work on my local machine.