reviewboard: integrate Review Board server into tests
authorGregory Szorc <gps@mozilla.com>
Tue, 24 Jun 2014 00:10:19 -0700
changeset 358852 27a1f27cb0acf1a234ed1d3b2599c879f9d9dba9
parent 358851 95679453fed6abc105ee831974257f6231104f2c
child 358853 782f314c73d4e4c481fa554661f7a527fc8a69f9
push id16998
push userrwood@mozilla.com
push dateMon, 02 May 2016 19:42:03 +0000
reviewboard: integrate Review Board server into tests We now launch a local Review Board service instance when running the Review Board tests. Along the way, the logic for mapping commits to reviews was rewritten to make it behave slightly better. The tests now need rework to download results from Review Board to ensure that the interaction actually does what we want it to do. There is a test failure in test-obsolescence.t, presumably due to a bug in commit<->review reconciling. The daemon process running the Review Board server also appears to get orphaned after the tests finish. These issues will be addressed later.
.hgignore
README.rst
hgext/reviewboard/tests/dummy_rbpost.py
hgext/reviewboard/tests/helpers.sh
hgext/reviewboard/tests/rbmanage.py
hgext/reviewboard/tests/test-errors.t
hgext/reviewboard/tests/test-obsolescence.t
hgext/reviewboard/tests/test-push.t
hgext/reviewboard/tests/test-store.t
pylib/reviewboardmods/reviewboardmods/pushhooks.py
run-mercurial-tests.py
test-requirements.txt
--- a/.hgignore
+++ b/.hgignore
@@ -1,4 +1,5 @@
+venv/
 \.pyc$
 \.pyo$
 \.swp$
 ~$
--- a/README.rst
+++ b/README.rst
@@ -39,8 +39,22 @@ number of features:
 * Pushlog data is synchronized to a local database.
 * Bug data is extracted from commit messages and stored in a database.
 * Many revision set and template functions are added.
 
 If you are looking to turn Mercurial into a more powerful query tool or
 want to maintain a unified repository, *mozext* is very valuable.
 
 This extension lives under ``hgext/mozext``.
+
+Testing
+=======
+
+Testing requires a Python virtualenv because some extensions launch
+actual servers instead of mocking. To create the virtualenv::
+
+  $ virtualenv env
+  $ source env/bin/activate
+  $ pip install -r test-requirements.txt
+  $ easy_install ReviewBoard==2.0.2
+
+(Yes, we need to use ``easy_install`` because ReviewBoard doesn't play
+nice with pip.)
deleted file mode 100644
--- a/hgext/reviewboard/tests/dummy_rbpost.py
+++ /dev/null
@@ -1,69 +0,0 @@
-# This software may be used and distributed according to the terms of the
-# GNU General Public License version 2 or any later version.
-
-"""This file implements a dummy extension that monkeypatches the
-post_reviews symbol in the server command handling module so it can
-be tested.
-"""
-
-import os
-
-from mercurial import extensions
-
-import hgrb.shared
-
-REPO = None
-
-def post_reviews(original, url, repoid, identifier, commits,
-        username=None, password=None, cookie=None):
-    reviews = []
-    if REPO.vfs.exists('DUMMY_REVIEWS'):
-        for i, line in enumerate(REPO.vfs('DUMMY_REVIEWS')):
-            line = line.strip()
-            reviews.append(line)
-
-    lines = []
-    parentid = None
-    try:
-        parentid = str(reviews.index(identifier) + 1)
-    except ValueError:
-        reviews.append(identifier)
-        parentid = str(len(reviews))
-
-    lines.extend([
-        'url: %s' % url,
-        'username: %s' % username,
-        'password: %s' % password,
-        'repoid: %s' % repoid,
-        'identifier: %s' % identifier,
-    ])
-
-    reviewmap = {}
-    for i, commit in enumerate(commits['individual']):
-        lines.extend([
-            str(i),
-            commit['id'],
-            commit['message'],
-            commit['diff'],
-            commit['parent_diff'] or 'NO PARENT DIFF'
-        ])
-
-        if not commit['rid']:
-            reviews.append(commit['id'])
-
-        reviewmap[commit['id']] = commit['rid'] or str(len(reviews))
-
-    lines.append('SQUASHED')
-    lines.append(commits['squashed']['diff'])
-
-    REPO.vfs.write('post_reviews', '%s\n' % '\n'.join(lines))
-    REPO.vfs.write('DUMMY_REVIEWS', '\n'.join(reviews))
-
-    return parentid, reviewmap
-
-def extsetup(ui):
-    extensions.wrapfunction(hgrb.shared, 'post_reviews', post_reviews)
-
-def reposetup(ui, repo):
-    global REPO
-    REPO = repo
--- a/hgext/reviewboard/tests/helpers.sh
+++ b/hgext/reviewboard/tests/helpers.sh
@@ -1,39 +1,40 @@
 serverconfig() {
   cat >> $1 << EOF
 [phases]
 publish = False
 
+[ui]
+ssh = python "$TESTDIR/pylib/mercurial-support/dummyssh"
+
 [web]
 push_ssl = False
 allow_push = *
 
 [reviewboard]
-url = http://dummy
+url = http://localhost:$2
 repoid = 1
 
 [extensions]
 reviewboard = $TESTDIR/hgext/reviewboard/server.py
 
 EOF
 }
 
 clientconfig() {
   cat >> $1 << EOF
 [ui]
 ssh = python "$TESTDIR/pylib/mercurial-support/dummyssh"
 
 [bugzilla]
-username = user
-password = pass
+username = testadmin
+password = password
 
 [extensions]
 reviewboard = $TESTDIR/hgext/reviewboard/client.py
 
 EOF
 }
 
