diff --git a/src/review_classification/cli/app.py b/src/review_classification/cli/app.py index 6f5d4bb..d51c580 100644 --- a/src/review_classification/cli/app.py +++ b/src/review_classification/cli/app.py @@ -570,6 +570,16 @@ def classify( ) raise typer.Exit(code=1) + from ..sqlite.database import database_is_initialized + + if not database_is_initialized(): + typer.echo( + "Error: No database found. Run 'review-classify fetch' first " + "to collect PR data.", + err=True, + ) + raise typer.Exit(code=1) + try: targets = _resolve_targets( repos=repo_list, diff --git a/src/review_classification/sqlite/database.py b/src/review_classification/sqlite/database.py index 8317d75..3fdd4bd 100644 --- a/src/review_classification/sqlite/database.py +++ b/src/review_classification/sqlite/database.py @@ -1,3 +1,4 @@ +import os from datetime import datetime from sqlalchemy import text @@ -12,6 +13,26 @@ engine = create_engine(sqlite_url) +def database_is_initialized() -> bool: + """Return True if the database file exists and contains the PR table. + + A bare file with no tables (e.g. left behind by an earlier failed run) + counts as *not* initialized, so callers can prompt the user to run + ``fetch`` first instead of producing an empty report. The on-disk check + runs first to avoid lazily creating an empty database file. + """ + from sqlalchemy import inspect + from sqlalchemy.exc import OperationalError + + if not os.path.exists(sqlite_file_name): + return False + + try: + return inspect(engine).has_table(str(PullRequest.__tablename__)) + except OperationalError: + return False + + def init_db() -> None: """Initialize the database tables.""" SQLModel.metadata.create_all(engine) @@ -123,14 +144,20 @@ def get_repos_for_org(org_name: str) -> list[str]: """Return distinct repository names stored in the DB for the given org/owner. Avoids any network call — resolves org repos entirely from fetched data. + Returns an empty list if the database has not been initialized yet. """ - with Session(engine) as session: - statement = ( - select(col(PullRequest.repository_name)) - .where(col(PullRequest.repository_name).like(f"{org_name}/%")) - .distinct() - ) - return sorted(session.exec(statement).all()) + from sqlalchemy.exc import OperationalError + + try: + with Session(engine) as session: + statement = ( + select(col(PullRequest.repository_name)) + .where(col(PullRequest.repository_name).like(f"{org_name}/%")) + .distinct() + ) + return sorted(session.exec(statement).all()) + except OperationalError: + return [] def get_outlier_scores( diff --git a/tests/cli/test_classify.py b/tests/cli/test_classify.py new file mode 100644 index 0000000..fb815d3 --- /dev/null +++ b/tests/cli/test_classify.py @@ -0,0 +1,105 @@ +"""Tests for classify command behavior.""" + +from importlib import import_module +from pathlib import Path + +import pytest +import typer +from sqlmodel import create_engine + +cli_app = import_module("review_classification.cli.app") + + +def test_classify_missing_database_shows_clean_error( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """classify in a dir with no DB exits 1 with a clean message, no traceback.""" + monkeypatch.chdir(tmp_path) + + with pytest.raises(typer.Exit) as exc_info: + cli_app.classify( + repo=["my-org/repo"], + org=None, + config=None, + threshold=2.0, + min_samples=30, + output_format="table", + verbose=False, + start=None, + end=None, + exclude_primary_merged=False, + ) + + assert exc_info.value.exit_code == 1 + + captured = capsys.readouterr() + assert "No database found" in captured.err + assert "fetch" in captured.err + # The raw SQLAlchemy error must never reach the user. + assert "OperationalError" not in captured.err + assert "no such table" not in captured.err + # No stray empty database file should be created. + assert not (tmp_path / "review_classification.db").exists() + + +def test_classify_missing_database_for_org_shows_clean_error( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """The org path (which reads the DB to resolve repos) also fails gracefully.""" + monkeypatch.chdir(tmp_path) + + with pytest.raises(typer.Exit) as exc_info: + cli_app.classify( + repo=None, + org=["my-org"], + config=None, + threshold=2.0, + min_samples=30, + output_format="table", + verbose=False, + start=None, + end=None, + exclude_primary_merged=False, + ) + + assert exc_info.value.exit_code == 1 + + captured = capsys.readouterr() + assert "No database found" in captured.err + assert "OperationalError" not in captured.err + + +def test_classify_empty_database_for_org_shows_clean_error( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A leftover empty DB file (no tables) must not yield a silent empty report.""" + import review_classification.sqlite.database as db_module + + monkeypatch.chdir(tmp_path) + db_path = tmp_path / db_module.sqlite_file_name + db_path.touch() # file exists but has no tables, e.g. from a failed run + monkeypatch.setattr(db_module, "engine", create_engine(f"sqlite:///{db_path}")) + + with pytest.raises(typer.Exit) as exc_info: + cli_app.classify( + repo=None, + org=["my-org"], + config=None, + threshold=2.0, + min_samples=30, + output_format="table", + verbose=False, + start=None, + end=None, + exclude_primary_merged=False, + ) + + assert exc_info.value.exit_code == 1 + captured = capsys.readouterr() + assert "No database found" in captured.err diff --git a/tests/sqlite/test_database.py b/tests/sqlite/test_database.py index f4e74da..a58675c 100644 --- a/tests/sqlite/test_database.py +++ b/tests/sqlite/test_database.py @@ -1,6 +1,7 @@ """Tests for sqlite.database helper functions.""" from datetime import UTC, datetime +from pathlib import Path import pytest from sqlalchemy.engine import Engine @@ -83,3 +84,57 @@ def test_get_repos_for_org_deduplicates(patched_engine: Engine) -> None: session.commit() assert get_repos_for_org("my-org") == ["my-org/repo-a"] + + +def test_get_repos_for_org_handles_missing_table( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Returns [] instead of raising when the database has no tables yet.""" + import review_classification.sqlite.database as db_module + + # Engine pointing at an in-memory DB with no tables created. + empty_engine = create_engine("sqlite:///:memory:") + monkeypatch.setattr(db_module, "engine", empty_engine) + + assert db_module.get_repos_for_org("any-org") == [] + + +def test_database_is_initialized_false_when_file_missing( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """No database file on disk → not initialized (and no file is created).""" + import review_classification.sqlite.database as db_module + + monkeypatch.chdir(tmp_path) + assert db_module.database_is_initialized() is False + # The check must not lazily create an empty database file. + assert not (tmp_path / db_module.sqlite_file_name).exists() + + +def test_database_is_initialized_false_for_empty_file( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A bare file with no tables (failed earlier run) counts as not initialized.""" + import review_classification.sqlite.database as db_module + + monkeypatch.chdir(tmp_path) + db_path = tmp_path / db_module.sqlite_file_name + db_path.touch() # empty SQLite file, no tables + monkeypatch.setattr(db_module, "engine", create_engine(f"sqlite:///{db_path}")) + + assert db_module.database_is_initialized() is False + + +def test_database_is_initialized_true_with_tables( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A file containing the expected tables counts as initialized.""" + import review_classification.sqlite.database as db_module + + monkeypatch.chdir(tmp_path) + db_path = tmp_path / db_module.sqlite_file_name + engine = create_engine(f"sqlite:///{db_path}") + SQLModel.metadata.create_all(engine) + monkeypatch.setattr(db_module, "engine", engine) + + assert db_module.database_is_initialized() is True