Address review: document snapshot/count caveats, isolate tests from config

- Document in the docstring and newsfragment that snapshots are not read (they
  can be large and slow) and that the record count may be less precise than
  DataSet.number_of_results.
- Rewrite the tests to build the database via an explicit connection object
  (load_or_create_experiment(conn=...)/new_data_set(conn=...)) so the global
  QCoDeS config is not mutated, and close the connection via request.addfinalizer
  so it is cleaned up even if a test fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Mikhail Astafev

docs/changes/newsfragments/8266.new

@@ -4,4 +4,9 @@ record counts and guids) via a single ``JOIN`` query on the ``runs`` and
 ``experiments`` tables, without instantiating a ``DataSet`` object per run,
 making it possible to list databases with many thousands of runs almost
 instantly. The returned :class:`qcodes.dataset.RunOverviewDict` is also exported
-on the public ``qcodes.dataset`` namespace.
+on the public ``qcodes.dataset`` namespace. The function does not return
+snapshots as they can be large and slow down building the database overview.
+The number of results in each dataset is taken from shape information when
+available, and otherwise falls back to a best-effort count of the rows in the
+so-called results table that exists for every dataset, which may NOT be as
+precise as ``DataSet.number_of_results``.

src/qcodes/dataset/sqlite/db_overview.py

