Rebase + refactor tests for new download functionality

This rebases the new download unread and download all functionality in the index
view of the journalist app on the current develop, and in the process does some
minor refactoring of the test suite.

* Organizes imports: first come external package imports, then the
  SECUREDROP_ENV environment variable is set, and finally the SD module imports.
  They are now alphabetized. Also removes some unused imports, and additional
  addition or removal was done as necessary to support the refactor.

* Prefers use of APIs for object construction versus explicit assignment. For
  example, a call to :function:`crypto_util.genrandomid()` is preferred to the
  assignment `sid = 'somestring'`.

* Class-based rewrite of common.py: in order to better organize the
  functionality, and encourage better OOP and DRY practices, all the loose
  functions in `securedrop/tests/common.py` have become (mostly) static or class
  methods various organizational classes. This allowed us to write a lot of
  code.

* More accurate test method names: a lot of test method names were vaguely
  descriptive. A common motif was to just end test names in 'success.' It's
  better to name a method `test_admin_login_redirects_to_index` if it just tests
  that after successful login you're redirected to the index page, than
  `test_admin_login_success`. There are many things that should happen on
  success--:obj:`flask.g` should be set correctly, calls to the database should
  be made, session should be set in a client-side cookie, and that cookie should
  be set--the list goes on. Let's be specific about which thing that's supposed
  to happen happened successfully. (Side note: we do really need to do more
  comprehensive tests in a lot of places where we only tests that the app
  redirects correctly, instead of checking that all state and context is
  modified--or not--in the expected way).

* Better async support: The new :class:`tests.common.Async` provides a little
  bit better asynchronous functionality for a variety of situations, and
  deprecates the use of sleep statements.

* General cleanup: PEP8 stuff, and nicer formatting--some other minor touches.

securedrop/tests/common.py

@@ -1,120 +1,291 @@
+# -*- coding: utf-8 -*-
+
+from functools import wraps
+import gnupg
+import mock
 import os
 import shutil
-import uuid
 import subprocess
-
-import gnupg
+import time
 
 # Set environment variable so config.py uses a test environment
 os.environ['SECUREDROP_ENV'] = 'test'
+
 import config
-from db import init_db, db_session, Journalist, Reply, Source, Submission
+from db import db_session, init_db, Journalist, Reply, Source, Submission
 import crypto_util
+import store
 
-# TODO: the PID file for the redis worker is hard-coded below.
-# Ideally this constant would be provided by a test harness.
-# It has been intentionally omitted from `config.py.example`
-# in order to isolate the test vars from prod vars.
-# When refactoring the test suite, the TEST_WORKER_PIDFILE
-# TEST_WORKER_PIDFILE is also hard-coded in `manage.py`.
-TEST_WORKER_PIDFILE = "/tmp/securedrop_test_worker.pid"
 
+class TestJournalist:
+    """Test fixtures for working with :class:`db.Journalist`.
+    """
+    @staticmethod
+    def init_journalist(is_admin=False):
+        """Initialize a journalist into the database. Return their
+        :class:`db.Journalist` object and password string.
 
-def clean_root():
-    shutil.rmtree(config.SECUREDROP_DATA_ROOT)
+        :param bool is_admin: Whether the user is an admin.
 
+        :returns: A 2-tuple. The first entry, the :class:`db.Journalist`
+                  initialized. The second, their password string.
+        """
+        username = crypto_util.genrandomid()
+        user_pw = crypto_util.genrandomid()
+        user = Journalist(username, user_pw, is_admin)
+        db_session.add(user)
+        db_session.commit()
+        return user, user_pw
 
-def create_directories():
-    # Create directories for the file store and the GPG keyring
-    for d in (config.SECUREDROP_DATA_ROOT, config.STORE_DIR,
-              config.GPG_KEY_DIR, config.TEMP_DIR):
-        if not os.path.isdir(d):
-            os.mkdir(d)
+    @staticmethod
+    def init_admin():
+        return TestJournalist.init_journalist(True)
 
+    @staticmethod
+    def mock_verify_token(self):
+        patcher = mock.patch('db.Journalist.verify_token')
+        self.addCleanup(patcher.stop)
+        self.mock_journalist_verify_token = patcher.start()
+        self.mock_journalist_verify_token.return_value = True
 
-def init_gpg():
-    # Initialize the GPG keyring
-    gpg = gnupg.GPG(homedir=config.GPG_KEY_DIR)
-    # Import the journalist key for testing (faster to import a pre-generated
-    # key than to gen a new one every time)
-    for keyfile in ("test_journalist_key.pub", "test_journalist_key.sec"):
-        gpg.import_keys(open(keyfile).read())
-    return gpg
 