-removeserverstate() {
-  rm $1/.hg/DUMMY_REVIEWS
-  rm $1/.hg/post_reviews
-
+rbmanage() {
+  python $TESTDIR/hgext/reviewboard/tests/rbmanage.py $1 $2 $3 $4 $5
 }
new file mode 100644
--- /dev/null
+++ b/hgext/reviewboard/tests/rbmanage.py
@@ -0,0 +1,150 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import os
+import subprocess
+import sys
+import time
+
+SETTINGS_LOCAL = """
+from __future__ import unicode_literals
+
+import os
+
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': 'reviewboard.db',
+        'USER': '',
+        'PASSWORD': '',
+        'HOST': '',
+        'PORT': '',
+    },
+}
+
+LOCAL_ROOT = os.path.abspath(os.path.dirname(__file__))
+PRODUCTION = False
+
+SECRET_KEY = "mbr7-l=uhl)rnu_dgl)um$62ad2ay=xw+$oxzo_ct!$xefe780"
+TIME_ZONE = 'UTC'
+LANGUAGE_CODE = 'en-us'
+SITE_ID = 1
+USE_I18N = True
+LDAP_TLS = False
+LOGGING_ENABLED = True
+LOGGING_LEVEL = "DEBUG"
+LOGGING_DIRECTORY = "."
+LOGGING_ALLOW_PROFILING = True
+DEBUG = True
+INTERNAL_IPS = "127.0.0.1"
+
+""".strip()
+
+
+def main(args):
+    path, action = args[0:2]
+    path = os.path.abspath(path)
+    sys.path.insert(0, path)
+
+    env = os.environ.copy()
+    env['PYTHONPATH'] = '%s:%s' % (path, env.get('PYTHONPATH', ''))
+    os.environ['DJANGO_SETTINGS_MODULE'] = 'reviewboard.settings'
+    manage = [sys.executable, '-m', 'reviewboard.manage']
+
+    if not os.path.exists(path):
+        os.mkdir(path)
+    os.chdir(path)
+
+    if action == 'create':
+        with open(os.path.join(path, 'settings_local.py'), 'wb') as fh:
+            fh.write(SETTINGS_LOCAL)
+
+        # TODO figure out how to suppress logging when invoking via native
+        # Python API.
+        f = open(os.devnull, 'w')
+        subprocess.check_call(manage + ['syncdb', '--noinput'], cwd=path,
+                env=env, stdout=f, stderr=f)
+
+        subprocess.check_call(manage + ['enable-extension',
+            'rbbz.extension.BugzillaExtension'], cwd=path,
+            env=env, stdout=f, stderr=f)
+
+        subprocess.check_call(manage + ['createsuperuser', '--username',
+            'testadmin', '--email', 'testadmin@example.com', '--noinput'], cwd=path,
+            env=env, stderr=f, stdout=f)
+
+        from django.contrib.auth.models import User
+        u = User.objects.get(username__exact='testadmin')
+        u.set_password('password')
+        u.save()
+
+        from reviewboard.cmdline.rbsite import Site, parse_options
+        class dummyoptions(object):
+            no_input = True
+            site_root = '/'
+            db_type = 'sqlite3'
+            copy_media = True
+
+        site = Site(path, dummyoptions())
+        site.rebuild_site_directory()
+
+        from djblets.siteconfig.models import SiteConfiguration
+        sc = SiteConfiguration.objects.get_current()
+        sc.set('site_static_root', os.path.join(path, 'htdocs', 'static'))
+        sc.set('site_media_root', os.path.join(path, 'htdocs', 'media'))
+        sc.save()
+
+    elif action == 'repo':
+        name, url = args[2:]
+
+        from reviewboard.scmtools.models import Repository, Tool
+        tool = Tool.objects.get(name__exact='Mercurial')
+        r = Repository(name=name, path=url, tool=tool)
+        r.save()
+    elif action == 'start':
+        port = args[2]
+
+        env['HOME'] = path
+        f = open(os.devnull, 'w')
+        proc = subprocess.Popen(manage + ['runserver', port],
+            cwd=path, env=env, stderr=f, stdout=f)
+
+        with open(os.path.join(path, 'server.pid'), 'wb') as fh:
+            fh.write('%d\n' % proc.pid)
+
+        # There is a race condition between us exiting and the tests
+        # querying the server before it is ready.
+        # TODO poll instead.
+        time.sleep(5)
+
+        pid = os.fork()
+        if pid > 0:
+            sys.exit(0)
+
+        os.chdir('/')
+        os.setsid()
+        os.umask(0)
+
+        pid = os.fork()
+        if pid > 0:
+            sys.exit(0)
+
+        sys.stdout.flush()
+        sys.stderr.flush()
+
+        os.dup2(f.fileno(), sys.stdin.fileno())
+        os.dup2(f.fileno(), sys.stdout.fileno())
+        os.dup2(f.fileno(), sys.stderr.fileno())
+
+        # And we spin forever.
+        try:
+            proc.wait()
+        except Exception as e:
+            print(e)
+            sys.exit(1)
+
+        sys.exit(0)
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv[1:]))
--- a/hgext/reviewboard/tests/test-errors.t
+++ b/hgext/reviewboard/tests/test-errors.t
@@ -1,20 +1,21 @@
+  $ . $TESTDIR/hgext/reviewboard/tests/helpers.sh
   $ hg init client
   $ hg init server
 
   $ cat >> server/.hg/hgrc <<EOF
   > [web]
   > push_ssl = False
   > allow_push = *
   > [extensions]
   > EOF
   $ echo "reviewboard=$(echo $TESTDIR)/hgext/reviewboard/server.py" >> server/.hg/hgrc
 