@@ -128,10 +128,13 @@ def get_db_overview(
     tables to fetch run metadata, avoiding the much more expensive
     ``experiments()`` + ``data_sets()`` enumeration that instantiates a
     ``DataSet`` object per run. It is therefore well suited for listing the
-    contents of databases with many runs.
+    contents of databases with many runs. The (potentially large) snapshot of
+    each run is deliberately not read, as it would slow down building the
+    overview significantly.
 
     The reported number of ``records`` is a best-effort estimate of the number
-    of data points in a run:
+    of data points in a run and may be less precise than
+    ``DataSet.number_of_results``:
 
     * For completed runs the shape information stored in the run description is
       preferred (it is the authoritative final count), falling back to the

tests/dataset/test_db_overview.py

@@ -3,18 +3,17 @@
 from __future__ import annotations
 
 import json
-import sqlite3
 from typing import TYPE_CHECKING
 
 import pytest
 
 from qcodes.dataset import (
     get_db_overview,
-    initialise_or_create_database_at,
     load_or_create_experiment,
     new_data_set,
 )
 from qcodes.dataset.descriptions.dependencies import InterDependencies_
+from qcodes.dataset.sqlite.connection import path_to_dbfile
 from qcodes.dataset.sqlite.database import connect
 from qcodes.dataset.sqlite.db_overview import (
     _format_timestamp,
@@ -25,29 +24,44 @@
 if TYPE_CHECKING:
     from pathlib import Path
 
+    from qcodes.dataset.sqlite.connection import AtomicConnection
 
-def _make_db_with_runs(
-    db_path: str,
+
+@pytest.fixture(name="db_conn")
+def _db_conn(tmp_path: Path, request: pytest.FixtureRequest) -> AtomicConnection:
+    """Connection to a fresh temporary database.
+
+    An explicit connection object is used rather than
+    ``initialise_or_create_database_at`` so that the global QCoDeS config is
+    left untouched. The connection is closed through a finalizer so that it is
+    cleaned up even if the test fails.
+    """
+    conn = connect(str(tmp_path / "overview.db"))
+    request.addfinalizer(conn.close)
+    return conn
+
+
+def _create_runs(
+    conn: AtomicConnection,
+    *,
     n_runs: int = 1,
     n_points: int = 10,
     experiment_name: str = "test_exp",
     sample_name: str = "test_sample",
 ) -> None:
-    """Create a database at ``db_path`` with ``n_runs`` simple numeric runs."""
-    initialise_or_create_database_at(db_path)
-    load_or_create_experiment(experiment_name, sample_name=sample_name)
+    """Create ``n_runs`` simple numeric runs on ``conn``."""
+    exp = load_or_create_experiment(experiment_name, sample_name=sample_name, conn=conn)
     p_x = ParamSpecBase("x", "numeric")
     p_y = ParamSpecBase("y", "numeric")
     interdeps = InterDependencies_(dependencies={p_y: (p_x,)})
 
     for r in range(n_runs):
-        ds = new_data_set(f"run_{r + 1}")
+        ds = new_data_set(f"run_{r + 1}", exp_id=exp.exp_id, conn=conn)
         ds.set_interdependencies(interdeps)
         ds.mark_started()
         for i in range(n_points):
             ds.add_results([{p_x.name: float(i), p_y.name: float(i**2)}])
         ds.mark_completed()
-        ds.conn.close()
 
 
 def test_records_from_run_description() -> None:
@@ -77,11 +91,10 @@ def test_format_timestamp() -> None:
     assert time.count(":") == 2
 
 
-def test_get_db_overview_basic_fields(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "basic.db")
-    _make_db_with_runs(db_path, n_runs=2)
+def test_get_db_overview_basic_fields(db_conn: AtomicConnection) -> None:
+    _create_runs(db_conn, n_runs=2)
 
-    overview = get_db_overview(db_path)
+    overview = get_db_overview(path_to_dbfile(db_conn))
 
     assert set(overview.keys()) == {1, 2}
     for run_id, info in overview.items():
@@ -94,109 +107,90 @@ def test_get_db_overview_basic_fields(tmp_path: Path) -> None:
         assert info["completed_date"] and info["completed_time"]
 
 
-def test_get_db_overview_counts_result_rows(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "counts.db")
-    _make_db_with_runs(db_path, n_runs=3, n_points=10)
+def test_get_db_overview_counts_result_rows(db_conn: AtomicConnection) -> None:
+    _create_runs(db_conn, n_runs=3, n_points=10)
 
-    overview = get_db_overview(db_path)
+    overview = get_db_overview(conn=db_conn)
 
-    conn = sqlite3.connect(db_path)
-    try:
-        for run_id, info in overview.items():
-            (table_name,) = conn.execute(
-                "SELECT result_table_name FROM runs WHERE run_id=?", (run_id,)
-            ).fetchone()
-            (actual,) = conn.execute(f'SELECT COUNT(*) FROM "{table_name}"').fetchone()
-            assert info["records"] == actual == 10
-    finally:
-        conn.close()
+    for run_id, info in overview.items():
+        (table_name,) = db_conn.execute(
+            "SELECT result_table_name FROM runs WHERE run_id=?", (run_id,)
+        ).fetchone()
+        (actual,) = db_conn.execute(f'SELECT COUNT(*) FROM "{table_name}"').fetchone()
+        assert info["records"] == actual == 10
 
 
-def test_get_db_overview_incremental(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "incremental.db")
-    _make_db_with_runs(db_path, n_runs=2)
+def test_get_db_overview_incremental(db_conn: AtomicConnection) -> None:
+    _create_runs(db_conn, n_runs=2)
+    path = path_to_dbfile(db_conn)
 
-    assert set(get_db_overview(db_path).keys()) == {1, 2}
-    assert get_db_overview(db_path, start_run_id=2) == {}
+    assert set(get_db_overview(path).keys()) == {1, 2}
+    assert get_db_overview(path, start_run_id=2) == {}
 
-    _make_db_with_runs(db_path, n_runs=1, experiment_name="test_exp2", sample_name="s2")
+    _create_runs(db_conn, n_runs=1, experiment_name="test_exp2", sample_name="s2")
 
-    incremental = get_db_overview(db_path, start_run_id=2)
+    incremental = get_db_overview(path, start_run_id=2)
     assert set(incremental.keys()) == {3}
     assert incremental[3]["experiment"] == "test_exp2"
 
 
-def test_get_db_overview_extra_columns(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "extra.db")
-    initialise_or_create_database_at(db_path)
-    load_or_create_experiment("exp", sample_name="sample")
+def test_get_db_overview_extra_columns(db_conn: AtomicConnection) -> None:
+    exp = load_or_create_experiment("exp", sample_name="sample", conn=db_conn)
     p_x = ParamSpecBase("x", "numeric")
     p_y = ParamSpecBase("y", "numeric")
     interdeps = InterDependencies_(dependencies={p_y: (p_x,)})
-    ds = new_data_set("tagged_run")
+    ds = new_data_set("tagged_run", exp_id=exp.exp_id, conn=db_conn)
     ds.set_interdependencies(interdeps)
     ds.mark_started()
     ds.add_results([{p_x.name: 1.0, p_y.name: 2.0}])
     ds.mark_completed()
     ds.add_metadata("my_tag", "hello")
-    ds.conn.close()
 
     # An existing ad-hoc metadata column is returned ...
-    overview = get_db_overview(db_path, extra_columns=["my_tag"])
+    overview = get_db_overview(conn=db_conn, extra_columns=["my_tag"])
     assert overview[1]["my_tag"] == "hello"  # type: ignore[typeddict-item]
 
     # ... while a non-existent column is silently skipped.
-    overview = get_db_overview(db_path, extra_columns=["does_not_exist"])
+    overview = get_db_overview(conn=db_conn, extra_columns=["does_not_exist"])
     assert "does_not_exist" not in overview[1]
 
 
-def test_get_db_overview_accepts_connection(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "conn.db")
-    _make_db_with_runs(db_path, n_runs=1)
+def test_get_db_overview_accepts_connection(db_conn: AtomicConnection) -> None:
+    _create_runs(db_conn, n_runs=1)
 
-    conn = connect(db_path)
-    try:
-        overview = get_db_overview(conn=conn)
-        assert set(overview.keys()) == {1}
-        # the externally supplied connection must remain usable
-        assert conn.execute("SELECT COUNT(*) FROM runs").fetchone()[0] == 1
-    finally:
-        conn.close()
+    overview = get_db_overview(conn=db_conn)
+    assert set(overview.keys()) == {1}
+    # the externally supplied connection must remain usable
+    assert db_conn.execute("SELECT COUNT(*) FROM runs").fetchone()[0] == 1
 
 
-def test_get_db_overview_rejects_conn_and_path(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "both.db")
-    _make_db_with_runs(db_path, n_runs=1)
+def test_get_db_overview_rejects_conn_and_path(db_conn: AtomicConnection) -> None:
+    _create_runs(db_conn, n_runs=1)
 
-    conn = connect(db_path)
-    try:
-        with pytest.raises(ValueError):
-            get_db_overview(path_to_db=db_path, conn=conn)
-    finally:
-        conn.close()
+    with pytest.raises(ValueError):
+        get_db_overview(path_to_db=path_to_dbfile(db_conn), conn=db_conn)
 
 
-def test_get_db_overview_in_progress_uses_live_row_count(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "in_progress.db")
-    initialise_or_create_database_at(db_path)
-    load_or_create_experiment("exp", sample_name="sample")
+def test_get_db_overview_in_progress_uses_live_row_count(
+    db_conn: AtomicConnection,
+) -> None:
+    exp = load_or_create_experiment("exp", sample_name="sample", conn=db_conn)
     p_x = ParamSpecBase("x", "numeric")
     p_y = ParamSpecBase("y", "numeric")
     interdeps = InterDependencies_(dependencies={p_y: (p_x,)})
 
     # run 1: started, still in progress, with 5 data points
-    ds_live = new_data_set("live")
+    ds_live = new_data_set("live", exp_id=exp.exp_id, conn=db_conn)
     ds_live.set_interdependencies(interdeps)
     ds_live.mark_started()
     for i in range(5):
         ds_live.add_results([{p_x.name: float(i), p_y.name: float(i)}])
     # run 2: started, in progress, no data yet
-    ds_empty = new_data_set("empty")
+    ds_empty = new_data_set("empty", exp_id=exp.exp_id, conn=db_conn)
     ds_empty.set_interdependencies(interdeps)
     ds_empty.mark_started()
-    ds_empty.conn.close()  # shared connection for both datasets
 
-    overview = get_db_overview(db_path)
+    overview = get_db_overview(conn=db_conn)
 
     # in-progress runs report the live results-table row count and have no
     # completed timestamp
@@ -207,37 +201,29 @@ def test_get_db_overview_in_progress_uses_live_row_count(tmp_path: Path) -> None
 
 
 def test_get_db_overview_missing_results_table_reports_zero(
-    tmp_path: Path,
+    db_conn: AtomicConnection,
 ) -> None:
-    db_path = str(tmp_path / "no_table.db")
-    _make_db_with_runs(db_path, n_runs=1, n_points=5)
+    _create_runs(db_conn, n_runs=1, n_points=5)
 
-    conn = connect(db_path)
-    try:
-        (table_name,) = conn.execute(
-            "SELECT result_table_name FROM runs WHERE run_id=1"
-        ).fetchone()
-        conn.execute(f'DROP TABLE "{table_name}"')
-        conn.commit()
-    finally:
-        conn.close()
+    (table_name,) = db_conn.execute(
+        "SELECT result_table_name FROM runs WHERE run_id=1"
+    ).fetchone()
+    db_conn.execute(f'DROP TABLE "{table_name}"')
+    db_conn.commit()
 
     # with the results table gone (and no shape info) the count is unknown;
     # ``result_counter`` is the run ordinal, not a data-point count, so it must
     # not be used as a fallback
-    overview = get_db_overview(db_path)
+    overview = get_db_overview(conn=db_conn)
     assert overview[1]["records"] == 0
 
 
-def test_get_db_overview_query_error_returns_empty(tmp_path: Path) -> None:
-    db_path = str(tmp_path / "broken.db")
-    _make_db_with_runs(db_path, n_runs=1)
+def test_get_db_overview_query_error_returns_empty(
+    db_conn: AtomicConnection,
+) -> None:
+    _create_runs(db_conn, n_runs=1)
 
-    conn = connect(db_path)
-    try:
-        # remove a table the overview query depends on so the JOIN fails
-        conn.execute("DROP TABLE experiments")
-        conn.commit()
-        assert get_db_overview(conn=conn) == {}
-    finally:
-        conn.close()
+    # remove a table the overview query depends on so the JOIN fails
+    db_conn.execute("DROP TABLE experiments")
+    db_conn.commit()
+    assert get_db_overview(conn=db_conn) == {}