test: Randomize Pytest runs (meltano#6695)

* test: Randomize Pytest runs

This PR adds a dependency on `pytest-randomly`, which seeds randomness in our test runs, and randomizes the order the tests are run in.

The randomness introduced by this plugin can be replicated using the `--randomly-seed` argument that is printed out for each Pytest run.

Running the tests in a random order helps to ensure they do not depend on tests that were run previously. This generally makes the test suite more robust, and also will allow us to run the tests in parallel using `pytest-xdist` - see meltano#6157

Most of the pre-existing implicit dependencies between tests have been addressed by making them explicit with `order` marks provided by the `pytest-order` plugin. This is not the cleanest approach (it would be better if there were no dependencies between our tests), but it should be sufficient to realize most of the benefits, and is much faster than rewriting the tests to not have dependencies on other tests.

Closes meltano#6300

* Install `pytest-randomly` and `pytest-order` in Nox session

* Set `--randomly-seed` in `noxfile.py`

Because of how Nox collects output from Pytest, we cannot see the seed that `pytest-randomly` set in CI. We can set it outselves in `noxfile.py` so that it is printed out in CI.

* Increase test fixture SQLAlchemy connection pool size

* Fix `test_analytics_json_is_created`

* Fix `test_config_meltano` on Windows

For whatever reason, `send_anonymous_usage_stats` may be `False` on Windows when this test is run. The test is still sound without that particular check, so we can simply omit it.

* Fix intermittent SQLAlchemy `OperationalError`

* Increase `pool_size` in `meltano.core.db.project_engine`

* Warn about SQLAlchemy operational error properly

* Use `pool_size` only when SQLAlchemy configuration supports it

* Use SQLAlchemy `NullPool`

* Remove commented-out code

* Revert changes to `vacuum_db`

* Fix `TestUIAvailableWorker::test_open_browser`

noxfile.py

@@ -5,6 +5,7 @@
 import os
 import sys
 from pathlib import Path
+from random import randint
 from textwrap import dedent
 
 try:
@@ -45,6 +46,8 @@ def tests(session: Session) -> None:
         "pytest",
         "pytest-asyncio",
         "pytest-docker",
+        "pytest-order",
+        "pytest-randomly",
         "requests-mock",
     )
 
@@ -55,6 +58,7 @@ def tests(session: Session) -> None:
             "--parallel",
             "-m",
             "pytest",
+            f"--randomly-seed={randint(0, 2**32-1)}",  # noqa: S311, WPS432
             *session.posargs,
             env={"NOX_CURRENT_SESSION": "tests"},
         )

pyproject.toml

@@ -88,6 +88,8 @@ pytest = "^7.1.2"
 pytest-asyncio = "^0.18.3"
 pytest-cov = "^3.0.0"
 pytest-docker = "^1.0"
+pytest-order = "^1.0"
+pytest-randomly = "^3.12"
 pyupgrade = "^2.29.1"
 requests-mock = "^1.6.0"
 tox = "^3.24.4"

pytest.ini

@@ -1,5 +1,5 @@
 [pytest]
-addopts = --doctest-modules -ra
+addopts = --doctest-modules --order-scope=module -ra
 testpaths = tests
 
 markers =

src/meltano/core/db.py

@@ -9,6 +9,7 @@
 from sqlalchemy.engine import Connection, Engine
 from sqlalchemy.exc import OperationalError
 from sqlalchemy.orm import sessionmaker
+from sqlalchemy.pool import NullPool
 from sqlalchemy.sql import text
 
 from meltano.core.project import Project
