Add a Selenium test suite for Review Board.

This adds a new Selenium test suite for testing various user-facing
parts of Review Board through the Selenium browser automation tools.

The tests cover most common operations. In particular:

* Logging in
* Creating/modifying/publishing a review request
* Closing review requests
* Uploading diffs
* Uploading screenshots
* Viewing diffs
* Reviewing
* Commenting (diffs and screenshots)
* And more...

This provides full coverage of our datastore.js, meaning that we have
complete UI coverage for all calls to our web API. When we start moving to
the new web API, this should help us catch any regressions.

The tests live in the webtests/ directory, outside of the main reviewboard/
package directory. The reason for this is that we don't necessarily want to
have these tests run every time the full test suite is run, as they need
to mess with the browser, and they're slow. If we choose, we can run
these tests with the main test suite by doing:

	$ ./reviewboard/manage.py test -- --with-webtests

Or, to run just the web tests:

	$ ./reviewboard/manage.py test -- webtests
Or:
	$ ./contrib/internal/webtests.sh

This updates some URLs and templates in order to add identification information
to some elements in order to let us more easily test them.

It also fixes up some of the test runner code to properly build and tear down
the media directories, so that we can access all bundled media and uploaded
media for the tests.

Reviewed at http://reviews.reviewboard.org/r/1527/

MANIFEST.in

@@ -2,6 +2,7 @@ recursive-include contrib *
 recursive-include docs *.txt *.png *.py Makefile
 recursive-include locale *
 recursive-include reviewboard *
+recursive-include webtests *
 prune docs/*/_build
 prune reviewboard/htdocs/media/uploaded/images
 include ez_setup.py

contrib/conf/settings_local.py.tmpl

@@ -56,3 +56,11 @@ LOGGING_ALLOW_PROFILING = True
 DEBUG = True
 
 INTERNAL_IPS = "127.0.0.1"
+
+
+# Selenium testing configuration
+#TEST_DATABASE_NAME = "test.db"
+#SELENIUM_BROWSER_COMMAND = "*firefox"
+#SELENIUM_HOST = "localhost"
+#SELENIUM_PORT = 4444
+#SELENIUM_LIVE_SERVER_ADDRESS = "127.0.0.1"

contrib/internal/webtests.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+./reviewboard/manage.py test -- webtests

reviewboard/htdocs/media/rb/js/datastore.js

@@ -193,14 +193,13 @@ $.extend(RB.Diff.prototype, {
         }, options);
 
         if (this.id != undefined) {
-            /* TODO: Support updating screenshots eventually. */
             options.error("The diff " + this.id + " was already created. " +
                           "This is a script error. Please report it.");
             return;
         }
 
         if (!this.form) {
-            options.error("No data has been set for this screenshot. This " +
+            options.error("No data has been set for this diff. This " +
                           "is a script error. Please report it.");
             return;
         }

reviewboard/htdocs/media/rb/js/reviews.js

@@ -302,7 +302,14 @@ $.fn.commentSection = function(review_id, context_id, context_type) {
      * Creates a new comment form in response to the "Add Comment" link.
      */
     function createNewCommentForm() {
-        var yourcomment_id = "yourcomment_" + sectionId + "-draft";
+        var yourcomment_id = "yourcomment_" + review_id + "_" +
+                             context_type;
+        if (sectionId) {
+            yourcomment_id += "_" + sectionId;
+        }
+
+        yourcomment_id += "-draft";
+
         $("<li/>")
             .addClass("reply-comment draft editor")
             .attr("id", yourcomment_id + "-item")
@@ -822,15 +829,21 @@ $.reviewForm = function(review) {
 
         buttons = $("input", dlg);
 
-        $(".body-top, .body-bottom", dlg)
-            .inlineEditor({
-                extraHeight: 50,
-                forceOpen: true,
-                multiline: true,
-                notifyUnchangedCompletion: true,
-                showButtons: false,
-                showEditIcon: false
-            });
+        var body_classes = ["body-top", "body-bottom"];
+
+        for (var i in body_classes) {
+            var cls = body_classes[i];
+            $("." + cls, dlg)
+                .inlineEditor({
+                    cls: cls + "-editor",
+                    extraHeight: 50,
+                    forceOpen: true,
+                    multiline: true,
+                    notifyUnchangedCompletion: true,
+                    showButtons: false,
+                    showEditIcon: false
+                });
+        }
 
         $("textarea:first", dlg).focus();
         dlg.attr("scrollTop", 0);

reviewboard/nose.cfg

@@ -0,0 +1,8 @@
+[nosetests]
+verbosity=3
+with-coverage=1
+with-doctest=1
+doctest-extension=.txt
+cover-package=reviewboard
+detailed-errors=1
+exclude=setup_test_environment|teardown_test_environment

reviewboard/reviews/models.py

@@ -424,8 +424,9 @@ def changeset_is_pending(self):
         if self.changenum:
             try:
                 changeset = self.repository.get_scmtool().get_changeset(self.changenum)
-            except EmptyChangeSetError:
+            except (EmptyChangeSetError, NotImplementedError):
                 pass
+
         return changeset and changeset.pending
 
     @permalink

reviewboard/reviews/urls.py

@@ -20,7 +20,8 @@
     url(r'^(?P<review_request_id>[0-9]+)/diff/(?P<revision>[0-9]+)/$', 'diff',
         name="view_diff_revision"),
 
-    (r'^(?P<review_request_id>[0-9]+)/diff/raw/$', 'raw_diff'),
+    url(r'^(?P<review_request_id>[0-9]+)/diff/raw/$', 'raw_diff',
+        name='raw_diff'),
     (r'^(?P<review_request_id>[0-9]+)/diff/(?P<revision>[0-9]+)/raw/$',
      'raw_diff'),
 

reviewboard/templates/reviews/review_reply_section.html

@@ -6,15 +6,17 @@
    </ol>
 {% if user.is_authenticated %}
    <ul class="controls">
-    <li><a href="#" id="add_{{context_type}}_{{comment.id}}" class="add_comment_link">{% trans "Add comment" %}</a></li>
+    <li><a href="#" id="add_{{review.id}}_{{context_type}}{% if comment %}_{{comment.id}}{% endif %}" class="add_comment_link">{% trans "Add comment" %}</a></li>
    </ul>
 {% endif %}
   </div>
 {% if user.is_authenticated %}
   <script language="javascript">
   $(document).ready(function() {
-      $("#{{context_id}}-{{review.id}}").commentSection("{{review.id}}",
-          "{% ifequal comment "" %}{{review.id}}{% else %}{{comment.id}}{% endifequal %}", "{{context_type}}");
+      $("#{{context_id}}-{{review.id}}").commentSection(
+          "{{review.id}}",
+          {% if comment %}"{{comment.id}}"{% else %}null{% endif %},
+          "{{context_type}}");
   });
   </script>
 {% endif %}

reviewboard/templates/reviews/review_request_box.html

@@ -27,25 +27,25 @@
   </tr>
   <tr>
    <td class="label"><label for="branch">{% trans "Branch" %}:</label></td>
-   <td class="value"><span id="branch" class="editable">{{review_request_details.branch|safe}}</span></td>
+   <td class="value" id="branch-value-cell"><span id="branch" class="editable">{{review_request_details.branch|safe}}</span></td>
    <td class="indented label"><label for="target_groups">{% trans "Groups" %}:</label></td>
-   <td class="value"><span id="target_groups" class="editable">{% spaceless %}
+   <td class="value" id="target-groups-value-cell"><span id="target_groups" class="editable">{% spaceless %}
 {%  for group in review_request_details.target_groups.all %}
 <a href="{% url group group %}">{{group}}</a>{%if not forloop.last %}, {%endif %}
 {%  endfor %}
 {% endspaceless %}</span></td>
   </tr>
   <tr>
    <td class="label"><label for="bugs_closed">{% trans "Bugs" %}:</label></td>
-   <td class="value"><span id="bugs_closed" class="editable comma-editable">{% spaceless %}
+   <td class="value" id="bugs-closed-value-cell"><span id="bugs_closed" class="editable comma-editable">{% spaceless %}
 {%  for bug in review_request_details.get_bug_list %}
 {%   with bug|bug_url:review_request as bug_url %}
 {%    if bug_url %}<a href="{{bug_url}}">{{bug}}</a>{% else %}{{bug}}{% endif %}{% if not forloop.last %}, {% endif %}
 {%   endwith %}
 {%  endfor %}
 {% endspaceless %}</span></td>
    <td class="indented label"><label for="target_people">{% trans "People" %}:</label></td>
-   <td class="value"><span id="target_people" class="editable comma-editable">{% spaceless %}
+   <td class="value" id="target-people-value-cell"><span id="target_people" class="editable comma-editable">{% spaceless %}
 {%  for person in review_request_details.target_people.all %}
 <a href="{% url user person %}"{% if not person.is_active %} class="inactive"{% endif %}>{{person}}</a>{% if not forloop.last %}, {% endif %}
 {%  endfor %}
@@ -58,11 +58,11 @@
    <td class="value"><span id="repository">{{review_request.repository}}</span></td>
   </tr>
  </table>
- <div class="content">
+ <div class="content" id="description-value-cell">
   <label for="description">{% trans "Description" %}:</label>
   <pre id="description" class="editable">{{review_request_details.description|escape}}</pre>
  </div>
- <div class="content">
+ <div class="content" id="testing-done-value-cell">
   <label for="testing_done">{% trans "Testing Done" %}:</label>
   <pre id="testing_done" class="editable">{{review_request_details.testing_done|escape}}</pre>
  </div>

reviewboard/test.py

@@ -23,9 +23,12 @@
 # SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-import os.path
+import os
+import platform
+import shutil
 import sys
 
+import pkg_resources
 import nose
 
 try:
@@ -42,23 +45,73 @@
 from django.test.utils import setup_test_environment, teardown_test_environment
 
 
-def runner(module_list, verbosity=1, interactive=True, extra_tests=[]):
-    setup_test_environment()
-    settings.DEBUG = False
-
-    # Default to testing in a non-subdir install.
-    settings.SITE_ROOT = "/"
+def setup_media_dirs():
+    old_media_root = settings.MEDIA_ROOT
     settings.MEDIA_ROOT = "/tmp/reviewboard-tests"
 
+    if os.path.exists(settings.MEDIA_ROOT):
+        destroy_media_dirs()
+
     images_dir = os.path.join(settings.MEDIA_ROOT, "uploaded", "images")
 
     if not os.path.exists(images_dir):
         os.makedirs(images_dir)
 
+    # Set up symlinks to the other directories for the web-based tests
+    bundled_media_dir = os.path.join(os.path.dirname(__file__),
+                                     'htdocs', 'media')
+
+    for path in os.listdir(bundled_media_dir):
+        if path == 'uploaded':
+            continue
+
+        os.symlink(os.path.join(bundled_media_dir, path),
+                   os.path.join(settings.MEDIA_ROOT, path))
+
+    if not os.path.exists(os.path.join(settings.MEDIA_ROOT, 'djblets')):
+        if not pkg_resources.resource_exists("djblets", "media"):
+            sys.stderr.write("Unable to find a valid Djblets installation.\n")
+            sys.stderr.write("Make sure you've installed Djblets.")
+            sys.exit(1)
+
+        src_dir = pkg_resources.resource_filename('djblets', 'media')
+        dest_dir = os.path.join(settings.MEDIA_ROOT, 'djblets')
+
+        if platform.system() == 'windows':
+            shutil.copytree(src_dir, dest_dir)
+        else:
+            os.symlink(src_dir, dest_dir)
+
+
+def destroy_media_dirs():
+    for root, dirs, files in os.walk(settings.MEDIA_ROOT, topdown=False):
+        for name in files:
+            os.remove(os.path.join(root, name))
+
+        for name in dirs:
+            path = os.path.join(root, name)
+
+            if os.path.islink(path):
+                os.remove(path)
+            else:
+                os.rmdir(path)
+
+    os.rmdir(settings.MEDIA_ROOT)
+
+
+def runner(module_list, verbosity=1, interactive=True, extra_tests=[]):
+    setup_test_environment()
+    settings.DEBUG = False
+
+    # Default to testing in a non-subdir install.
+    settings.SITE_ROOT = "/"
+
     settings.MEDIA_URL = settings.SITE_ROOT + 'media/'
     settings.ADMIN_MEDIA_PREFIX = settings.MEDIA_URL + 'admin/'
     settings.RUNNING_TEST = True
 
+    setup_media_dirs()
+
     old_name = settings.DATABASE_NAME
     connection.creation.create_test_db(verbosity, autoclobber=not interactive)
     management.call_command('syncdb', verbosity=verbosity, interactive=interactive)
@@ -76,18 +129,17 @@ def runner(module_list, verbosity=1, interactive=True, extra_tests=[]):
     for cover in ['reviewboard', 'djblets']:
         nose_argv += ['--cover-package=' + cover]
 
+    if '--with-webtests' in sys.argv:
+        nose_argv += ['--cover-package=webtests']
+        sys.argv.remove('--with-webtests')
+
     # manage.py captures everything before "--"
     if len(sys.argv) > 2 and sys.argv.__contains__("--"):
         nose_argv += sys.argv[(sys.argv.index("--") + 1):]
 
-    nose.main(argv=nose_argv)
+    nose.main(argv=nose_argv, exit=False)
 
-    for root, dirs, files in os.walk(settings.MEDIA_ROOT, topdown=False):
-        for name in files:
-            os.remove(os.path.join(root, name))
-
-        for name in dirs:
-            os.rmdir(os.path.join(root, name))
+    destroy_media_dirs()
 
-    connection.creation.destroy_test_db(old_name, verbosity)
+    connection.creation.destroy_test_db(old_name, verbosity=0)
     teardown_test_environment()

reviewboard/urls.py

@@ -25,11 +25,11 @@
 )
 
 # Add static media if running in DEBUG mode
-if settings.DEBUG:
+if settings.DEBUG or getattr(settings, 'RUNNING_TEST', False):
     urlpatterns += patterns('django.views.static',
         (r'^media/(?P<path>.*)$', 'serve', {
             'show_indexes': True,
-            'document_root': os.path.join(settings.HTDOCS_ROOT, "media"),
+            'document_root': settings.MEDIA_ROOT,
             }),
     )