-Server should complain if the extension is not configured
+Sserver should complain if the extension is not configured
 
   $ hg -R server identify
   abort: Please set reviewboard.url to the URL of the Review Board instance to talk to.
   [255]
 
   $ echo "[reviewboard]" >> server/.hg/hgrc
   $ echo "url = http://localhost/" >> server/.hg/hgrc
   $ hg -R server identify
--- a/hgext/reviewboard/tests/test-obsolescence.t
+++ b/hgext/reviewboard/tests/test-obsolescence.t
@@ -1,22 +1,27 @@
   $ . $TESTDIR/hgext/reviewboard/tests/helpers.sh
   $ hg init client
   $ hg init server
-  $ serverconfig server/.hg/hgrc
+  $ rbmanage rbserver create
+  $ rbmanage rbserver repo test-repo http://localhost:$HGPORT2/
+  $ rbmanage rbserver start $HGPORT
+  $ cat rbserver/server.pid >> $DAEMON_PIDS
+  $ serverconfig server/.hg/hgrc $HGPORT
   $ clientconfig client/.hg/hgrc
+  $ hg serve -R server -d -p $HGPORT2 --pid-file hg.pid
+  $ cat hg.pid >> $DAEMON_PIDS
 
   $ cat > obs.py << EOF
   > import mercurial.obsolete
   > mercurial.obsolete._enabled = True
   > EOF
 
   $ echo "rebase=" >> client/.hg/hgrc
   $ echo "obs=$TESTTMP/obs.py" >> client/.hg/hgrc
-  $ echo "server_monkeypatch = ${TESTDIR}/hgext/reviewboard/tests/dummy_rbpost.py" >> server/.hg/hgrc
 
 Set up the repo
 
   $ cd client
   $ echo 'foo' > foo
   $ hg commit -A -m 'root commit'
   adding foo
   $ hg phase --public -r .
@@ -28,20 +33,20 @@ Set up the repo
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 2 changesets with 2 changes to 1 files
   submitting 1 changesets for review
   
   changeset:  1:c3480b3f6944
   summary:    foo2
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT/r/2
   
   review id:  bz://123
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT/r/1
 
 Now create a new head and push a rebase
 
   $ hg up -r 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ echo 'bar' > bar
   $ hg commit -A -m 'bar'
   adding bar
@@ -55,16 +60,16 @@ Now create a new head and push a rebase
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 2 changesets with 1 changes to 1 files (+1 heads)
   submitting 2 changesets for review
   
   changeset:  2:e7315a409763
   summary:    bar
-  review:     http://dummy/r/3
+  review:     http://localhost:$HGPORT/r/3
   
   changeset:  3:5003cd557db3
   summary:    foo2
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT/r/2
   
   review id:  bz://123
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT/r/1
--- a/hgext/reviewboard/tests/test-push.t
+++ b/hgext/reviewboard/tests/test-push.t
@@ -1,20 +1,23 @@
   $ . $TESTDIR/hgext/reviewboard/tests/helpers.sh
   $ hg init client
   $ hg init server
-  $ serverconfig server/.hg/hgrc
+  $ rbmanage rbserver create
+  $ rbmanage rbserver repo test-repo http://localhost:$HGPORT/
+  $ rbmanage rbserver start $HGPORT1
+  $ cat rbserver/server.pid >> $DAEMON_PIDS
+  $ serverconfig server/.hg/hgrc $HGPORT1
   $ clientconfig client/.hg/hgrc
 
   $ cat > obs.py << EOF
   > import mercurial.obsolete
   > mercurial.obsolete._enabled = True
   > EOF
 
-  $ echo "server_monkeypatch = ${TESTDIR}/hgext/reviewboard/tests/dummy_rbpost.py" >> server/.hg/hgrc
   $ cat >> client/.hg/hgrc << EOF
   > mq=
   > rebase=
   > EOF
 
   $ hg serve -R server -d -p $HGPORT --pid-file hg.pid
   $ cat hg.pid >> $DAEMON_PIDS
 
@@ -34,26 +37,24 @@ Set up the repo
   $ hg up -r 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg bookmark bookmark-1
   $ echo 'bookmark-1' > foo
   $ hg commit -m 'bookmark with single commit'
   created new head
   $ hg up -r 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  (leaving bookmark bookmark-1)
   $ hg bookmark bookmark-2
   $ echo 'bookmark-2a' > foo
   $ hg commit -m 'bookmark with 2 commits, 1st'
   created new head
   $ echo 'bookmark-2b' > foo
   $ hg commit -m 'bookmark with 2 commits, 2nd'
   $ hg up -r 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  (leaving bookmark bookmark-2)
   $ hg branch test-branch
   marked working directory as branch test-branch
   (branches are permanent and global, did you want a bookmark?)
   $ echo 'branch' > foo
   $ hg commit -m 'branch with single commit'
   $ hg up -r 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
@@ -76,43 +77,42 @@ Pushing a single changeset will initiate
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
   submitting 1 changesets for review
   
   changeset:  1:6f06b4ac6efe
   summary:    anonymous head
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/2
   
   review id:  bz://345
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT1/r/1
 
 {reviewurl} template works
 
   $ hg log -r 0::1 --template '{node|short} {reviewurl}\n'
   3a9f6899ef84 
-  6f06b4ac6efe http://dummy/r/2
+  6f06b4ac6efe http://localhost:$HGPORT1/r/2
 
 Pushing no changesets will do a re-review
 
   $ hg push -r 1 --reviewid 345 http://localhost:$HGPORT
   pushing to http://localhost:$HGPORT/
   searching for changes
   no changes found
   submitting 1 changesets for review
   
   changeset:  1:6f06b4ac6efe
   summary:    anonymous head
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/2
   
   review id:  bz://345
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT1/r/1
   [1]
-  $ removeserverstate ../server
 
 Pushing patches from mq will result in a warning
 
   $ echo 'mq patch' > foo
   $ hg qnew -m 'mq patch' -d '0 0' patch1
   $ hg push -r . --reviewid 784841 http://localhost:$HGPORT
   pushing to http://localhost:$HGPORT/
   searching for changes
