Skip to content

Add db_uri fixture and replace cached_db usage across test suite#19236

Merged
harupy merged 5 commits intomasterfrom
copilot/add-db-uri-fixture
Dec 5, 2025
Merged

Add db_uri fixture and replace cached_db usage across test suite#19236
harupy merged 5 commits intomasterfrom
copilot/add-db-uri-fixture

Conversation

Copy link
Contributor

Copilot AI commented Dec 5, 2025

Related Issues/PRs

#xxx

What changes are proposed in this pull request?

Adds a reusable db_uri fixture to tests/conftest.py that provides a fresh SQLite database URI for each test by copying the session-scoped cached_db, and refactors all existing tests to use this new fixture instead of manually copying the database.

Implementation:

@pytest.fixture
def db_uri(tmp_path: Path, cached_db: Path) -> str:
    """Returns a fresh SQLite URI for each test by copying the cached database."""
    db_path = tmp_path / "mlflow.db"
    shutil.copy2(cached_db, db_path)
    return f"sqlite:///{db_path}"

Benefits:

  • Eliminates boilerplate: tests no longer need to manually copy cached_db and construct URIs
  • Ensures isolation: each test gets its own database copy in its temporary directory
  • Standardizes pattern: consistent approach across the test suite
  • Reduces code duplication: removed 86 lines of repetitive database copying code

Changes made:

# Before (manual pattern repeated in 13 test files)
db_path = tmp_path / "mlflow.db"
shutil.copy(cached_db, db_path)
uri = f"sqlite:///{db_path}"

# After (using the new fixture)
def test_example(db_uri):
    # uri ready to use

Files refactored to use db_uri fixture:

  • tests/genai/evaluate/test_evaluation.py
  • tests/store/model_registry/test_sqlalchemy_store.py
  • tests/store/tracking/test_sqlalchemy_store.py
  • tests/tracing/test_assessment.py
  • tests/tracing/test_fluent.py
  • tests/tracing/test_otel_loading.py
  • tests/tracing/test_otel_logging.py
  • tests/tracking/fluent/test_fluent.py
  • tests/tracking/test_client.py
  • tests/tracking/test_log_image.py
  • tests/tracking/test_model_registry.py
  • tests/tracking/test_rest_tracking.py
  • tests/tracking/test_tracking.py

How is this PR tested?

  • Existing unit/integration tests
  • New unit/integration tests
  • Manual tests

Verified the fixture works correctly by running existing tests that previously used cached_db directly. All tests continue to pass with the new fixture-based approach.

Does this PR require documentation update?

  • No. You can skip the rest of this section.
  • Yes. I've updated:
    • Examples
    • API references
    • Instructions

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release notes for MLflow users.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

  • area/tracking: Tracking Service, tracking client APIs, autologging
  • area/models: MLmodel format, model serialization/deserialization, flavors
  • area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
  • area/scoring: MLflow Model server, model deployment tools, Spark UDFs
  • area/evaluation: MLflow model evaluation features, evaluation metrics, and evaluation workflows
  • area/gateway: MLflow AI Gateway client APIs, server, and third-party integrations
  • area/prompts: MLflow prompt engineering features, prompt templates, and prompt management
  • area/tracing: MLflow Tracing features, tracing APIs, and LLM tracing functionality
  • area/projects: MLproject format, project running backends
  • area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
  • area/build: Build and test infrastructure for MLflow
  • area/docs: MLflow documentation pages

How should the PR be classified in the release notes? Choose one:

  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
  • rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
  • rn/feature - A new user-facing feature worth mentioning in the release notes
  • rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
  • rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

  • Yes (this PR will be cherry-picked and included in the next patch release)
  • No (this PR will be included in the next minor release)
Original prompt

Add the following fixture in tests/conftest.py and replace cached_db usage in db_uri fixture:

# tests/conftest.py

@pytest.fixture
def db_uri(tmp_path: Path, cached_db: Path) -> str:
    """Returns a fresh SQLite URI for each test by copying the cached database."""
    db_path = tmp_path / "mlflow.db"
    shutil.copy2(cached_db, db_path)
    return f"sqlite:///{db_path}"

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@harupy harupy marked this pull request as ready for review December 5, 2025 07:01
Copilot AI review requested due to automatic review settings December 5, 2025 07:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copilot AI changed the title [WIP] Add db_uri fixture and replace cached_db usage Add db_uri fixture to tests/conftest.py Dec 5, 2025
Copilot AI requested a review from harupy December 5, 2025 07:12
@harupy
Copy link
Member

harupy commented Dec 5, 2025

@copilot can you replace existing cached_db usage with db_uri?

here's an example

diff --git a/tests/tracing/test_otel_logging.py b/tests/tracing/test_otel_logging.py
index 5a4368bece..e214a76687 100644
--- a/tests/tracing/test_otel_logging.py
+++ b/tests/tracing/test_otel_logging.py
@@ -6,7 +6,6 @@ when using OpenTelemetry clients to send spans to MLflow's OTel endpoint.
 """
 
 import gzip
-import shutil
 import time
 import zlib
 from pathlib import Path
@@ -49,17 +48,12 @@ if IS_TRACING_SDK_ONLY:
 
 
 @pytest.fixture
-def mlflow_server(tmp_path: Path, cached_db: Path) -> Iterator[str]:
-    # Copy the pre-initialized cached DB into this test's tmp path
-    db_path = tmp_path / "mlflow.db"
-    shutil.copy(cached_db, db_path)
-
-    backend_store_uri = f"sqlite:///{db_path}"
+def mlflow_server(tmp_path: Path, db_uri: str) -> Iterator[str]:
     artifact_root = tmp_path.as_uri()
 
     handlers._tracking_store = None
     handlers._model_registry_store = None
-    initialize_backend_stores(backend_store_uri, default_artifact_root=artifact_root)
+    initialize_backend_stores(db_uri, default_artifact_root=artifact_root)
 
     # Start the FastAPI app in a background thread and yield its URL.
     with ServerThread(mlflow_app, get_safe_port()) as url:

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 5, 2025

@copilot can you replace existing cached_db usage with db_uri?

here's an example...

Done! I've replaced all cached_db usage with the new db_uri fixture across 13 test files. The changes eliminate 86 lines of repetitive database copying code and provide a consistent pattern throughout the test suite.

Changes made in commit 7eda524:

  • Updated all fixtures that were manually copying cached_db to use db_uri instead
  • Removed unnecessary shutil imports where no longer needed
  • Tests pass and code is properly linted/formatted

Copilot AI changed the title Add db_uri fixture to tests/conftest.py Add db_uri fixture and replace cached_db usage across test suite Dec 5, 2025
@github-actions github-actions bot added area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs. labels Dec 5, 2025
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy harupy enabled auto-merge December 5, 2025 10:03
@harupy harupy added this pull request to the merge queue Dec 5, 2025
Merged via the queue into master with commit e3345f7 Dec 5, 2025
42 checks passed
@harupy harupy deleted the copilot/add-db-uri-fixture branch December 5, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants