From 2ff056df8cf5750846b1ad4fd9b38f89f20ce6f3 Mon Sep 17 00:00:00 2001
From: Jay Zoldak <zoldak@edx.org>
Date: Wed, 4 Sep 2013 09:28:30 -0400
Subject: [PATCH 1/2] Simplify retry logic for ui helper functions

---
 .../features/advanced-settings.py             |   4 +-
 .../contentstore/features/common.py           |  60 ++++--
 .../component_settings_editor_helpers.py      |   4 +-
 .../contentstore/features/course-settings.py  |   2 +-
 .../contentstore/features/course-team.py      |  10 +-
 .../contentstore/features/course-updates.py   |  18 +-
 .../contentstore/features/grading.py          |  17 +-
 .../contentstore/features/problem-editor.py   |  10 +-
 .../contentstore/features/signup.py           |   2 +-
 .../contentstore/features/static-pages.py     |  32 ++-
 .../contentstore/features/upload.py           |  30 ++-
 common/djangoapps/terrain/browser.py          |   4 +-
 common/djangoapps/terrain/ui_helpers.py       | 199 ++++++++++--------
 .../courseware/features/navigation.py         |   6 +-
 .../courseware/features/problems.feature      |   2 +-
 .../courseware/features/problems.py           |   3 +
 lms/djangoapps/courseware/features/signup.py  |   5 +-
 17 files changed, 245 insertions(+), 163 deletions(-)

diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py
index 201ac49e52..e202031a01 100644
--- a/cms/djangoapps/contentstore/features/advanced-settings.py
+++ b/cms/djangoapps/contentstore/features/advanced-settings.py
@@ -93,7 +93,9 @@ def assert_policy_entries(expected_keys, expected_values):
     for key, value in zip(expected_keys, expected_values):
         index = get_index_of(key)
         assert_false(index == -1, "Could not find key: {key}".format(key=key))
-        assert_equal(value, world.css_find(VALUE_CSS)[index].value, "value is incorrect")
+        found_value = world.css_find(VALUE_CSS)[index].value
+        assert_equal(value, found_value,
+            "{} is not equal to {}".format(value, found_value))
 
 
 def get_index_of(expected_key):
diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py
index c5405b1951..74bab10a24 100644
--- a/cms/djangoapps/contentstore/features/common.py
+++ b/cms/djangoapps/contentstore/features/common.py
@@ -2,7 +2,7 @@
 # pylint: disable=W0621
 
 from lettuce import world, step
-from nose.tools import assert_true  # pylint: disable=E0611
+from nose.tools import assert_true, assert_in, assert_false  # pylint: disable=E0611
 
 from auth.authz import get_user_by_email, get_course_groupname_for_role
 from django.conf import settings
@@ -19,8 +19,6 @@ from terrain.browser import reset_data
 
 TEST_ROOT = settings.COMMON_TEST_DATA_ROOT
 
-###########  STEP HELPERS ##############
-
 
 @step('I (?:visit|access|open) the Studio homepage$')
 def i_visit_the_studio_homepage(_step):
@@ -66,20 +64,32 @@ def select_new_course(_step, whom):
 
 @step(u'I press the "([^"]*)" notification button$')
 def press_the_notification_button(_step, name):
-    css = 'a.action-%s' % name.lower()
+    # TODO: fix up this code. Selenium is not dealing well with css transforms,
+    # as it thinks that the notification and the buttons are always visible
+
+    # First wait for the notification to pop up
+    notification_css = 'div#page-notification div.wrapper-notification'
+    world.wait_for_visible(notification_css)
+
+    # You would think that the above would have worked, but it doesn't.
+    # Brute force wait for now.
+    world.wait(.5)
+
+    # Now make sure the button is there
+    btn_css = 'div#page-notification a.action-%s' % name.lower()
+    world.wait_for_visible(btn_css)
+
+    # You would think that the above would have worked, but it doesn't.
+    # Brute force wait for now.
+    world.wait(.5)
 
-    # The button was clicked if either the notification bar is gone,
-    # or we see an error overlaying it (expected for invalid inputs).
-    def button_clicked():
-        confirmation_dismissed = world.is_css_not_present('.is-shown.wrapper-notification-warning')
-        error_showing = world.is_css_present('.is-shown.wrapper-notification-error')
-        return confirmation_dismissed or error_showing
     if world.is_firefox():
-        # This is done to explicitly make the changes save on firefox.  It will remove focus from the previously focused element
-        world.trigger_event(css, event='focus')
-        world.browser.execute_script("$('{}').click()".format(css))
+        # This is done to explicitly make the changes save on firefox.
+        # It will remove focus from the previously focused element
+        world.trigger_event(btn_css, event='focus')
+        world.browser.execute_script("$('{}').click()".format(btn_css))
     else:
-        world.css_click(css, success_condition=button_clicked), '%s button not clicked after 5 attempts.' % name
+        world.css_click(btn_css)
 
 
 @step('I change the "(.*)" field to "(.*)"$')
@@ -110,7 +120,6 @@ def i_see_a_confirmation(step):
     assert world.is_css_present(confirmation_css)
 
 
-####### HELPER FUNCTIONS ##############
 def open_new_course():
     world.clear_courses()
     create_studio_user()
@@ -156,8 +165,8 @@ def log_into_studio(
     world.log_in(username=uname, password=password, email=email, name=name)
     # Navigate to the studio dashboard
     world.visit('/')
+    assert_in(uname, world.css_text('h2.title', timeout=60))
 
-    assert uname in world.css_text('h2.title', max_attempts=15)
 
 def create_a_course():
     course = world.CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
@@ -247,8 +256,22 @@ def button_disabled(step, value):
 
 @step('I confirm the prompt')
 def confirm_the_prompt(step):
-    prompt_css = 'a.button.action-primary'
-    world.css_click(prompt_css, success_condition=lambda: not world.css_visible(prompt_css))
+
+    def click_button(btn_css):
+        world.css_click(btn_css)
+        return world.css_find(btn_css).visible == False
+
+    prompt_css = 'div.prompt.has-actions'
+    world.wait_for_visible(prompt_css)
+
+    btn_css = 'a.button.action-primary'
+    world.wait_for_visible(btn_css)
+
+    # Sometimes you can do a click before the prompt is up.
+    # Thus we need some retry logic here.
+    world.wait_for(lambda _driver: click_button(btn_css))
+
+    assert_false(world.css_find(btn_css).visible)
 
 
 @step(u'I am shown a (.*)$')
@@ -257,6 +280,7 @@ def i_am_shown_a_notification(step, notification_type):
 
 
 def type_in_codemirror(index, text):
+    world.wait(1)  # For now, slow this down so that it works. TODO: fix it.
     world.css_click("div.CodeMirror-lines", index=index)
     world.browser.execute_script("$('div.CodeMirror.CodeMirror-focused > div').css('overflow', '')")
     g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea")
diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
index 2971085081..4656b7f29d 100644
--- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
+++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
@@ -48,9 +48,7 @@ def click_component_from_menu(category, boilerplate, expected_css):
         elem_css = "a[data-category='{}']:not([data-boilerplate])".format(category)
     elements = world.css_find(elem_css)
     assert_equal(len(elements), 1)
-    world.wait_for(lambda _driver: world.css_visible(elem_css))
-    world.css_click(elem_css, success_condition=lambda: 1 == len(world.css_find(expected_css)))
-
+    world.css_click(elem_css)
 
 @world.absorb
 def edit_component_and_select_settings():
diff --git a/cms/djangoapps/contentstore/features/course-settings.py b/cms/djangoapps/contentstore/features/course-settings.py
index 7004b9f99e..5f026146b4 100644
--- a/cms/djangoapps/contentstore/features/course-settings.py
+++ b/cms/djangoapps/contentstore/features/course-settings.py
@@ -113,7 +113,7 @@ def test_i_have_entered_a_new_course_start_date(step):
 
 @step('The warning about course start date goes away$')
 def test_the_warning_about_course_start_date_goes_away(step):
-    assert_equal(0, len(world.css_find('.message-error')))
+    assert world.is_css_not_present('.message-error')
     assert_false('error' in world.css_find(COURSE_START_DATE_CSS).first._element.get_attribute('class'))
     assert_false('error' in world.css_find(COURSE_START_TIME_CSS).first._element.get_attribute('class'))
 
diff --git a/cms/djangoapps/contentstore/features/course-team.py b/cms/djangoapps/contentstore/features/course-team.py
index 85044dbbad..763c116a56 100644
--- a/cms/djangoapps/contentstore/features/course-team.py
+++ b/cms/djangoapps/contentstore/features/course-team.py
@@ -5,7 +5,7 @@ from lettuce import world, step
 from common import create_studio_user
 from django.contrib.auth.models import Group
 from auth.authz import get_course_groupname_for_role, get_user_by_email
-from nose.tools import assert_true  # pylint: disable=E0611
+from nose.tools import assert_true, assert_in  # pylint: disable=E0611
 
 PASSWORD = 'test'
 EMAIL_EXTENSION = '@edx.org'
@@ -112,12 +112,12 @@ def other_user_login(_step, name):
 @step(u's?he does( not)? see the course on (his|her) page')
 def see_course(_step, inverted, gender='self'):
     class_css = 'h3.course-title'
-    all_courses = world.css_find(class_css, wait_time=1)
-    all_names = [item.html for item in all_courses]
     if inverted:
-        assert not world.scenario_dict['COURSE'].display_name in all_names
+        assert world.is_css_not_present(class_css)
     else:
-        assert world.scenario_dict['COURSE'].display_name in all_names
+        all_courses = world.css_find(class_css)
+        all_names = [item.html for item in all_courses]
+        assert_in(world.scenario_dict['COURSE'].display_name, all_names)
 
 
 @step(u'"([^"]*)" should( not)? be marked as an admin')
diff --git a/cms/djangoapps/contentstore/features/course-updates.py b/cms/djangoapps/contentstore/features/course-updates.py
index 3278805a48..da74f5aa4b 100644
--- a/cms/djangoapps/contentstore/features/course-updates.py
+++ b/cms/djangoapps/contentstore/features/course-updates.py
@@ -4,6 +4,7 @@
 from lettuce import world, step
 from selenium.webdriver.common.keys import Keys
 from common import type_in_codemirror
+from nose.tools import assert_in  # pylint: disable=E0611
 
 
 @step(u'I go to the course updates page')
@@ -21,14 +22,17 @@ def add_update(_step, text):
     change_text(text)
 
 
-@step(u'I should( not)? see the update "([^"]*)"$')
-def check_update(_step, doesnt_see_update, text):
+@step(u'I should see the update "([^"]*)"$')
+def check_update(_step, text):
     update_css = 'div.update-contents'
-    update = world.css_find(update_css, wait_time=1)
-    if doesnt_see_update:
-        assert len(update) == 0 or not text in update.html
-    else:
-        assert text in update.html
+    update_html = world.css_find(update_css).html
+    assert_in(text, update_html)
+
+
+@step(u'I should not see the update "([^"]*)"$')
+def check_no_update(_step, text):
+    update_css = 'div.update-contents'
+    assert world.is_css_not_present(update_css)
 
 
 @step(u'I modify the text to "([^"]*)"$')
diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py
index 93e44b3893..6f702b8a5d 100644
--- a/cms/djangoapps/contentstore/features/grading.py
+++ b/cms/djangoapps/contentstore/features/grading.py
@@ -5,6 +5,7 @@ from lettuce import world, step
 from common import *
 from terrain.steps import reload_the_page
 from selenium.common.exceptions import InvalidElementStateException
+from nose.tools import assert_in, assert_not_in  # pylint: disable=E0611
 
 
 @step(u'I am viewing the grading settings')
@@ -65,21 +66,25 @@ def change_assignment_name(step, old_name, new_name):
 
 @step(u'I go back to the main course page')
 def main_course_page(step):
-    main_page_link_css = 'a[href="/%s/%s/course/%s"]' % (world.scenario_dict['COURSE'].org,
-                                                        world.scenario_dict['COURSE'].number,
-                                                        world.scenario_dict['COURSE'].display_name.replace(' ', '_'),)
-    world.css_click(main_page_link_css)
+    main_page_link = '/{}/{}/course/{}'.format(world.scenario_dict['COURSE'].org,
+                                               world.scenario_dict['COURSE'].number,
+                                               world.scenario_dict['COURSE'].display_name.replace(' ', '_'),)
+    world.visit(main_page_link)
+    assert_in('Course Outline', world.css_text('h1.page-header'))
 
 
 @step(u'I do( not)? see the assignment name "([^"]*)"$')
 def see_assignment_name(step, do_not, name):
     assignment_menu_css = 'ul.menu > li > a'
+    # First assert that it is there, make take a bit to redraw
+    assert world.css_find(assignment_menu_css)
+
     assignment_menu = world.css_find(assignment_menu_css)
     allnames = [item.html for item in assignment_menu]
     if do_not:
-        assert not name in allnames
+        assert_not_in (name, allnames)
     else:
-        assert name in allnames
+        assert_in (name, allnames)
 
 
 @step(u'I delete the assignment type "([^"]*)"$')
diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py
index 5e4fe6364d..414b2dea80 100644
--- a/cms/djangoapps/contentstore/features/problem-editor.py
+++ b/cms/djangoapps/contentstore/features/problem-editor.py
@@ -197,9 +197,15 @@ def high_level_source_in_editor(step):
 
 
 def verify_high_level_source_links(step, visible):
-    assert_equal(visible, world.is_css_present('.launch-latex-compiler'))
+    if visible:
+        assert world.is_css_present('.launch-latex-compiler')
+    else:
+        assert world.is_css_not_present('.launch-latex-compiler')
     world.cancel_component(step)
-    assert_equal(visible, world.is_css_present('.upload-button'))
+    if visible:
+        assert world.is_css_present('.upload-button')
+    else:
+        assert world.is_css_not_present('.upload-button')
 
 
 def verify_modified_weight():
diff --git a/cms/djangoapps/contentstore/features/signup.py b/cms/djangoapps/contentstore/features/signup.py
index 94c6e6f18e..ee8950bac2 100644
--- a/cms/djangoapps/contentstore/features/signup.py
+++ b/cms/djangoapps/contentstore/features/signup.py
@@ -12,7 +12,7 @@ def i_fill_in_the_registration_form(step):
         register_form.find_by_name('password').fill('test')
         register_form.find_by_name('username').fill('robot-studio')
         register_form.find_by_name('name').fill('Robot Studio')
-        register_form.find_by_name('terms_of_service').check()
+        register_form.find_by_name('terms_of_service').click()
     world.retry_on_exception(fill_in_reg_form)
 
 
diff --git a/cms/djangoapps/contentstore/features/static-pages.py b/cms/djangoapps/contentstore/features/static-pages.py
index 66649a116a..3e4054b9ad 100644
--- a/cms/djangoapps/contentstore/features/static-pages.py
+++ b/cms/djangoapps/contentstore/features/static-pages.py
@@ -2,12 +2,11 @@
 #pylint: disable=W0621
 
 from lettuce import world, step
-from selenium.webdriver.common.keys import Keys
 from nose.tools import assert_true # pylint: disable=E0611
 
 
 @step(u'I go to the static pages page')
-def go_to_static(_step):
+def go_to_static(step):
     menu_css = 'li.nav-course-courseware'
     static_css = 'li.nav-course-courseware-pages a'
     world.css_click(menu_css)
@@ -15,25 +14,37 @@ def go_to_static(_step):
 
 
 @step(u'I add a new page')
-def add_page(_step):
+def add_page(step):
     button_css = 'a.new-button'
     world.css_click(button_css)
 
 
-@step(u'I should( not)? see a "([^"]*)" static page$')
-def see_page(_step, doesnt, page):
+@step(u'I should not see a "([^"]*)" static page$')
+def not_see_page(step, page):
+    # Either there are no pages, or there are pages but
+    # not the one I expect not to exist.
 
-    should_exist = not doesnt
+    # Since our only test for deletion right now deletes
+    # the only static page that existed, our success criteria
+    # will be that there are no static pages.
+    # In the future we can refactor if necessary.
+    tabs_css = 'li.component'
+    assert (world.is_css_not_present(tabs_css, wait_time=30))
+
+
+@step(u'I should see a "([^"]*)" static page$')
+def see_page(step, page):
 
     # Need to retry here because the element
     # will sometimes exist before the HTML content is loaded
-    exists_func = lambda(driver): page_exists(page) == should_exist
+    exists_func = lambda(driver): page_exists(page)
+
     world.wait_for(exists_func)
     assert_true(exists_func(None))
 
 
 @step(u'I "([^"]*)" the "([^"]*)" page$')
-def click_edit_delete(_step, edit_delete, page):
+def click_edit_delete(step, edit_delete, page):
     button_css = 'a.%s-button' % edit_delete
     index = get_index(page)
     assert index is not None
@@ -41,7 +52,7 @@ def click_edit_delete(_step, edit_delete, page):
 
 
 @step(u'I change the name to "([^"]*)"$')
-def change_name(_step, new_name):
+def change_name(step, new_name):
     settings_css = '#settings-mode a'
     world.css_click(settings_css)
     input_css = 'input.setting-input'
@@ -56,9 +67,10 @@ def get_index(name):
     page_name_css = 'section[data-type="HTMLModule"]'
     all_pages = world.css_find(page_name_css)
     for i in range(len(all_pages)):
-        if world.css_html(page_name_css, index=i) == '\n    {name}\n'.format(name=name):
+        if all_pages[i].html == '\n    {name}\n'.format(name=name):
             return i
     return None
 
+
 def page_exists(page):
     return get_index(page) is not None
diff --git a/cms/djangoapps/contentstore/features/upload.py b/cms/djangoapps/contentstore/features/upload.py
index 6eb96fc731..d0a4e9f366 100644
--- a/cms/djangoapps/contentstore/features/upload.py
+++ b/cms/djangoapps/contentstore/features/upload.py
@@ -7,6 +7,8 @@ import requests
 import string
 import random
 import os
+from nose.tools import assert_equal, assert_not_equal # pylint: disable=E0611
+
 
 TEST_ROOT = settings.COMMON_TEST_DATA_ROOT
 
@@ -32,7 +34,7 @@ def upload_file(_step, file_name):
 
 
 @step(u'I upload the files (".*")$')
-def upload_file(_step, files_string):
+def upload_files(_step, files_string):
     # Turn files_string to a list of file names
     files = files_string.split(",")
     files = map(lambda x: string.strip(x, ' "\''), files)
@@ -48,19 +50,29 @@ def upload_file(_step, files_string):
     world.css_click(close_css)
 
 
-@step(u'I should( not)? see the file "([^"]*)" was uploaded$')
-def check_upload(_step, do_not_see_file, file_name):
+@step(u'I should not see the file "([^"]*)" was uploaded$')
+def check_not_there(_step, file_name):
+    # Either there are no files, or there are files but
+    # not the one I expect not to exist.
+
+    # Since our only test for deletion right now deletes
+    # the only file that was uploaded, our success criteria
+    # will be that there are no files.
+    # In the future we can refactor if necessary.
+    names_css = 'td.name-col > a.filename'
+    assert(world.is_css_not_present(names_css))
+
+
+@step(u'I should see the file "([^"]*)" was uploaded$')
+def check_upload(_step, file_name):
     index = get_index(file_name)
-    if do_not_see_file:
-        assert index == -1
-    else:
-        assert index != -1
+    assert_not_equal(index, -1)
 
 
 @step(u'The url for the file "([^"]*)" is valid$')
 def check_url(_step, file_name):
     r = get_file(file_name)
-    assert r.status_code == 200
+    assert_equal(r.status_code , 200)
 
 
 @step(u'I delete the file "([^"]*)"$')
@@ -71,7 +83,7 @@ def delete_file(_step, file_name):
     world.css_click(delete_css, index=index)
 
     prompt_confirm_css = 'li.nav-item > a.action-primary'
-    world.css_click(prompt_confirm_css, success_condition=lambda: not world.css_visible(prompt_confirm_css))
+    world.css_click(prompt_confirm_css)
 
 
 @step(u'I should see only one "([^"]*)"$')
diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py
index a554a34f19..0dc7516a15 100644
--- a/common/djangoapps/terrain/browser.py
+++ b/common/djangoapps/terrain/browser.py
@@ -75,7 +75,8 @@ def make_desired_capabilities():
     desired_capabilities['build'] = settings.SAUCE.get('BUILD')
     desired_capabilities['video-upload-on-pass'] = False
     desired_capabilities['sauce-advisor'] = False
-    desired_capabilities['record-screenshots'] = False
+    desired_capabilities['capture-html'] = True
+    desired_capabilities['record-screenshots'] = True
     desired_capabilities['selenium-version'] = "2.34.0"
     desired_capabilities['max-duration'] = 3600
     desired_capabilities['public'] = 'public restricted'
@@ -87,6 +88,7 @@ def initial_setup(server):
     """
     Launch the browser once before executing the tests.
     """
+    # from nose.tools import set_trace; set_trace()
     world.absorb(settings.SAUCE.get('SAUCE_ENABLED'), 'SAUCE_ENABLED')
 
     if not world.SAUCE_ENABLED:
diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py
index 7d308931b2..6bdb11fa2e 100644
--- a/common/djangoapps/terrain/ui_helpers.py
+++ b/common/djangoapps/terrain/ui_helpers.py
@@ -5,7 +5,7 @@ from lettuce import world
 import time
 import platform
 from urllib import quote_plus
-from selenium.common.exceptions import WebDriverException, StaleElementReferenceException
+from selenium.common.exceptions import WebDriverException
 from selenium.webdriver.support import expected_conditions as EC
 from selenium.webdriver.common.by import By
 from selenium.webdriver.support.ui import WebDriverWait
@@ -18,11 +18,6 @@ def wait(seconds):
     time.sleep(float(seconds))
 
 
-@world.absorb
-def wait_for(func):
-    WebDriverWait(world.browser.driver, 5).until(func)
-
-
 @world.absorb
 def visit(url):
     world.browser.visit(django_url(url))
@@ -34,7 +29,7 @@ def url_equals(url):
 
 
 @world.absorb
-def is_css_present(css_selector, wait_time=5):
+def is_css_present(css_selector, wait_time=10):
     return world.browser.is_element_present_by_css(css_selector, wait_time=wait_time)
 
 
@@ -44,92 +39,116 @@ def is_css_not_present(css_selector, wait_time=5):
 
 
 @world.absorb
-def css_has_text(css_selector, text, index=0, max_attempts=5):
-    return world.css_text(css_selector, index=index, max_attempts=max_attempts) == text
+def css_has_text(css_selector, text, index=0):
+    return world.css_text(css_selector, index=index) == text
 
 
 @world.absorb
-def css_find(css, wait_time=5):
-    def is_visible(_driver):
-        return EC.visibility_of_element_located((By.CSS_SELECTOR, css,))
+def wait_for(func, timeout=5):
+        WebDriverWait(world.browser.driver, timeout).until(func)
 
-    world.browser.is_element_present_by_css(css, wait_time=wait_time)
-    wait_for(is_visible)
+
+def wait_for_present(css_selector, timeout=30):
+    """
+    Waiting for the element to be present in the DOM.
+    Throws an error if the wait_for time expires.
+    Otherwise this method will return None
+    """
+    WebDriverWait(driver=world.browser.driver,
+        timeout=60).until(EC.presence_of_element_located((By.CSS_SELECTOR, css_selector,)))
+
+
+@world.absorb
+def wait_for_visible(css_selector, timeout=30):
+    """
+    Waiting for the element to be visible in the DOM.
+    Throws an error if the wait_for time expires.
+    Otherwise this method will return None
+    """
+    WebDriverWait(driver=world.browser.driver,
+        timeout=timeout).until(EC.visibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+
+
+@world.absorb
+def wait_for_invisible(css_selector, timeout=30):
+    """
+    Waiting for the element to be either invisible or not present on the DOM.
+    Throws an error if the wait_for time expires.
+    Otherwise this method will return None
+    """
+    WebDriverWait(driver=world.browser.driver,
+        timeout=timeout).until(EC.invisibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+
+
+@world.absorb
+def wait_for_clickable(css_selector, timeout=30):
+    """
+    Waiting for the element to be present and clickable.
+    Throws an error if the wait_for time expires.
+    Otherwise this method will return None.
+    """
+    # Sometimes the element is clickable then gets obscured.
+    # In this case, pause so that it is not reported clickable too early
+    WebDriverWait(world.browser.driver,
+        timeout=timeout).until(EC.element_to_be_clickable((By.CSS_SELECTOR, css_selector,)))
+
+
+@world.absorb
+def css_find(css, wait_time=30):
+    """
+    Wait for the element(s) as defined by css locator
+    to be present.
+
+    This method will return a WebDriverElement.
+    """
+    wait_for_present(css_selector=css, timeout=wait_time)
     return world.browser.find_by_css(css)
 
 
 @world.absorb
-def css_click(css_selector, index=0, max_attempts=5, success_condition=lambda: True):
+def css_click(css_selector, index=0, wait_time=30):
     """
-    Perform a click on a CSS selector, retrying if it initially fails.
+    Perform a click on a CSS selector, first waiting for the element
+    to be present and clickable.
 
-    This function handles errors that may be thrown if the component cannot be clicked on.
-    However, there are cases where an error may not be thrown, and yet the operation did not
-    actually succeed. For those cases, a success_condition lambda can be supplied to verify that the click worked.
-
-    This function will return True if the click worked (taking into account both errors and the optional
-    success_condition).
+    This method will return True if the click worked.
     """
-    assert is_css_present(css_selector), "{} is not present".format(css_selector)
-    for _ in range(max_attempts):
-        try:
-            world.css_find(css_selector)[index].click()
-            if success_condition():
-                return
-        except WebDriverException:
-            # Occasionally, MathJax or other JavaScript can cover up
-            # an element temporarily.
-            # If this happens, wait a second, then try again
-            world.wait(1)
-        except:
-            pass
-    else:
-        # try once more, letting exceptions raise
-        world.css_find(css_selector)[index].click()
-        if not success_condition():
-            raise Exception("unsuccessful click")
+    wait_for_clickable(css_selector, timeout=wait_time)
+    assert world.css_find(css_selector)[index].visible
+
+    # Sometimes you can't click in the center of the element, as
+    # another element might be on top of it. In this case, try
+    # clicking in the upper left corner.
+    try:
+        return world.css_find(css_selector)[index].click()
+
+    except WebDriverException:
+        return css_click_at(css_selector, index=index)
 
 
 @world.absorb
-def css_check(css_selector, index=0, max_attempts=5, success_condition=lambda: True):
+def css_check(css_selector, index=0, wait_time=30):
     """
-    Checks a check box based on a CSS selector, retrying if it initially fails.
+    Checks a check box based on a CSS selector, first waiting for the element
+    to be present and clickable. This is just a wrapper for calling "click"
+    because that's how selenium interacts with check boxes and radio buttons.
 
-    This function handles errors that may be thrown if the component cannot be clicked on.
-    However, there are cases where an error may not be thrown, and yet the operation did not
-    actually succeed. For those cases, a success_condition lambda can be supplied to verify that the check worked.
-
-    This function will return True if the check worked (taking into account both errors and the optional
-    success_condition).
+    This method will return True if the check worked.
     """
-    assert is_css_present(css_selector), "{} is not present".format(css_selector)
-    for _ in range(max_attempts):
-        try:
-            world.css_find(css_selector)[index].check()
-            if success_condition():
-                return
-        except WebDriverException:
-            # Occasionally, MathJax or other JavaScript can cover up
-            # an element temporarily.
-            # If this happens, wait a second, then try again
-            world.wait(1)
-        except:
-            pass
-    else:
-        # try once more, letting exceptions raise
-        world.css_find(css_selector)[index].check()
-        if not success_condition():
-            raise Exception("unsuccessful check")
+    return css_click(css_selector=css_selector, index=index, wait_time=wait_time)
 
 
 @world.absorb
-def css_click_at(css, x_cord=10, y_cord=10):
+def css_click_at(css_selector, index=0, x_coord=10, y_coord=10, timeout=5):
     '''
     A method to click at x,y coordinates of the element
     rather than in the center of the element
     '''
-    element = css_find(css).first
-    element.action_chains.move_to_element_with_offset(element._element, x_cord, y_cord)
+    wait_for_clickable(css_selector, timeout=timeout)
+    element = css_find(css_selector)[index]
+    assert element.visible
+
+    element.action_chains.move_to_element_with_offset(element._element, x_coord, y_coord)
     element.action_chains.click()
     element.action_chains.perform()
 
@@ -139,58 +158,55 @@ def id_click(elem_id):
     """
     Perform a click on an element as specified by its id
     """
-    world.css_click('#%s' % elem_id)
+    css_click('#%s' % elem_id)
 
 
 @world.absorb
-def css_fill(css_selector, text, index=0, max_attempts=5):
-    assert is_css_present(css_selector)
-    return world.retry_on_exception(lambda: world.browser.find_by_css(css_selector)[index].fill(text), max_attempts=max_attempts)
+def css_fill(css_selector, text, index=0):
+    css_find(css_selector)[index].fill(text)
 
 
 @world.absorb
-def click_link(partial_text, index=0, max_attempts=5):
-    return world.retry_on_exception(lambda: world.browser.find_link_by_partial_text(partial_text)[index].click(), max_attempts=max_attempts)
+def click_link(partial_text, index=0):
+    world.browser.find_link_by_partial_text(partial_text)[index].click()
 
 
 @world.absorb
-def css_text(css_selector, index=0, max_attempts=5):
-
+def css_text(css_selector, index=0, timeout=30):
     # Wait for the css selector to appear
-    if world.is_css_present(css_selector):
-        return world.retry_on_exception(lambda: world.browser.find_by_css(css_selector)[index].text, max_attempts=max_attempts)
+    if is_css_present(css_selector):
+        return css_find(css_selector, wait_time=timeout)[index].text
     else:
         return ""
 
 
 @world.absorb
-def css_value(css_selector, index=0, max_attempts=5):
-
+def css_value(css_selector, index=0):
     # Wait for the css selector to appear
-    if world.is_css_present(css_selector):
-        return world.retry_on_exception(lambda: world.browser.find_by_css(css_selector)[index].value, max_attempts=max_attempts)
+    if is_css_present(css_selector):
+        return css_find(css_selector)[index].value
     else:
         return ""
 
 
 @world.absorb
-def css_html(css_selector, index=0, max_attempts=5):
+def css_html(css_selector, index=0):
     """
-    Returns the HTML of a css_selector and will retry if there is a StaleElementReferenceException
+    Returns the HTML of a css_selector
     """
     assert is_css_present(css_selector)
-    return world.retry_on_exception(lambda: world.browser.find_by_css(css_selector)[index].html, max_attempts=max_attempts)
+    return css_find(css_selector)[index].html
 
 
 @world.absorb
-def css_has_class(css_selector, class_name, index=0, max_attempts=5):
-    return world.retry_on_exception(lambda: world.css_find(css_selector)[index].has_class(class_name), max_attempts=max_attempts)
+def css_has_class(css_selector, class_name, index=0):
+    return css_find(css_selector)[index].has_class(class_name)
 
 
 @world.absorb
-def css_visible(css_selector, index=0, max_attempts=5):
+def css_visible(css_selector, index=0):
     assert is_css_present(css_selector)
-    return world.retry_on_exception(lambda: world.browser.find_by_css(css_selector)[index].visible, max_attempts=max_attempts)
+    return css_find(css_selector)[index].visible
 
 
 @world.absorb
@@ -235,14 +251,17 @@ def click_tools():
 def is_mac():
     return platform.mac_ver()[0] is not ''
 
+
 @world.absorb
 def is_firefox():
     return world.browser.driver_name is 'Firefox'
 
+
 @world.absorb
 def trigger_event(css_selector, event='change', index=0):
     world.browser.execute_script("$('{}:eq({})').trigger('{}')".format(css_selector, index, event))
 
+
 @world.absorb
 def retry_on_exception(func, max_attempts=5):
     attempt = 0
diff --git a/lms/djangoapps/courseware/features/navigation.py b/lms/djangoapps/courseware/features/navigation.py
index fcbf21b095..5ccfdd54ff 100644
--- a/lms/djangoapps/courseware/features/navigation.py
+++ b/lms/djangoapps/courseware/features/navigation.py
@@ -82,11 +82,7 @@ def click_on_section(step, section):
 
     subid = "ui-accordion-accordion-panel-" + str(int(section) - 1)
     subsection_css = 'ul.ui-accordion-content-active[id=\'%s\'] > li > a' % subid
-    prev_url = world.browser.url
-    changed_section = lambda: prev_url != world.browser.url
-
-    #for some reason needed to do it in two steps
-    world.css_click(subsection_css, success_condition=changed_section)
+    world.css_click(subsection_css)
 
 
 @step(u'I click on subsection "([^"]*)"$')
diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature
index e3e7b8ac5a..ad70e5a062 100644
--- a/lms/djangoapps/courseware/features/problems.feature
+++ b/lms/djangoapps/courseware/features/problems.feature
@@ -129,7 +129,7 @@ Feature: Answer problems
         When I press the button with the label "Hide Answer(s)"
         Then the button with the label "Show Answer(s)" does appear
         And I should not see "4.14159" anywhere on the page
-    
+
     Scenario: I can see my score on a problem when I answer it and after I reset it
         Given I am viewing a "<ProblemType>" problem
         When I answer a "<ProblemType>" problem "<Correctness>ly"
diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py
index 2eaa8b5c07..1bb803d09d 100644
--- a/lms/djangoapps/courseware/features/problems.py
+++ b/lms/djangoapps/courseware/features/problems.py
@@ -101,6 +101,9 @@ def input_problem_answer(_, problem_type, correctness):
 
 @step(u'I check a problem')
 def check_problem(step):
+    # first scroll down so the loading mathjax button does not
+    # cover up the Check button
+    world.browser.execute_script("window.scrollTo(0,1024)")
     world.css_click("input.check")
 
 
diff --git a/lms/djangoapps/courseware/features/signup.py b/lms/djangoapps/courseware/features/signup.py
index ef864aa855..c12af6c47b 100644
--- a/lms/djangoapps/courseware/features/signup.py
+++ b/lms/djangoapps/courseware/features/signup.py
@@ -23,9 +23,8 @@ def i_press_the_button_on_the_registration_form(step):
 
 @step('I check the checkbox named "([^"]*)"$')
 def i_check_checkbox(step, checkbox):
-    def check_box():
-        world.browser.find_by_name(checkbox).check()
-    world.retry_on_exception(check_box)
+    css_selector = 'input[name={}]'.format(checkbox)
+    world.css_check(css_selector)
 
 
 @step('I should see "([^"]*)" in the dashboard banner$')

From 30b13d3cf189b67c1ab924fdc13870d43400c985 Mon Sep 17 00:00:00 2001
From: Jay Zoldak <zoldak@edx.org>
Date: Mon, 16 Sep 2013 11:09:54 -0400
Subject: [PATCH 2/2] Improve code clarity and error messages for css selection

---
 .../features/advanced-settings.py             |  5 +--
 .../contentstore/features/common.py           |  2 +-
 .../contentstore/features/course-team.py      | 24 ++++++------
 .../contentstore/features/grading.py          |  4 +-
 .../contentstore/features/problem-editor.py   | 15 +++++---
 common/djangoapps/terrain/browser.py          | 12 +++---
 common/djangoapps/terrain/ui_helpers.py       | 38 +++++++++++++------
 .../courseware/features/navigation.py         |  4 +-
 .../courseware/features/problems_setup.py     |  2 +
 9 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py
index e202031a01..664d8d00ae 100644
--- a/cms/djangoapps/contentstore/features/advanced-settings.py
+++ b/cms/djangoapps/contentstore/features/advanced-settings.py
@@ -11,7 +11,6 @@ DISPLAY_NAME_KEY = "display_name"
 DISPLAY_NAME_VALUE = '"Robot Super Course"'
 
 
-############### ACTIONS ####################
 @step('I select the Advanced Settings$')
 def i_select_advanced_settings(step):
     world.click_course_settings()
@@ -45,7 +44,6 @@ def create_value_not_in_quotes(step):
     change_display_name_value(step, 'quote me')
 
 
-############### RESULTS ####################
 @step('I see default advanced settings$')
 def i_see_default_advanced_settings(step):
     # Test only a few of the existing properties (there are around 34 of them)
@@ -88,14 +86,13 @@ def the_policy_key_value_is_changed(step):
     assert_equal(get_display_name_value(), '"foo"')
 
 
-############# HELPERS ###############
 def assert_policy_entries(expected_keys, expected_values):
     for key, value in zip(expected_keys, expected_values):
         index = get_index_of(key)
         assert_false(index == -1, "Could not find key: {key}".format(key=key))
         found_value = world.css_find(VALUE_CSS)[index].value
         assert_equal(value, found_value,
-            "{} is not equal to {}".format(value, found_value))
+            "Expected {} to have value {} but found {}".format(key, value, found_value))
 
 
 def get_index_of(expected_key):
diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py
index 74bab10a24..eb666d1837 100644
--- a/cms/djangoapps/contentstore/features/common.py
+++ b/cms/djangoapps/contentstore/features/common.py
@@ -165,7 +165,7 @@ def log_into_studio(
     world.log_in(username=uname, password=password, email=email, name=name)
     # Navigate to the studio dashboard
     world.visit('/')
-    assert_in(uname, world.css_text('h2.title', timeout=60))
+    assert_in(uname, world.css_text('h2.title', timeout=10))
 
 
 def create_a_course():
diff --git a/cms/djangoapps/contentstore/features/course-team.py b/cms/djangoapps/contentstore/features/course-team.py
index 763c116a56..91049b029e 100644
--- a/cms/djangoapps/contentstore/features/course-team.py
+++ b/cms/djangoapps/contentstore/features/course-team.py
@@ -110,9 +110,9 @@ def other_user_login(_step, name):
 
 @step(u'I( do not)? see the course on my page')
 @step(u's?he does( not)? see the course on (his|her) page')
-def see_course(_step, inverted, gender='self'):
+def see_course(_step, do_not_see, gender='self'):
     class_css = 'h3.course-title'
-    if inverted:
+    if do_not_see:
         assert world.is_css_not_present(class_css)
     else:
         all_courses = world.css_find(class_css)
@@ -121,25 +121,25 @@ def see_course(_step, inverted, gender='self'):
 
 
 @step(u'"([^"]*)" should( not)? be marked as an admin')
-def marked_as_admin(_step, name, inverted):
+def marked_as_admin(_step, name, not_marked_admin):
     flag_css = '.user-item[data-email="{email}"] .flag-role.flag-role-admin'.format(
         email=name+EMAIL_EXTENSION)
-    if inverted:
+    if not_marked_admin:
         assert world.is_css_not_present(flag_css)
     else:
         assert world.is_css_present(flag_css)
 
 
 @step(u'I should( not)? be marked as an admin')
-def self_marked_as_admin(_step, inverted):
-    return marked_as_admin(_step, "robot+studio", inverted)
+def self_marked_as_admin(_step, not_marked_admin):
+    return marked_as_admin(_step, "robot+studio", not_marked_admin)
 
 
 @step(u'I can(not)? delete users')
 @step(u's?he can(not)? delete users')
-def can_delete_users(_step, inverted):
+def can_delete_users(_step, can_not_delete):
     to_delete_css = 'a.remove-user'
-    if inverted:
+    if can_not_delete:
         assert world.is_css_not_present(to_delete_css)
     else:
         assert world.is_css_present(to_delete_css)
@@ -147,9 +147,9 @@ def can_delete_users(_step, inverted):
 
 @step(u'I can(not)? add users')
 @step(u's?he can(not)? add users')
-def can_add_users(_step, inverted):
+def can_add_users(_step, can_not_add):
     add_css = 'a.create-user-button'
-    if inverted:
+    if can_not_add:
         assert world.is_css_not_present(add_css)
     else:
         assert world.is_css_present(add_css)
@@ -157,13 +157,13 @@ def can_add_users(_step, inverted):
 
 @step(u'I can(not)? make ("([^"]*)"|myself) a course team admin')
 @step(u's?he can(not)? make ("([^"]*)"|me) a course team admin')
-def can_make_course_admin(_step, inverted, outer_capture, name):
+def can_make_course_admin(_step, can_not_make_admin, outer_capture, name):
     if outer_capture == "myself":
         email = world.scenario_dict["USER"].email
     else:
         email = name + EMAIL_EXTENSION
     add_button_css = '.user-item[data-email="{email}"] .add-admin-role'.format(email=email)
-    if inverted:
+    if can_not_make_admin:
         assert world.is_css_not_present(add_button_css)
     else:
         assert world.is_css_present(add_button_css)
diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py
index 6f702b8a5d..62b6617823 100644
--- a/cms/djangoapps/contentstore/features/grading.py
+++ b/cms/djangoapps/contentstore/features/grading.py
@@ -82,9 +82,9 @@ def see_assignment_name(step, do_not, name):
     assignment_menu = world.css_find(assignment_menu_css)
     allnames = [item.html for item in assignment_menu]
     if do_not:
-        assert_not_in (name, allnames)
+        assert_not_in(name, allnames)
     else:
-        assert_in (name, allnames)
+        assert_in(name, allnames)
 
 
 @step(u'I delete the assignment type "([^"]*)"$')
diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py
index 414b2dea80..e6528f06f7 100644
--- a/cms/djangoapps/contentstore/features/problem-editor.py
+++ b/cms/djangoapps/contentstore/features/problem-editor.py
@@ -2,7 +2,7 @@
 #pylint: disable=C0111
 
 from lettuce import world, step
-from nose.tools import assert_equal  # pylint: disable=E0611
+from nose.tools import assert_equal, assert_true  # pylint: disable=E0611
 from common import type_in_codemirror
 
 DISPLAY_NAME = "Display Name"
@@ -198,14 +198,19 @@ def high_level_source_in_editor(step):
 
 def verify_high_level_source_links(step, visible):
     if visible:
-        assert world.is_css_present('.launch-latex-compiler')
+        assert_true(world.is_css_present('.launch-latex-compiler'),
+                    msg="Expected to find the latex button but it is not present.")
     else:
-        assert world.is_css_not_present('.launch-latex-compiler')
+        assert_true(world.is_css_not_present('.launch-latex-compiler'),
+                    msg="Expected not to find the latex button but it is present.")
+
     world.cancel_component(step)
     if visible:
-        assert world.is_css_present('.upload-button')
+        assert_true(world.is_css_present('.upload-button'),
+                    msg="Expected to find the upload button but it is not present.")
     else:
-        assert world.is_css_not_present('.upload-button')
+        assert_true(world.is_css_not_present('.upload-button'),
+                    msg="Expected not to find the upload button but it is present.")
 
 
 def verify_modified_weight():
diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py
index 0dc7516a15..6454cbf644 100644
--- a/common/djangoapps/terrain/browser.py
+++ b/common/djangoapps/terrain/browser.py
@@ -88,7 +88,6 @@ def initial_setup(server):
     """
     Launch the browser once before executing the tests.
     """
-    # from nose.tools import set_trace; set_trace()
     world.absorb(settings.SAUCE.get('SAUCE_ENABLED'), 'SAUCE_ENABLED')
 
     if not world.SAUCE_ENABLED:
@@ -166,15 +165,18 @@ def reset_databases(scenario):
     xmodule.modulestore.django.clear_existing_modulestores()
 
 
-# Uncomment below to trigger a screenshot on error
-# @after.each_scenario
+@after.each_scenario
 def screenshot_on_error(scenario):
     """
     Save a screenshot to help with debugging.
     """
     if scenario.failed:
-        world.browser.driver.save_screenshot('/tmp/last_failed_scenario.png')
-
+        try:
+            output_dir = '{}/log'.format(settings.TEST_ROOT)
+            image_name = '{}/{}.png'.format(output_dir, scenario.name.replace(' ', '_'))
+            world.browser.driver.save_screenshot(image_name)
+        except WebDriverException:
+            LOGGER.error('Could not capture a screenshot')
 
 @after.all
 def teardown_browser(total):
diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py
index 6bdb11fa2e..0ba2dfba18 100644
--- a/common/djangoapps/terrain/ui_helpers.py
+++ b/common/djangoapps/terrain/ui_helpers.py
@@ -5,7 +5,7 @@ from lettuce import world
 import time
 import platform
 from urllib import quote_plus
-from selenium.common.exceptions import WebDriverException
+from selenium.common.exceptions import WebDriverException, TimeoutException
 from selenium.webdriver.support import expected_conditions as EC
 from selenium.webdriver.common.by import By
 from selenium.webdriver.support.ui import WebDriverWait
@@ -54,8 +54,11 @@ def wait_for_present(css_selector, timeout=30):
     Throws an error if the wait_for time expires.
     Otherwise this method will return None
     """
-    WebDriverWait(driver=world.browser.driver,
-        timeout=60).until(EC.presence_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    try:
+        WebDriverWait(driver=world.browser.driver,
+            timeout=60).until(EC.presence_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    except TimeoutException:
+        raise TimeoutException("Timed out waiting for {} to be present.".format(css_selector))
 
 
 @world.absorb
@@ -65,8 +68,11 @@ def wait_for_visible(css_selector, timeout=30):
     Throws an error if the wait_for time expires.
     Otherwise this method will return None
     """
-    WebDriverWait(driver=world.browser.driver,
-        timeout=timeout).until(EC.visibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    try:
+        WebDriverWait(driver=world.browser.driver,
+            timeout=timeout).until(EC.visibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    except TimeoutException:
+        raise TimeoutException("Timed out waiting for {} to be visible.".format(css_selector))
 
 
 @world.absorb
@@ -76,8 +82,11 @@ def wait_for_invisible(css_selector, timeout=30):
     Throws an error if the wait_for time expires.
     Otherwise this method will return None
     """
-    WebDriverWait(driver=world.browser.driver,
-        timeout=timeout).until(EC.invisibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    try:
+        WebDriverWait(driver=world.browser.driver,
+            timeout=timeout).until(EC.invisibility_of_element_located((By.CSS_SELECTOR, css_selector,)))
+    except TimeoutException:
+        raise TimeoutException("Timed out waiting for {} to be invisible.".format(css_selector))
 
 
 @world.absorb
@@ -89,8 +98,11 @@ def wait_for_clickable(css_selector, timeout=30):
     """
     # Sometimes the element is clickable then gets obscured.
     # In this case, pause so that it is not reported clickable too early
-    WebDriverWait(world.browser.driver,
-        timeout=timeout).until(EC.element_to_be_clickable((By.CSS_SELECTOR, css_selector,)))
+    try:
+        WebDriverWait(world.browser.driver,
+            timeout=timeout).until(EC.element_to_be_clickable((By.CSS_SELECTOR, css_selector,)))
+    except TimeoutException:
+        raise TimeoutException("Timed out waiting for {} to be clickable.".format(css_selector))
 
 
 @world.absorb
@@ -114,7 +126,8 @@ def css_click(css_selector, index=0, wait_time=30):
     This method will return True if the click worked.
     """
     wait_for_clickable(css_selector, timeout=wait_time)
-    assert world.css_find(css_selector)[index].visible
+    assert_true(world.css_find(css_selector)[index].visible,
+                msg="Element {}[{}] is present but not visible".format(css_selector, index))
 
     # Sometimes you can't click in the center of the element, as
     # another element might be on top of it. In this case, try
@@ -146,7 +159,8 @@ def css_click_at(css_selector, index=0, x_coord=10, y_coord=10, timeout=5):
     '''
     wait_for_clickable(css_selector, timeout=timeout)
     element = css_find(css_selector)[index]
-    assert element.visible
+    assert_true(element.visible,
+                msg="Element {}[{}] is present but not visible".format(css_selector, index))
 
     element.action_chains.move_to_element_with_offset(element._element, x_coord, y_coord)
     element.action_chains.click()
@@ -158,7 +172,7 @@ def id_click(elem_id):
     """
     Perform a click on an element as specified by its id
     """
-    css_click('#%s' % elem_id)
+    css_click('#{}'.format(elem_id))
 
 
 @world.absorb
diff --git a/lms/djangoapps/courseware/features/navigation.py b/lms/djangoapps/courseware/features/navigation.py
index 5ccfdd54ff..e4cd4961c2 100644
--- a/lms/djangoapps/courseware/features/navigation.py
+++ b/lms/djangoapps/courseware/features/navigation.py
@@ -80,8 +80,8 @@ def click_on_section(step, section):
     section_css = 'h3[tabindex="-1"]'
     world.css_click(section_css)
 
-    subid = "ui-accordion-accordion-panel-" + str(int(section) - 1)
-    subsection_css = 'ul.ui-accordion-content-active[id=\'%s\'] > li > a' % subid
+    subid = "ui-accordion-accordion-panel-{}".format(str(int(section) - 1))
+    subsection_css = "ul.ui-accordion-content-active[id='{}'] > li > a".format(subid)
     world.css_click(subsection_css)
 
 
diff --git a/lms/djangoapps/courseware/features/problems_setup.py b/lms/djangoapps/courseware/features/problems_setup.py
index 45b77eddfa..0253571b47 100644
--- a/lms/djangoapps/courseware/features/problems_setup.py
+++ b/lms/djangoapps/courseware/features/problems_setup.py
@@ -166,6 +166,8 @@ def answer_problem(problem_type, correctness):
     if problem_type == "drop down":
         select_name = "input_i4x-edx-model_course-problem-drop_down_2_1"
         option_text = 'Option 2' if correctness == 'correct' else 'Option 3'
+        # First wait for the element to be there on the page
+        world.wait_for_visible("select#{}".format(select_name))
         world.browser.select(select_name, option_text)
 
     elif problem_type == "multiple choice":