@@ -120,187 +120,111 @@ Pushing patches from mq will result in a
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files (+1 heads)
   You are using mq to develop patches. * (glob)
   submitting 1 changesets for review
   
   changeset:  7:7458cff9569f
   summary:    mq patch
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/4
   
   review id:  bz://784841
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT1/r/3
 
   $ hg qpop
   popping patch1
   patch queue now empty
 
 Custom identifier will create a new review from same changesets.
 
   $ hg push -r 1 --reviewid 3452 http://localhost:$HGPORT
   pushing to http://localhost:$HGPORT/
   searching for changes
   no changes found
   submitting 1 changesets for review
   
   changeset:  1:6f06b4ac6efe
   summary:    anonymous head
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/2
   
   review id:  bz://3452
-  review url: http://dummy/r/3
+  review url: http://localhost:$HGPORT1/r/5
   [1]
 
-  $ removeserverstate ../server
-
 SSH works
 
   $ hg push -r 2 ssh://user@dummy/$TESTTMP/server
   pushing to ssh://user@dummy/$TESTTMP/server
   searching for changes
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files (+1 heads)
   submitting 1 changesets for review
   
   changeset:  2:a21bef69f0d4
   summary:    Bug 123 - Test identifier
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/7
   
   review id:  bz://123
-  review url: http://dummy/r/1
-  $ removeserverstate ../server
+  review url: http://localhost:$HGPORT1/r/6
 
 A single diff is generated properly
 
   $ hg up bookmark-1
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  (activating bookmark bookmark-1)
   $ hg push --reviewid bz://789213 ssh://user@dummy/$TESTTMP/server
   pushing to ssh://user@dummy/$TESTTMP/server
   searching for changes
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files (+1 heads)
   submitting 1 changesets for review
   
   changeset:  3:afef2b530106
   summary:    bookmark with single commit
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/9
   
   review id:  bz://789213
-  review url: http://dummy/r/1
-
-  $ cat ../server/.hg/post_reviews
-  url: http://dummy
-  username: user
-  password: pass
-  repoid: 1
-  identifier: bz://789213
-  0
-  afef2b530106d00832a59244a852230bd88a70a7
-  bookmark with single commit
-  diff -r 3a9f6899ef84 -r afef2b530106 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -foo
-  +bookmark-1
-  
-  NO PARENT DIFF
-  SQUASHED
-  diff -r 3a9f6899ef84 -r afef2b530106 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -foo
-  +bookmark-1
-  
-
-  $ removeserverstate ../server
+  review url: http://localhost:$HGPORT1/r/8
 
 Test that multiple changesets result in parent diffs
 
   $ hg up bookmark-2
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  (activating bookmark bookmark-2)
   $ hg push -B bookmark-2 --reviewid 567 ssh://user@dummy/$TESTTMP/server
   pushing to ssh://user@dummy/$TESTTMP/server
   searching for changes
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 2 changesets with 2 changes to 1 files (+1 heads)
   submitting 2 changesets for review
   
   changeset:  4:773ae5edc399
   summary:    bookmark with 2 commits, 1st
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/11
   
   changeset:  5:659bcc59ed36
   summary:    bookmark with 2 commits, 2nd
-  review:     http://dummy/r/3
+  review:     http://localhost:$HGPORT1/r/12
   
   review id:  bz://567
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT1/r/10
   exporting bookmark bookmark-2
 
-  $ cat ../server/.hg/post_reviews
-  url: http://dummy
-  username: user
-  password: pass
-  repoid: 1
-  identifier: bz://567
-  0
-  773ae5edc39985853a8f396765fd5b65e951cbc4
-  bookmark with 2 commits, 1st
-  diff -r 3a9f6899ef84 -r 773ae5edc399 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -foo
-  +bookmark-2a
-  
-  NO PARENT DIFF
-  1
-  659bcc59ed36f1a82f17545c97d0322b16422d5b
-  bookmark with 2 commits, 2nd
-  diff -r 773ae5edc399 -r 659bcc59ed36 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -bookmark-2a
-  +bookmark-2b
-  
-  diff -r 3a9f6899ef84 -r 773ae5edc399 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -foo
-  +bookmark-2a
-  
-  SQUASHED
-  diff -r 3a9f6899ef84 -r 659bcc59ed36 foo
-  --- a/foo	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/foo	Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -foo
-  +bookmark-2b
-  
-
 Specifying multiple -r for the same head works
 
   $ hg push -r 0 -r 1 --reviewid 50000 ssh://user@dummy/$TESTTMP/server
   pushing to ssh://user@dummy/$TESTTMP/server
   searching for changes
   no changes found
   submitting 1 changesets for review
   
   changeset:  1:6f06b4ac6efe
   summary:    anonymous head
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT1/r/2
   
   review id:  bz://50000