+class TestSource:
+    """Test fixtures for working with :class:`db.Source`.
+    """
+    @staticmethod
+    def init_source():
+        """Initialize a source: create their database record, the
+        filesystem directory that stores their submissions & replies,
+        and their GPG key encrypted with their codename.
 
-def create_file(filename):
-    dirname = os.path.dirname(filename)
-    if not os.path.exists(dirname):
-        os.makedirs(dirname)
-    with open(filename, 'w') as fp:
-        fp.write(str(uuid.uuid4()))
+        :returns: A 2-tuple. The first entry, the :class:`db.Source`
+        initialized. The second, their codename string.
+        """
+        # Create source identity and database record
+        codename = crypto_util.genrandomid()
+        filesystem_id = crypto_util.hash_codename(codename)
+        journalist_filename = crypto_util.display_id()
+        source = Source(filesystem_id, journalist_filename)
+        db_session.add(source)
+        db_session.commit()
+        # Create the directory to store their submissions and replies
+        os.mkdir(store.path(source.filesystem_id))
+        # Generate their key, blocking for as long as necessary
+        crypto_util.genkeypair(source.filesystem_id, codename)
 
+        return source, codename
 
-def setup_test_docs(sid, files):
-    filenames = [os.path.join(config.STORE_DIR, sid, file) for file in files]
+    # NOTE: this method is potentially dangerous to rely on for now due
+    # to the fact flask_testing.TestCase only uses on request context
+    # per method (see
+    # https://github.com/freedomofpress/securedrop/issues/1444).
+    @staticmethod
+    def new_codename(client, session):
+        """Helper function to go through the "generate codename" flow.
+        """
+        with client as c:
+            c.get('/generate')
+            codename = session['codename']
+            c.post('/create')
+        return codename
 
-    for filename in filenames:
-        create_file(filename)
 
-        # Add Submission to the db
-        source = Source.query.filter(Source.filesystem_id == sid).one()
-        submission = Submission(source, os.path.basename(filename))
-        db_session.add(submission)
+class TestSubmission:
+    """Test fixtures for working with :class:`db.Submission`.
+    """
+    @staticmethod
+    def submit(source, num_submissions):
+        """Generates and submits *num_submissions*
+        :class:`db.Submission`s on behalf of a :class:`db.Source`
+        *source*.
+
+        :param db.Source source: The source on who's behalf to make
+                                 submissions.
+
+        :param int num_submissions: Number of random-data submissions
+                                    to make.
+
+        :returns: A list of the :class:`db.Submission`s submitted.
+        """
+        assert num_submissions >= 1
+        submissions = []
+        for _ in range(num_submissions):
+            source.interaction_count += 1
+            fpath = store.save_message_submission(source.filesystem_id,
+                                                  source.interaction_count,
+                                                  source.journalist_filename,
+                                                  str(os.urandom(1)))
+            submission = Submission(source, fpath)
+            submissions.append(submission)
+            db_session.add(submission)
+
+        db_session.commit()
+        return submissions
+
+    @staticmethod
+    def mark_downloaded(*submissions):
+        for submission in submissions:
+            submission.downloaded = True
         db_session.commit()
 
-    return filenames
 
+class TestReply:
+    """Test fixtures for working with :class:`db.Reply`.
+    """
+    @staticmethod
+    def reply(journalist, source, num_replies):
+        """Generates and submits *num_replies* replies to *source*
+        :class:`db.Source`.
 
-def setup_test_replies(sid, journo_id, files):
-    filenames = [os.path.join(config.STORE_DIR, sid, file) for file in files]
+        :param db.Journalist journalist: The journalist to write the
+                                         reply from.
 
-    for filename in filenames:
-        create_file(filename)
+        :param db.Source source: The source to send the reply to.
+
+        :param int num_replies: Number of random-data replies to make.
+
+        :returns: A list of the :class:`db.Reply`s submitted.
+        """
+        assert num_replies >= 1
+        replies = []
+        for _ in range(num_replies):
+            source.interaction_count += 1
+            fname = "{}-{}-reply.gpg".format(source.interaction_count,
+                                             source.journalist_filename)
+            crypto_util.encrypt(str(os.urandom(1)),
+                                [
+                                    crypto_util.getkey(source.filesystem_id),
+                                    config.JOURNALIST_KEY
+                                ],
+                                store.path(source.filesystem_id, fname))
+            reply = Reply(journalist, source, fname)
+            replies.append(reply)
+            db_session.add(reply)
 
-        # Add Reply to the db
-        source = Source.query.filter(Source.filesystem_id == sid).one()
-        journalist = Journalist.query.filter(Journalist.id == journo_id).one()
-        reply = Reply(journalist, source, os.path.basename(filename))
-        db_session.add(reply)
         db_session.commit()