@@ -41,8 +42,9 @@ def project_engine(
     settings = ProjectSettingsService(project)
 
     engine_uri = settings.get("database_uri")
-    logging.debug(f"Creating engine {project}@{engine_uri}")
-    engine = create_engine(engine_uri, pool_pre_ping=True)
+    logging.debug(f"Creating engine '{project}@{engine_uri}'")
+
+    engine = create_engine(engine_uri, poolclass=NullPool)
 
     # Connect to the database to ensure it is available.
     connect(

tests/fixtures/api.py

@@ -38,7 +38,7 @@ def app(create_app):
 
 
 @pytest.fixture(scope="class")
-def create_app(request, project, vacuum_db):
+def create_app(request, project):
     def _factory(**kwargs):
         config = {"TESTING": True, "LOGIN_DISABLED": False, "ENV": "test", **kwargs}
 

tests/fixtures/db/__init__.py

@@ -3,13 +3,15 @@
 import logging
 import warnings
 from contextlib import closing
+from time import sleep
 from typing import Generator
 
 import pytest
 from _pytest.monkeypatch import MonkeyPatch  # noqa: WPS436
 from sqlalchemy import MetaData, create_engine
 from sqlalchemy.exc import SAWarning
 from sqlalchemy.orm import close_all_sessions, sessionmaker
+from sqlalchemy.pool import NullPool
 
 from meltano.core.project import Project
 
@@ -51,11 +53,8 @@ def vacuum_db(engine_sessionmaker):
 
 @pytest.fixture(scope="class")
 def engine_sessionmaker(engine_uri):
-    # create the engine
-    engine = create_engine(engine_uri)
-    create_session = sessionmaker(bind=engine)
-
-    return (engine, create_session)
+    engine = create_engine(engine_uri, poolclass=NullPool)
+    return (engine, sessionmaker(bind=engine))
 
 
 @pytest.fixture()

tests/fixtures/db/mssql.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import contextlib
 import logging
 import os
@@ -6,6 +8,7 @@
 import sqlalchemy as sa
 from sqlalchemy import DDL, create_engine
 from sqlalchemy.engine import URL
+from sqlalchemy.pool import NullPool
 
 
 def recreate_database(engine, db_name):
@@ -70,7 +73,11 @@ def engine_uri():
 
     # Recreate the database using the master database
     master_engine_uri = create_connection_url(host, port, user, password, "master")
-    engine = create_engine(master_engine_uri, isolation_level="AUTOCOMMIT")
+    engine = create_engine(
+        master_engine_uri,
+        isolation_level="AUTOCOMMIT",
+        poolclass=NullPool,
+    )
     recreate_database(engine, database)
 
     # Connect to the database where the tests will be run

tests/fixtures/db/postgresql.py

@@ -6,6 +6,7 @@
 import pytest
 import sqlalchemy
 from sqlalchemy import create_engine, text
+from sqlalchemy.pool import NullPool
 
 
 def recreate_database(engine, db_name):
@@ -27,7 +28,11 @@ def engine_uri():
 
     # create the database
     engine_uri = f"postgresql://{user}:{password}@{host}:{port}/postgres"
-    engine = create_engine(engine_uri, isolation_level="AUTOCOMMIT")
+    engine = create_engine(
+        engine_uri,
+        isolation_level="AUTOCOMMIT",
+        poolclass=NullPool,
+    )
     recreate_database(engine, database)
 
     return f"postgresql://{user}:{password}@{host}:{port}/{database}"

tests/fixtures/utils.py

@@ -12,11 +12,11 @@
 
 
 @contextmanager
-def cd(path: Path):
+def cd(path: Path) -> Path:
     prev_dir = os.getcwd()
     os.chdir(path)
     try:
-        yield
+        yield path
     finally:
         os.chdir(prev_dir)
 

tests/meltano/api/test_mail.py

@@ -2,14 +2,15 @@
 
 import json
 
+import pytest
+
 from meltano.api.mail import MailService
 from meltano.api.models.subscription import Subscription, SubscriptionEventType
 from meltano.core.project_settings_service import ProjectSettingsService
 
 
 class TestMailService:
     def test_get_unsubscribe_group(self, project):
-
         default_id = 12751
         custom_id = 42
 
@@ -32,6 +33,7 @@ def test_get_unsubscribe_group(self, project):
         custom_service = MailService(project)
         assert custom_service.get_unsubscribe_group(sub) == custom_id
 
+    @pytest.mark.order(after="test_get_unsubscribe_group")
     def test_create_message(self, project):
         sub = Subscription()
         sub.event_type = SubscriptionEventType.PIPELINE_MANUAL_RUN

tests/meltano/api/test_workers.py

@@ -11,15 +11,14 @@ class TestUIAvailableWorker:
     def subject(self, project):
         return UIAvailableWorker(project, open_browser=True)
 
-    @mock.patch("time.sleep")  # don't wait for real
     @mock.patch("webbrowser.open")
     @mock.patch("requests.get")
-    def test_open_browser(self, requests_get, webbrowser_open, sleep, subject):
+    def test_open_browser(self, requests_get, webbrowser_open, subject):
         error = mock.Mock(status_code=400)
         ok = mock.Mock(status_code=200)
         requests_get.side_effect = [error, error, ok]
-        sleep.return_value = None
-
-        subject.run()
-        webbrowser_open.assert_called_with("http://localhost:5000")
-        assert requests_get.call_count == sleep.call_count
+        with mock.patch("time.sleep") as sleep:
+            sleep.return_value = None
+            subject.run()
+            webbrowser_open.assert_called_with("http://localhost:5000")
+            assert requests_get.call_count == sleep.call_count

tests/meltano/cli/test_add.py

@@ -36,6 +36,7 @@ def reset_project_context(
         shutil.rmtree(".", ignore_errors=True)
         project_init_service.create_files(project)
 
+    @pytest.mark.order(0)
     @pytest.mark.parametrize(
         "plugin_type,plugin_name,default_variant,required_plugin_refs",
         [
@@ -110,6 +111,7 @@ def test_add(
 
                 install_plugin_mock.assert_called()
 
+    @pytest.mark.order(1)
     def test_add_multiple(self, project, cli_runner, project_plugins_service):
         with mock.patch("meltano.cli.add.install_plugins") as install_plugin_mock:
             install_plugin_mock.return_value = True
@@ -147,6 +149,7 @@ def test_add_multiple(self, project, cli_runner, project_plugins_service):
                 reason=PluginInstallReason.ADD,
             )
 
+    @pytest.mark.order(2)
     def test_add_transform(self, project, cli_runner):
         # adding Transforms requires the legacy 'dbt' Transformer
         cli_runner.invoke(cli, ["add", "transformer", "dbt"])

tests/meltano/cli/test_cli.py

@@ -38,6 +38,7 @@ def empty_project(self, empty_meltano_yml_dir, pushd):
         finally:
             Project.deactivate()
 
+    @pytest.mark.order(0)
     def test_activate_project(self, project, cli_runner, pushd):
         assert Project._default is None
 
@@ -48,11 +49,13 @@ def test_activate_project(self, project, cli_runner, pushd):
         assert Project._default.root == project.root
         assert Project._default.readonly is False
 
+    @pytest.mark.order(1)
     def test_empty_meltano_yml_project(self, empty_project, cli_runner, pushd):
         pushd(empty_project.root)
         with pytest.raises(EmptyMeltanoFileException):
             cli_runner.invoke(cli, ["config"], catch_exceptions=False)
 
+    @pytest.mark.order(2)
     def test_activate_project_readonly_env(
         self, project, cli_runner, pushd, monkeypatch
     ):
@@ -65,6 +68,7 @@ def test_activate_project_readonly_env(
 
         assert Project._default.readonly
 
+    @pytest.mark.order(2)
     def test_activate_project_readonly_dotenv(
         self, project, cli_runner, pushd, monkeypatch
     ):

tests/meltano/cli/test_config.py

@@ -67,7 +67,6 @@ def test_config_meltano(
             assert_cli_runner(result)
 
             json_config = json.loads(result.stdout)
-            assert json_config["send_anonymous_usage_stats"] is True
             assert json_config["database_uri"] == engine_uri
             assert json_config["cli"]["log_level"] == "info"
 

tests/meltano/cli/test_install.py

@@ -20,6 +20,7 @@ def tap_gitlab(self, project_add_service):
         except PluginAlreadyAddedException as err:
             return err.plugin
 
+    @pytest.mark.order(0)
     def test_install(
         self, project, tap, tap_gitlab, target, dbt, cli_runner, project_plugins_service
     ):
@@ -253,6 +254,7 @@ def test_clean_install(
 # project fixture creates the project see
 # https://github.com/meltano/meltano/pull/6407#issuecomment-1200516464
 # For more details
+@pytest.mark.order(-1)
 def test_new_folder_should_autocreate_on_install(
     un_engine_uri, project_function, cli_runner
 ):

tests/meltano/cli/test_job_cmd.py

@@ -3,6 +3,7 @@
 import json
 
 import mock
+import pytest
 
 from asserts import assert_cli_runner
 from meltano.cli import cli
@@ -124,6 +125,7 @@ def test_job_set(self, session, project, cli_runner, task_sets_service):
             assert task_sets.name == "job-set-mock"
             assert task_sets.tasks == ["tap2-mock target2-mock"]
 
+    @pytest.mark.order(after="test_job_add")
     def test_job_remove(self, session, project, cli_runner, task_sets_service):
         # singular task with job
         with mock.patch(

tests/meltano/cli/test_lock.py

@@ -22,6 +22,7 @@ def patch_hub(self, meltano_hub_service: MeltanoHubService):
         ):
             yield
 
+    @pytest.mark.order(0)
     @pytest.mark.parametrize(
         "args",
         [
@@ -37,6 +38,7 @@ def test_lock_invalid_options(self, cli_runner: CliRunner, args: list[str]):
         exception_message = "Exactly one of --all or plugin name must be specified."
         assert exception_message == str(result.exception)
 
+    @pytest.mark.order(1)
     def test_lockfile_exists(
         self,
         cli_runner: CliRunner,
@@ -53,6 +55,7 @@ def test_lockfile_exists(
         assert "Lockfile exists for loader target-mock" in result.stdout
         assert "Locked definition" not in result.stdout
 
+    @pytest.mark.order(2)
     def test_lockfile_update(
         self,
         cli_runner: CliRunner,
@@ -88,6 +91,7 @@ def test_lockfile_update(
         assert new_setting.name == "foo"
         assert new_setting.value == "bar"
 
+    @pytest.mark.order(3)
     def test_lockfile_update_extractors(
         self,
         cli_runner: CliRunner,

tests/meltano/cli/test_schedule.py

@@ -9,6 +9,7 @@
 
 
 class TestCliSchedule:
+    @pytest.mark.order(0)
     @pytest.mark.usefixtures("tap", "target")
     @mock.patch(
         "meltano.core.schedule_service.PluginSettingsService.get", autospec=True