-  review url: http://dummy/r/4
+  review url: http://localhost:$HGPORT1/r/13
   [1]
--- a/hgext/reviewboard/tests/test-store.t
+++ b/hgext/reviewboard/tests/test-store.t
@@ -1,47 +1,44 @@
   $ . $TESTDIR/hgext/reviewboard/tests/helpers.sh
   $ hg init client
   $ hg init server
-
-  $ serverconfig server/.hg/hgrc
-  $ cat >> server/.hg/hgrc << EOF
-  > server_monkeypatch = $TESTDIR/hgext/reviewboard/tests/dummy_rbpost.py
-  > EOF
+  $ rbmanage rbserver create
+  $ rbmanage rbserver repo test-repo http://localhost:$HGPORT1
+  $ rbmanage rbserver start $HGPORT
+  $ cat rbserver/server.pid >> $DAEMON_PIDS
+  $ hg serve -R server -d -p $HGPORT1 --pid-file hg.pid
+  $ cat hg.pid >> $DAEMON_PIDS
 
+  $ serverconfig server/.hg/hgrc $HGPORT
   $ clientconfig client/.hg/hgrc
 
-  $ cat >> client/.hg/hgrc << EOF
-  > [paths]
-  > default-push = ssh://user@dummy/$TESTTMP/server
-  > EOF
-
 Pushing a review will create the reviews file
 
   $ cd client
   $ echo "dummy" > foo
   $ hg commit -A -m 'initial commit'
   adding foo
   $ hg phase --public -r .
 
   $ echo "foo" >> foo
   $ hg commit -m 'Bug 456 - second commit'
-  $ hg push
+  $ hg push ssh://user@dummy/$TESTTMP/server
   pushing to ssh://user@dummy/$TESTTMP/server
   searching for changes
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 2 changesets with 2 changes to 1 files
   submitting 1 changesets for review
   
   changeset:  1:7f387c765e68
   summary:    Bug 456 - second commit
-  review:     http://dummy/r/2
+  review:     http://localhost:$HGPORT/r/2
   
   review id:  bz://456
-  review url: http://dummy/r/1
+  review url: http://localhost:$HGPORT/r/1
 
   $ cat .hg/reviews
-  u http://dummy
+  u http://localhost:$HGPORT
   p bz://456 1
   c 7f387c765e685da95d7a4ffab2ccf06548c06fcf 2
   pc 7f387c765e685da95d7a4ffab2ccf06548c06fcf 1
--- a/pylib/reviewboardmods/reviewboardmods/pushhooks.py
+++ b/pylib/reviewboardmods/reviewboardmods/pushhooks.py
@@ -71,112 +71,139 @@ def post_reviews(url, repoid, identifier
             "extra_data.p2rb.identifier": identifier,
             "commit_id": identifier,
             "repository": repoid,
         }
         squashed_rr = rrs.create(data=data)
 
     squashed_rr.get_diffs().upload_diff(commits["squashed"]["diff"])
 
+    def update_review(rid, commit):
+        rr = api_root.get_review_request(review_request_id=rid)
+        draft = rr.get_or_create_draft(
+            commit_id=commit['id'],
+            summary=commit['message'].splitlines()[0],
+            description=commit['message'])
+        rr.get_diffs().upload_diff(commit['diff'],
+                                   parent_diff=commit['parent_diff'])
+
     previous_commits = get_previous_commits(squashed_rr)
+    remaining_nodes = {t[0]: t[1] for i, t in enumerate(previous_commits)}
+    unclaimed_rids = [t[1] for t in previous_commits]
+    processed_nodes = set()
+    node_to_rid = {}
+
+    # Do a pass and find all commits that map cleanly to old reviews.
+    for commit in commits['individual']:
+        node = commit['id']
 