+        return replies
+
+
+class SetUp:
+    """Test fixtures for initializing a generic test environment.
+    """
+    # TODO: the PID file for the redis worker is hard-coded below.
+    # Ideally this constant would be provided by a test harness.
+    # It has been intentionally omitted from `config.py.example`
+    # in order to isolate the test vars from prod vars.
+    # When refactoring the test suite, the test_worker_pidfile
+    # test_worker_pidfile is also hard-coded in `manage.py`.
+    test_worker_pidfile = "/tmp/securedrop_test_worker.pid"
+
+    @staticmethod
+    def create_directories():
+        # Create directories for the file store and the GPG keyring
+        for d in (config.SECUREDROP_DATA_ROOT, config.STORE_DIR,
+                  config.GPG_KEY_DIR, config.TEMP_DIR):
+            if not os.path.isdir(d):
+                os.mkdir(d)
+
+    @staticmethod
+    def init_gpg():
+        # Initialize the GPG keyring
+        gpg = gnupg.GPG(homedir=config.GPG_KEY_DIR)
+        # Import the journalist key for testing (faster to import a pre-generated
+        # key than to gen a new one every time)
+        for keyfile in ("test_journalist_key.pub", "test_journalist_key.sec"):
+            gpg.import_keys(open(keyfile).read())
+        return gpg
+
+    @classmethod
+    def setup(cls):
+        """Set up the file system, GPG, and database."""
+        SetUp.create_directories()
+        SetUp.init_gpg()
+        init_db()
+        # Do tests that should always run on app startup
+        crypto_util.do_runtime_tests()
+        # Start the Python-RQ worker if it's not already running
+        if not os.path.exists(cls.test_worker_pidfile):
+            subprocess.Popen(["rqworker",
+                              "-P", config.SECUREDROP_ROOT,
+                              "--pid", cls.test_worker_pidfile])
+
+
+class TearDown:
+    """Test fixtures for tearing down a generic test environment.
+    """
+    @staticmethod
+    def teardown():
+        shutil.rmtree(config.SECUREDROP_DATA_ROOT)
+
+    # TODO: now that SD has a logout button, we can deprecate use of
+    # this function.
+    @staticmethod
+    def logout(test_client):
+        with test_client.session_transaction() as sess:
+            sess.clear()
+
+
+class Async:
+    """Test fixtures for use with asynchronous processes.
+    """
+    redis_success_return_value = 'success'
 
-    return filenames
+    @classmethod
+    def wait_for_redis_worker(cls, job, timeout=5):
+        """Raise an error if the Redis job doesn't complete successfully
+        before a timeout.
 
+        :param rq.job.Job job: A Redis job to wait for.
 
-def new_codename(client, session):
-    """Helper function to go through the "generate codename" flow"""
-    with client as c:
-        rv = c.get('/generate')
-        codename = session['codename']
-        rv = c.post('/create')
-    return codename
+        :param int timeout: Seconds to wait for the job to finish.
 
+        :raises: An :exc:`AssertionError`.
+        """
+        start_time = time.time()
+        while time.time() - start_time < timeout:
+            if job.result == cls.redis_success_return_value:
+                return
+            elif job.result not in (None, cls.redis_success_return_value):
+                assert False, 'Redis worker failed!'
+        assert False, 'Redis worker timed out!'
 
-def shared_setup():
-    """Set up the file system, GPG, and database"""
-    create_directories()
-    init_gpg()
-    init_db()
+    @staticmethod
+    def wait_for_function_result(f, timeout=5):
+        """Wraps function *f*: calls *f* until *f* returns a non-None
+        value, or timeout is reached.
 
-    # Do tests that should always run on app startup
-    crypto_util.do_runtime_tests()
+        :param function f: The function to run.
 
-    # Start the Python-RQ worker if it's not already running
-    if not os.path.exists(TEST_WORKER_PIDFILE):
-        subprocess.Popen(["rqworker", "-P", config.SECUREDROP_ROOT,
-                                      "--pid", TEST_WORKER_PIDFILE])
+        :param int timeout: Seconds to wait for the function to return.
 
+        :raises: An :exc:`AssertionError`.
+        """
+        @wraps(f)
+        def wrapper(*args, **kwargs):
+            start_time = time.time()
+            while time.time() - start_time < timeout:
+                result = f(*args, **kwargs)
+                if result is not None:
+                    return result
+            assert False, 'Asynchronous function timeout!'
+        return wrapper
 
-def shared_teardown():
-    clean_root()
+    @staticmethod
+    def wait_for_assertion(assertion_expression, timeout=5):
+        """Calls an assertion_expression repeatedly, until the assertion
+        passes or a timeout is reached.
 
+        :param assertion_expression: An assertion expression. Generally
+                                     a call to a
+                                     :class:`unittest.TestCase` method.
 
-def logout(test_client):
-    # See http://flask.pocoo.org/docs/testing/#accessing-and-modifying-sessions
-    # This is necessary because SecureDrop doesn't have a logout button, so a
-    # user is logged in until they close the browser, which clears the session.
-    # For testing, this function simulates closing the browser at places
-    # where a source is likely to do so (for instance, between submitting a
-    # document and checking for a journalist reply).
-    with test_client.session_transaction() as sess:
-        sess.clear()
+        :param int timeout: Seconds to wait for the function to return.
+        """
+        start_time = time.time()
+        while time.time() - start_time < timeout:
+            try:
+                return assertion_expression()
+            except AssertionError:
+                pass
+        # one more try, which will raise any errors if they are outstanding
+        return assertion_expression()