Clear the functests search index when clearing the DB

For speed h does not normally clear the DB between functests unless the
test has `@pytest.mark.parametrize("with_clean_db")`, see:
#6845

At the time when this change was made the _search index_ was still being
cleared before each functest which makes no sense: the search API will
not return an annotation if it's not in the search index, even if the
annotation is in the DB. If you aren't going to clear the DB before each
functest, you shouldn't clear the search index either.

A later PR accidentally broke the
`always_delete_all_elasticsearch_documents` fixture so that the search
index wasn't actually being cleared between functests anyway, see:
#9636

This commit:

1. Removes the broken `always_delete_all_elasticsearch_documents`
   fixture: it doesn't do anything, and since we don't clear the DB
   between tests it doesn't make sense to clear the search index either.
2. Replaces the `with_clean_db` fixture with
   `with_clean_db_and_search_index`: it doesn't make sense to clear the
   DB without also clearing the search index.

Longer-term I think we probably want to find a fast way to clear both
the DB and search index before each functional test.

Author: seanh

tests/functional/api/bulk/action_test.py

@@ -10,7 +10,7 @@
 pytestmark = pytest.mark.usefixtures("init_elasticsearch")
 
 
-@pytest.mark.usefixtures("with_clean_db")
+@pytest.mark.usefixtures("with_clean_db_and_search_index")
 class TestBulk:
     AUTHORITY = "lms.hypothes.is"
 

tests/functional/api/bulk/annotation_test.py

@@ -5,7 +5,7 @@
 from h.models import GroupMembership
 
 
-@pytest.mark.usefixtures("with_clean_db")
+@pytest.mark.usefixtures("with_clean_db_and_search_index")
 class TestBulkAnnotation:
     def test_it_requires_authentication(self, make_request):
         response = make_request(headers={"bad_auth": "BAD"}, expect_errors=True)

tests/functional/bin/run_data_task_test.py

@@ -11,7 +11,7 @@
 
 class TestRunSQLTask:
     # We use "clean DB" here to ensure the schema is created
-    @pytest.mark.usefixtures("with_clean_db")
+    @pytest.mark.usefixtures("with_clean_db_and_search_index")
     def test_reporting_tasks(self, environ):
         for task_name in (
             # Run all the jobs in one line in the order they are expected to

tests/functional/conftest.py

@@ -57,7 +57,24 @@ def reset_app(app):
 
 
 @pytest.fixture
-def with_clean_db(db_engine):
+def with_clean_db_and_search_index(
+        db_engine,
+        # The es_client fixture clears the search index as part of its fixture
+        # teardown, so this apparently unused dependency is what actually
+        # causes this fixture to clear the search index.
+        es_client,  # noqa: F811
+):
+    """Empty the DB and search index before running the test.
+
+    h doesn't normally reset the DB or search index before each functest
+    because doing so was taking too long, see:
+    https://github.com/hypothesis/h/pull/6845
+
+    This does mean that functests need to be robust against data from previous
+    tests still being in the DB and search index. Alternatively, a test can use
+    @pytest.mark.usefixtures("with_clean_db_and_search_index") to request have
+    the DB and search index be emptied before running the test.
+    """
     tables = reversed(db.Base.metadata.sorted_tables)
     with contextlib.closing(db_engine.connect()) as conn:
         tx = conn.begin()
@@ -90,10 +107,3 @@ def factories(db_session):
 @pytest.fixture(scope="session")
 def pyramid_app():
     return create_app(None, **TEST_SETTINGS)
-
-
-# Always unconditionally wipe the Elasticsearch index after every functional
-# test.
-@pytest.fixture(autouse=True)
-def always_delete_all_elasticsearch_documents():
-    pass