-    # Create/update the individual commit review requests. Currently
-    # we will update them in push order, with no thought to history
-    # rewrites which reordered, squashed, or deleted commits.
-    #
-    # TODO: Handle rebasing using information provided to us. This
-    # most likely requires some sort of extra UI or calculations
-    # on the callers part, along with more intelligence when updating
-    # the review requests
-    reviewmap = {}
-    draft_rrs = []
-    commits_list = []
-    new_commits = []
-    individuals = commits['individual']
-    np = len(previous_commits)
-    ni = len(individuals)
-    i = 0
+        # If the commit appears in an old review, by definition of commits
+        # deriving from content, the commit has not changed and there
+        # is nothing to update.
+        # TODO handle the review ID of the commit changing. Can this happen?
+        if node in remaining_nodes:
+            rid = remaining_nodes[node]
+            del remaining_nodes[node]
+            unclaimed_rids.remove(rid)
+            processed_nodes.add(node)
+            node_to_rid[node] = rid
+            continue
 
-    for i in range(max(np, ni)):
-        pcid, rid = previous_commits[i] if i < np else (None, None)
-        commit = individuals[i] if i < ni else None
-        rr = None
+        # We haven't seen this commit before.
+
+        # The client may tell us what review it is associated with. If so,
+        # listen to the client, for they are always right.
+        if commit['rid']:
+            for n, rid in remaining_nodes.items():
+                if rid == commit['rid']:
+                    del remaining_nodes[n]
+                    unclaimed_rids.remove(rid)
+                    break
 
-        if pcid is not None and commit is not None:
-            # We have a previous commit and a new commit to
-            # update it with.
-            rr = api_root.get_review_request(review_request_id=rid)
-            draft = rr.get_or_create_draft(**{
-                "commit_id": commit["id"],
-                "summary": commit['message'].rsplit("\n", 1)[0],
-                "description": commit['message'],
-            })
-            rr.get_diffs().upload_diff(commit["diff"],
-                                       parent_diff=commit["parent_diff"])
+            update_review(commit['rid'], commit)
+            processed_nodes.add(node)
+            node_to_rid[node] = commit['rid']
+            continue
+
+    # Now do a pass over the commits that didn't map cleanly.
+    for commit in commits['individual']:
+        node = commit['id']
+        if node in processed_nodes:
+            continue
+
+        # We haven't seen this commit before *and* the client doesn't know
+        # where it belongs.
+
+        # This is where things could get complicated. We could involve
+        # heuristic based matching (comparing commit messages, changes
+        # files, etc). We may do that in the future.
 
-        elif pcid is not None and commit is None:
-            # We have a previous commit but no new commit. We need
-            # to discard this now-unused review request.
-            rr = api_root.get_review_request(review_request_id=rid)
-            rr.update(status="discarded")
-        else:
-            # There is no previous commit so we need to create one
-            # from scratch.
-            data = {
-                "extra_data.p2rb": "True",
-                "extra_data.p2rb.is_squashed": "False",
-                "extra_data.p2rb.identifier": identifier,
-                "commit_id": commit["id"],
-                "repository": repoid,
-            }
+        # For now, match the commit up against the next one in the index.
+        if unclaimed_rids:
+            assumed_old_rid = unclaimed_rids[0]
+            unclaimed_rids.pop(0)
+            update_review(assumed_old_rid, commit)
+            processed_nodes.add(commit['id'])
+            node_to_rid[node] = assumed_old_rid
+            continue
 
-            rr = rrs.create(data=data)
-            rr.get_diffs().upload_diff(commit["diff"],
-                                       parent_diff=commit["parent_diff"])
+        # There are no more unclaimed review IDs. This means we have more
+        # commits than before. Create new reviews as appropriate.
+        rr = rrs.create(data={
+            'extra_data.p2rb': 'True',
+            'extra_data.p2rb.is_squashed': 'False',
+            'extra_data.p2rb.identifier': identifier,
+            'commit_id': commit['id'],
+            'repository': repoid,
+        })
+        rr.get_diffs().upload_diff(commit['diff'],
+                                   parent_diff=commit['parent_diff'])
+        draft = rr.get_or_create_draft(
+            summary=commit['message'].splitlines()[0],
+            description=commit['message'])
+        processed_nodes.add(commit['id'])
+        node_to_rid[node] = rr.id
 
-            draft = rr.get_or_create_draft(**{
-                "summary": commit['message'].rsplit("\n", 1)[0],
-                "description": commit['message']
-            })
-
-
-        if commit is not None:
-            reviewmap[commit['id']] = rr.id
-            commits_list.append((commit['id'], rr.id))
-            draft_rrs.append(draft)
-
+    # At this point every incoming commit has been accounted for.
+    # If there are any remaining reviews, they must belong to deleted
+    # commits. (Or, we made a mistake and updated the wrong review.)
+    for rid in unclaimed_rids:
+        rr = api_root.get_review_request(review_request_id=rid)
+        rr.update(status='discared')
 
     squashed_description = []
-    for rr in draft_rrs:
-        squashed_description.append(
-            "/r/%s - %s" % (rr.id, rr.summary))
-
+    for commit in commits['individual']:
+        squashed_description.append('/r/%s - %s' % (
+            node_to_rid[commit['id']],
+            commit['message'].splitlines()[0]))
 
-    squashed_draft = squashed_rr.get_or_create_draft(**{
-        "summary": "Review for review ID: %s" % identifier,
-        "description": "%s\n" % ("\n".join(squashed_description)),
-        "depends_on": ",".join([str(rr.id) for rr in draft_rrs]),
-    })
+    depends = ','.join(str(i) for i in sorted(node_to_rid.values()))
+    squashed_draft = squashed_rr.get_or_create_draft(
+        summary='Review for review ID: %s' % identifier,
+        description='%s\n' % '\n'.join(squashed_description),
+        depends_on=depends)
+
+    commit_list = []
+    for commit in commits['individual']:
+        node = commit['id']
+        commit_list.append([node, node_to_rid[node]])
+
     squashed_rr.update(data={
-        "extra_data.p2rb.commits": json.dumps([
-            (draft.commit_id, draft.id) for draft in draft_rrs
-        ])})
+        'extra_data.p2rb.commits': json.dumps(commit_list)})
 
-    return squashed_rr.id, reviewmap
-
+    return squashed_rr.id, node_to_rid
 
 def get_previous_commits(squashed_rr):
     """Retrieve the previous commits from a squashed review request.
 
     This will return a list of tuples specifying the previous commit
     id as well as the review request it is represented by. ex::
         [
             # (<commit-id>, <review-request-id>),
             ('d4bd89322f54', '13'),
             ('373537353134', '14'),
         ]
     """
     extra_data = squashed_rr.extra_data
     commits = (
         extra_data["p2rb.commits"] if "p2rb.commits" in extra_data else "[]")
-    print commits
     return json.loads(commits)
 
--- a/run-mercurial-tests.py
+++ b/run-mercurial-tests.py
@@ -33,11 +33,18 @@ def find_test_files():
             if f.startswith('.'):
                 continue
 
             if f.startswith('test-') and f.endswith(('.py', '.t')):
                 yield os.path.join(test_dir, f)
 
 
 if __name__ == '__main__':
+    if not hasattr(sys, 'real_prefix'):
+        raise Exception('You are not running inside the virtualenv. Please '
+                'create one and `pip install -r test-requirements.txt`')
+
+    hg = os.path.join(os.path.dirname(sys.executable), 'hg')
+    sys.argv.extend(['--with-hg', hg])
+
     runner = runtestsmod.TestRunner()
     sys.argv.extend(find_test_files())
     sys.exit(runner.run(sys.argv[1:]))
new file mode 100644
--- /dev/null
+++ b/test-requirements.txt
@@ -0,0 +1,25 @@
+--find-links http://downloads.reviewboard.org/releases/Djblets/0.8/
+--find-links http://downloads.reviewboard.org/releases/django-evolution/0.7/
+git+git://github.com/mozilla/rbbz.git@master#egg=rbbz
+Django==1.6.5
+Markdown==2.4.1
+Pillow==2.4.0
+Pygments==1.6
+RBTools==0.6.1
+Whoosh==2.6.0
+django-evolution==0.7.2
+django-haystack==2.1.0
+django-pipeline==1.3.24
+docutils==0.11
+ecdsa==0.11
+feedparser==5.1.3
+futures==2.1.6
+mercurial==3.0.1
+mimeparse==0.1.3
+paramiko==1.14.0
+pillowfight==0.2
+pycrypto==2.6.1
+python-dateutil==1.5
+python-memcached==1.53
+pytz==2014.4
+recaptcha-client==1.0.6