diff --git a/.claude/skills/flight-search/SKILL.md b/.claude/skills/flight-search/SKILL.md index b70ef59..19cd409 100644 --- a/.claude/skills/flight-search/SKILL.md +++ b/.claude/skills/flight-search/SKILL.md @@ -26,7 +26,10 @@ Global flags (every search-printing command): - `--json` — emit raw response JSON - `--no-cache` — bypass the on-disk response cache - `--matrix-url` / `--google-url` — toggle deep-link emission -- `--no-pp` — skip the PointsPath award overlay even if tokens are logged in (PP runs implicitly on both matrix and gflight backends when tokens are present) +- `--cash-only` — skip all award providers; show only the cash table +- `--awards-only` — skip the cash table; show only the award provider output +- `--providers pp[,seats]` — restrict to a named subset of award providers (default: all configured) +- `--provider-opt KEY=VAL` — per-provider override, repeatable, e.g. `--provider-opt pp.airlines=United,Delta` or `--provider-opt pp.cabins=Economy,Business`. Defaults live in `~/.config/flight-cli/config.toml` under `[providers.]` tables. ## Intent → flag cheat sheet diff --git a/src/flight_cli/_config.py b/src/flight_cli/_config.py new file mode 100644 index 0000000..d0e37c1 --- /dev/null +++ b/src/flight_cli/_config.py @@ -0,0 +1,101 @@ +"""User-level config loader for ~/.config/flight-cli/config.toml. + +Today's only consumer is the provider section — per-provider airline lists, +cabin lists, etc. The shape is intentionally flat per provider so adding a +new provider (seats.aero, etc.) is a new `[providers.]` table, not a +schema migration. + +The loader is forgiving: missing file → empty config, partial sections → +fill in with what's there, unknown keys → kept verbatim (callers decide +whether to honor them). +""" + +from __future__ import annotations + +import os +import tomllib +from pathlib import Path +from typing import Any, cast + +CONFIG_DIR_ENV = "FLIGHT_CLI_CONFIG_DIR" +DEFAULT_CONFIG_PATH = Path.home() / ".config" / "flight-cli" / "config.toml" + + +def _config_path() -> Path: + override = os.environ.get(CONFIG_DIR_ENV) + if override: + return Path(override) / "config.toml" + return DEFAULT_CONFIG_PATH + + +def load() -> dict[str, Any]: + """Read config.toml, return parsed dict. Missing file → {}. + + Parse errors raise — silent fallback would hide typos that defeat the + user's intent.""" + path = _config_path() + if not path.exists(): + return {} + with path.open("rb") as f: + return tomllib.load(f) + + +def provider_options(name: str, *, config: dict[str, Any] | None = None) -> dict[str, Any]: + """Pull the `[providers.]` table out of config, or {} if absent. + + Caller is responsible for type-coercing the values (lists stay as lists, + strings stay as strings — TOML's native typing is preserved).""" + cfg = config if config is not None else load() + providers_any: Any = cfg.get("providers", {}) + if not isinstance(providers_any, dict): + return {} + providers_dict = cast("dict[str, Any]", providers_any) + section_any: Any = providers_dict.get(name, {}) + if not isinstance(section_any, dict): + return {} + return cast("dict[str, Any]", section_any) + + +def parse_provider_opt_overrides(raw: list[str]) -> dict[str, dict[str, Any]]: + """Parse `--provider-opt pp.airlines=United,Delta` items into nested dict. + + `pp.airlines=United,Delta` → `{"pp": {"airlines": ["United", "Delta"]}}`. + A bare scalar (no comma) stays a string; comma-list becomes list[str]. + + Repeated keys for the same provider merge at the outer dict; same inner + key wins last (caller is responsible for order if it matters). + """ + out: dict[str, dict[str, Any]] = {} + for item in raw: + if "=" not in item: + msg = f"--provider-opt {item!r} missing '='; expected 'provider.key=value'" + raise ValueError(msg) + path, value = item.split("=", 1) + if "." not in path: + msg = f"--provider-opt {item!r} missing '.'; expected 'provider.key=value'" + raise ValueError(msg) + provider, key = path.split(".", 1) + provider = provider.strip() + key = key.strip() + if not provider or not key: + msg = f"--provider-opt {item!r}: provider and key must be non-empty" + raise ValueError(msg) + parsed: str | list[str] = ( + [v.strip() for v in value.split(",")] if "," in value else value.strip() + ) + out.setdefault(provider, {})[key] = parsed + return out + + +def merge_provider_options(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]: + """Shallow merge: override's keys win, both at provider level and key level. + + Used to layer CLI --provider-opt on top of config.toml.""" + out: dict[str, Any] = {**base} + for k, v in override.items(): + existing = out.get(k) + if isinstance(existing, dict) and isinstance(v, dict): + out[k] = {**existing, **v} + else: + out[k] = v + return out diff --git a/src/flight_cli/cli.py b/src/flight_cli/cli.py index 35ea2ac..c44450c 100644 --- a/src/flight_cli/cli.py +++ b/src/flight_cli/cli.py @@ -24,6 +24,7 @@ from rich.console import Console from rich.table import Table +from . import _config from .client import MatrixApiError, MatrixClient from .domain import ( Cabin, @@ -245,7 +246,7 @@ def _pick_backend( return backend -def _should_run_pp(*, no_pp: bool, pp_only: bool) -> bool: +def _should_run_pp(*, no_pp: bool, pp_only: bool) -> bool: # pyright: ignore[reportUnusedFunction] """Decide whether PP augmentation runs. Tokens present → True (unless --no-pp). Both backends support PP overlay @@ -272,6 +273,173 @@ def _should_run_pp(*, no_pp: bool, pp_only: bool) -> bool: return True +class ProviderSelection: + """Resolved provider selection: what runs, which subset, with what options. + + `provider_filter` is a tuple of provider names to use (None = all enabled). + `cash_only` skips every award provider. `awards_only` suppresses the cash + table render. `provider_opts` is `{provider_name: {key: value}}` — e.g. + `{"pp": {"airlines": ["United", "Delta"], "cabins": ["Economy"]}}`. + """ + + __slots__ = ("awards_only", "cash_only", "provider_filter", "provider_opts") + + def __init__( + self, + *, + provider_filter: tuple[str, ...] | None, + cash_only: bool, + awards_only: bool, + provider_opts: dict[str, dict[str, Any]], + ) -> None: + self.provider_filter = provider_filter + self.cash_only = cash_only + self.awards_only = awards_only + self.provider_opts = provider_opts + + def pp_airlines(self) -> str | None: + """Backward-compat shim: PP's `airlines` as CSV (None = use default).""" + v: Any = self.provider_opts.get("pp", {}).get("airlines") + if v is None: + return None + if isinstance(v, list): + return ",".join(str(x) for x in cast("list[Any]", v)) + return str(v) + + def pp_cabins(self) -> str | None: + """Backward-compat shim: PP's `cabins` as CSV (None = use default).""" + v: Any = self.provider_opts.get("pp", {}).get("cabins") + if v is None: + return None + if isinstance(v, list): + return ",".join(str(x) for x in cast("list[Any]", v)) + return str(v) + + +def _resolve_providers( # noqa: PLR0912 — single-purpose validator + merge; splitting hurts readability + *, + providers: str | None, + cash_only: bool, + awards_only: bool, + provider_opt: tuple[str, ...], + # deprecated aliases — forwarded into the new shape: + legacy_no_pp: bool = False, + legacy_pp_only: bool = False, + legacy_pp_airlines: str | None = None, + legacy_pp_cabin: str | None = None, +) -> ProviderSelection: + """Resolve the new + deprecated provider flags into one ProviderSelection. + + Precedence for per-provider options: config.toml < --provider-opt CLI. + Deprecated --pp-airlines / --pp-cabin map onto the --provider-opt path + so legacy invocations land in the same downstream shape. + """ + # Conflict checks across the new surface. + if cash_only and awards_only: + err.print("[red]--cash-only and --awards-only are mutually exclusive.[/]") + raise typer.Exit(2) + # Conflict checks across the old + new surface. + new_surface_set = cash_only or awards_only or providers is not None + if legacy_no_pp and new_surface_set: + err.print( + "[red]--no-pp conflicts with --cash-only/--awards-only/--providers; use one.[/]", + ) + raise typer.Exit(2) + if legacy_pp_only and new_surface_set: + err.print( + "[red]--pp-only conflicts with --cash-only/--awards-only/--providers; use one.[/]", + ) + raise typer.Exit(2) + if legacy_no_pp and legacy_pp_only: + err.print("[red]--no-pp and --pp-only are mutually exclusive.[/]") + raise typer.Exit(2) + + # Forward legacy intent. + if legacy_no_pp: + cash_only = True + if legacy_pp_only: + awards_only = True + + # --providers parsing. + provider_filter: tuple[str, ...] | None = None + if providers is not None: + provider_filter = tuple(p.strip() for p in providers.split(",") if p.strip()) + if not provider_filter: + err.print("[red]--providers cannot be empty.[/]") + raise typer.Exit(2) + + # Build provider_opts: config.toml < --provider-opt CLI. + try: + config = _config.load() + except (OSError, ValueError) as e: + err.print(f"[red]Failed to load ~/.config/flight-cli/config.toml: {e}[/]") + raise typer.Exit(2) from e + base_opts: dict[str, dict[str, Any]] = {} + providers_section: Any = config.get("providers", {}) + if isinstance(providers_section, dict): + for name, opts in cast("dict[str, Any]", providers_section).items(): + if isinstance(opts, dict): + base_opts[name] = dict(cast("dict[str, Any]", opts)) + + try: + cli_opts = _config.parse_provider_opt_overrides(list(provider_opt)) + except ValueError as e: + err.print(f"[red]{e}[/]") + raise typer.Exit(2) from e + merged_opts = _config.merge_provider_options(base_opts, cli_opts) + + # Forward legacy --pp-airlines / --pp-cabin into the merged opts. + # CLI --provider-opt still wins (no-op if user set both, since merged_opts + # already has the override from cli_opts). + pp_section: dict[str, Any] = dict(merged_opts.get("pp", {})) + if legacy_pp_airlines is not None and "airlines" not in pp_section: + pp_section["airlines"] = [v.strip() for v in legacy_pp_airlines.split(",") if v.strip()] + if legacy_pp_cabin is not None and "cabins" not in pp_section: + pp_section["cabins"] = [v.strip() for v in legacy_pp_cabin.split(",") if v.strip()] + if pp_section: + merged_opts["pp"] = pp_section + + return ProviderSelection( + provider_filter=provider_filter, + cash_only=cash_only, + awards_only=awards_only, + provider_opts=merged_opts, + ) + + +def _should_run_awards(sel: ProviderSelection) -> bool: + """Award providers run iff (a) not --cash-only, (b) at least one is + configured, and (c) the filter (if any) names at least one configured + provider. Mirrors the old _should_run_pp semantics for the PP-only era; + generalizes naturally once more providers exist. + """ + if sel.cash_only: + return False + # Today we only know about PP. has_any_configured() is provider-blind by + # construction (see providers/registry.py) so this stays correct when + # work-2eoa adds seats.aero. + from .providers.registry import has_any_configured # noqa: PLC0415 + + if not has_any_configured(): + if sel.awards_only: + err.print( + "[red]--awards-only set but no award provider is configured.[/] " + "Run `flight auth pp login --tokens-file ...` first.", + ) + raise typer.Exit(2) + return False + if sel.provider_filter is not None and "pp" not in sel.provider_filter: + # Filter excludes PP; nothing else is configured yet. + if sel.awards_only: + err.print( + f"[red]--awards-only set but --providers={sel.provider_filter} " + "matches no configured provider.[/]", + ) + raise typer.Exit(2) + return False + return True + + # ─────────────────────────── shared execution ────────────────────────────── @@ -504,9 +672,7 @@ def _run_matrix_path( matrix_url: bool, google_url: bool, run_pp: bool, - pp_only: bool, - pp_airlines: str | None, - pp_cabin: str | None, + sel: ProviderSelection, ) -> None: """Matrix path: Alkali call → optional cash render → optional PP augmentation → URLs.""" search = SpecificDateSearch(legs=legs, options=opts) @@ -515,7 +681,7 @@ def _run_matrix_path( if json_out and not run_pp: sys.stdout.write(json.dumps(res.raw, indent=2)) return - if not pp_only: + if not sel.awards_only: _render_search(res) if run_pp: p = opts.pax @@ -523,10 +689,11 @@ def _run_matrix_path( res, legs=_build_pp_legs(legs), num_passengers=p.adults + p.children + p.seniors + p.youth, - airlines=pp_airlines, - cabins=pp_cabin, - pp_only=pp_only, + airlines=sel.pp_airlines(), + cabins=sel.pp_cabins(), + pp_only=sel.awards_only, json_out=json_out, + provider_filter=sel.provider_filter, ) _emit_urls(search, matrix_url=matrix_url, google_url=google_url) @@ -538,9 +705,7 @@ def _run_gflight_path( top_n: int, json_out: bool, run_pp: bool = False, - pp_only: bool = False, - pp_airlines: str | None = None, - pp_cabin: str | None = None, + sel: ProviderSelection | None = None, ) -> None: """Google Flights path: build fli filter → query → render. Single-leg or round-trip. @@ -583,7 +748,8 @@ def _run_gflight_path( sys.stdout.write(json.dumps(out, indent=2, default=str)) return - if not pp_only: + awards_only = sel.awards_only if sel is not None else False + if not awards_only: _render_gflight_table(results, legs=legs, top_n=top_n) if run_pp: @@ -595,10 +761,11 @@ def _run_gflight_path( sr, legs=_build_pp_legs(legs), num_passengers=p.adults + p.children + p.seniors + p.youth, - airlines=pp_airlines, - cabins=pp_cabin, - pp_only=pp_only, + airlines=sel.pp_airlines() if sel is not None else None, + cabins=sel.pp_cabins() if sel is not None else None, + pp_only=awards_only, json_out=json_out, + provider_filter=sel.provider_filter if sel is not None else None, ) @@ -729,6 +896,14 @@ def _fmt_gflight_legroom(fli_legs: list[Any], amenities: list[Any]) -> str: # Common-args helpers — these reduce repetition across commands. _RPS_OPT = typer.Option(1.0, help="Requests per second") _IMPERSONATE_OPT = typer.Option("chrome", help="curl_cffi profile") +_PROVIDER_OPT = typer.Option( + None, + "--provider-opt", + help=( + "Per-provider override, repeatable: 'pp.airlines=United,Delta'. " + "Overrides ~/.config/flight-cli/config.toml [providers.]." + ), +) @app.command() @@ -817,31 +992,45 @@ def search( matrix_url: bool = typer.Option(True, "--matrix-url/--no-matrix-url"), google_url: bool = typer.Option(True, "--google-url/--no-google-url"), no_cache: bool = typer.Option(False, "--no-cache"), + providers: str | None = typer.Option( + None, + "--providers", + help=("CSV of award providers to use (e.g. 'pp'). Default: all configured providers."), + ), + cash_only: bool = typer.Option( + False, + "--cash-only", + help="Skip all award providers; just show the cash table.", + ), + awards_only: bool = typer.Option( + False, + "--awards-only", + help="Skip the cash table; show only the award provider output.", + ), + provider_opt: list[str] | None = _PROVIDER_OPT, no_pp: bool = typer.Option( False, "--no-pp", - help=( - "Skip PointsPath award augmentation even if tokens are present. " - "PP runs implicitly on matrix backend when valid tokens exist." - ), + hidden=True, + help="[deprecated] Use --cash-only.", ), pp_only: bool = typer.Option( False, "--pp-only", - help="Show only PointsPath award availability; skip Matrix cash table.", + hidden=True, + help="[deprecated] Use --awards-only.", ), pp_airlines: str | None = typer.Option( None, "--pp-airlines", - help=( - "CSV of PointsPath airline names (e.g. United,Delta). " - "Default: discovered from your account's enabled airline set." - ), + hidden=True, + help="[deprecated] Use --provider-opt pp.airlines=A,B.", ), pp_cabin: str | None = typer.Option( None, "--pp-cabin", - help="CSV of cabins to query (Economy,Business,First). Default: Economy,Business.", + hidden=True, + help="[deprecated] Use --provider-opt pp.cabins=Economy,Business.", ), ) -> None: """Specific-date flight search across Matrix and Google Flights backends. @@ -850,6 +1039,23 @@ def search( Matrix-only flag is set (routing/extension/multi-city slice/time-of-day/ extra pax types/PP config). Force with --backend matrix|gflight. """ + # Deprecated-flag warning surfaces at runtime since hidden=True hides the + # banner from --help. + if no_pp or pp_only or pp_airlines or pp_cabin: + err.print( + "[yellow]--no-pp/--pp-only/--pp-airlines/--pp-cabin are deprecated; " + "use --cash-only / --awards-only / --provider-opt instead.[/]", + ) + sel = _resolve_providers( + providers=providers, + cash_only=cash_only, + awards_only=awards_only, + provider_opt=tuple(provider_opt or ()), + legacy_no_pp=no_pp, + legacy_pp_only=pp_only, + legacy_pp_airlines=pp_airlines, + legacy_pp_cabin=pp_cabin, + ) resolved = _pick_backend( backend=backend, routing=routing, @@ -906,21 +1112,18 @@ def search( page_size=page_size, ) + run_awards = _should_run_awards(sel) if resolved == BACKEND_GFLIGHT: - run_pp = _should_run_pp(no_pp=no_pp, pp_only=pp_only) _run_gflight_path( legs=legs, opts=opts, top_n=page_size, json_out=json_out, - run_pp=run_pp, - pp_only=pp_only, - pp_airlines=pp_airlines, - pp_cabin=pp_cabin, + run_pp=run_awards, + sel=sel, ) return - run_pp = _should_run_pp(no_pp=no_pp, pp_only=pp_only) _run_matrix_path( legs=legs, opts=opts, @@ -930,10 +1133,8 @@ def search( json_out=json_out, matrix_url=matrix_url, google_url=google_url, - run_pp=run_pp, - pp_only=pp_only, - pp_airlines=pp_airlines, - pp_cabin=pp_cabin, + run_pp=run_awards, + sel=sel, ) @@ -1080,7 +1281,17 @@ def fare( show_only_available=only_available, page_size=page_size, ) - run_pp = _should_run_pp(no_pp=no_pp, pp_only=pp_only) + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_no_pp=no_pp, + legacy_pp_only=pp_only, + legacy_pp_airlines=pp_airlines, + legacy_pp_cabin=pp_cabin, + ) + run_pp = _should_run_awards(sel) _run_matrix_path( legs=legs, opts=opts, @@ -1091,9 +1302,7 @@ def fare( matrix_url=matrix_url, google_url=google_url, run_pp=run_pp, - pp_only=pp_only, - pp_airlines=pp_airlines, - pp_cabin=pp_cabin, + sel=sel, ) diff --git a/src/flight_cli/pp/cli.py b/src/flight_cli/pp/cli.py index e49625b..d7010b4 100644 --- a/src/flight_cli/pp/cli.py +++ b/src/flight_cli/pp/cli.py @@ -195,6 +195,7 @@ def run_pp_for_search( cabins: str | None = None, pp_only: bool = False, json_out: bool = False, + provider_filter: tuple[str, ...] | None = None, ) -> None: """Run award augmentation through the provider registry, join against `res`'s cash itineraries, render. Today the registry hands back @@ -227,6 +228,7 @@ async def _go() -> tuple[list[list[AwardFlight]], list[Any]]: cabins=cabin_list, pp_airlines=explicit_airlines, cash_hints_per_leg=cash_hints_per_leg, + provider_filter=provider_filter, ) try: diff --git a/src/flight_cli/providers/registry.py b/src/flight_cli/providers/registry.py index d7bb6a3..21e9082 100644 --- a/src/flight_cli/providers/registry.py +++ b/src/flight_cli/providers/registry.py @@ -31,6 +31,7 @@ async def _construct_enabled( *, pp_airlines: tuple[str, ...] | None = None, + provider_filter: tuple[str, ...] | None = None, ) -> list[AwardProvider]: """Build the list of enabled provider instances. @@ -38,9 +39,16 @@ async def _construct_enabled( configured providers get instantiated (which is when network/auth actually happens). Failures during construction are logged and swallowed so one provider's outage can't take down the others. + + `provider_filter` (when non-None) restricts to a named subset. Matching + is case-insensitive against the provider's short name (e.g. "pp", + "seats"). A filter that names no configured providers yields an empty + list — the caller decides whether that's a hard error (--awards-only) + or silent skip (cash-only path). """ out: list[AwardProvider] = [] - if pp_is_configured(): + allow_pp = provider_filter is None or _matches(provider_filter, "pp") + if allow_pp and pp_is_configured(): try: out.append(await PointsPathProvider.create(explicit_airlines=pp_airlines)) except Exception as e: # noqa: BLE001 — per-provider failures are non-fatal @@ -48,6 +56,11 @@ async def _construct_enabled( return out +def _matches(filter_: tuple[str, ...], name: str) -> bool: + """Case-insensitive membership test for the provider filter.""" + return any(p.strip().lower() == name.lower() for p in filter_) + + async def _gather_one_leg( providers: list[AwardProvider], leg: LegQuery, @@ -90,6 +103,7 @@ async def gather_awards( num_passengers: int = 1, pp_airlines: tuple[str, ...] | None = None, cash_hints_per_leg: list[tuple[CashFlightHint, ...]] | None = None, + provider_filter: tuple[str, ...] | None = None, ) -> tuple[list[list[AwardFlight]], list[AwardProvider]]: """End-to-end registry call: construct enabled providers, fan out per leg. @@ -106,7 +120,10 @@ async def gather_awards( providers is the constructed provider instances — the caller is responsible for closing them (PointsPath uses HTTP keepalive). """ - providers = await _construct_enabled(pp_airlines=pp_airlines) + providers = await _construct_enabled( + pp_airlines=pp_airlines, + provider_filter=provider_filter, + ) per_leg: list[list[AwardFlight]] = [] for i, leg in enumerate(legs): hints: tuple[CashFlightHint, ...] = () diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..438c8a4 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,117 @@ +# pyright: reportPrivateUsage=false, reportUnusedFunction=false +"""Tests for the user-level config.toml loader. + +The loader powers the new --providers / --provider-opt surface (work-4byx) +and is the seed of the broader surface-hygiene config story (work-4uls). +Round-trip tests pin: missing file → {}, partial sections → preserved verbatim, +provider lookups by name, CLI override parsing, merge semantics. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from flight_cli import _config + +if TYPE_CHECKING: + from pathlib import Path + + from pytest import MonkeyPatch + + +@pytest.fixture +def _config_dir(tmp_path: Path, monkeypatch: MonkeyPatch) -> Path: + monkeypatch.setenv(_config.CONFIG_DIR_ENV, str(tmp_path)) + return tmp_path + + +def test_load_missing_file_returns_empty(_config_dir: Path) -> None: + assert _config.load() == {} + + +def test_load_round_trip(_config_dir: Path) -> None: + (_config_dir / "config.toml").write_text( + """\ +[providers.pp] +airlines = ["United", "Delta"] +cabins = ["Economy", "Business"] + +[providers.seats] +api_key = "secret" +""" + ) + cfg = _config.load() + assert cfg["providers"]["pp"]["airlines"] == ["United", "Delta"] + assert cfg["providers"]["pp"]["cabins"] == ["Economy", "Business"] + assert cfg["providers"]["seats"]["api_key"] == "secret" + + +def test_provider_options_named_lookup(_config_dir: Path) -> None: + (_config_dir / "config.toml").write_text( + """\ +[providers.pp] +airlines = ["United"] +""" + ) + assert _config.provider_options("pp") == {"airlines": ["United"]} + assert _config.provider_options("seats") == {} + + +def test_provider_options_missing_providers_section(_config_dir: Path) -> None: + (_config_dir / "config.toml").write_text("[http]\nrps = 2.0\n") + assert _config.provider_options("pp") == {} + + +def test_parse_provider_opt_csv_value() -> None: + parsed = _config.parse_provider_opt_overrides( + ["pp.airlines=United,Delta"], + ) + assert parsed == {"pp": {"airlines": ["United", "Delta"]}} + + +def test_parse_provider_opt_scalar_value() -> None: + parsed = _config.parse_provider_opt_overrides(["pp.api_key=secret"]) + assert parsed == {"pp": {"api_key": "secret"}} + + +def test_parse_provider_opt_multiple_providers() -> None: + parsed = _config.parse_provider_opt_overrides( + ["pp.airlines=United", "seats.api_key=k"], + ) + assert parsed == {"pp": {"airlines": "United"}, "seats": {"api_key": "k"}} + + +def test_parse_provider_opt_rejects_missing_eq() -> None: + with pytest.raises(ValueError, match="missing '='"): + _config.parse_provider_opt_overrides(["pp.airlines"]) + + +def test_parse_provider_opt_rejects_missing_dot() -> None: + with pytest.raises(ValueError, match=r"missing '\.'"): + _config.parse_provider_opt_overrides(["airlines=United"]) + + +def test_parse_provider_opt_rejects_empty_provider() -> None: + with pytest.raises(ValueError, match="must be non-empty"): + _config.parse_provider_opt_overrides([".airlines=United"]) + + +def test_merge_provider_options_inner_keys() -> None: + base = {"pp": {"airlines": ["A"], "cabins": ["Economy"]}} + override = {"pp": {"airlines": ["B", "C"]}} + merged = _config.merge_provider_options(base, override) + assert merged == { + "pp": {"airlines": ["B", "C"], "cabins": ["Economy"]}, + } + + +def test_merge_provider_options_adds_new_provider() -> None: + base = {"pp": {"airlines": ["A"]}} + override = {"seats": {"api_key": "k"}} + merged = _config.merge_provider_options(base, override) + assert merged == { + "pp": {"airlines": ["A"]}, + "seats": {"api_key": "k"}, + } diff --git a/tests/test_provider_selection.py b/tests/test_provider_selection.py new file mode 100644 index 0000000..4ffde46 --- /dev/null +++ b/tests/test_provider_selection.py @@ -0,0 +1,225 @@ +# pyright: reportPrivateUsage=false, reportUnusedFunction=false +"""Tests for the provider-selection resolver in cli.py. + +Covers: + - New surface: --providers / --cash-only / --awards-only / --provider-opt + - Deprecated alias forwarding: --no-pp / --pp-only / --pp-airlines / --pp-cabin + - Conflict detection across old + new surface + - Config.toml merging with CLI overrides +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest +import typer + +from flight_cli import _config +from flight_cli.cli import _resolve_providers + +if TYPE_CHECKING: + from pathlib import Path + + from pytest import MonkeyPatch + + +@pytest.fixture +def _config_dir(tmp_path: Path, monkeypatch: MonkeyPatch) -> Path: + monkeypatch.setenv(_config.CONFIG_DIR_ENV, str(tmp_path)) + return tmp_path + + +def test_default_resolution_no_flags(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + ) + assert sel.provider_filter is None + assert sel.cash_only is False + assert sel.awards_only is False + assert sel.provider_opts == {} + + +def test_providers_csv_parsing(_config_dir: Path) -> None: + sel = _resolve_providers( + providers="pp,seats", + cash_only=False, + awards_only=False, + provider_opt=(), + ) + assert sel.provider_filter == ("pp", "seats") + + +def test_providers_empty_string_rejected(_config_dir: Path) -> None: + with pytest.raises(typer.Exit): + _resolve_providers( + providers=",,", + cash_only=False, + awards_only=False, + provider_opt=(), + ) + + +def test_cash_only_and_awards_only_conflict(_config_dir: Path) -> None: + with pytest.raises(typer.Exit): + _resolve_providers( + providers=None, + cash_only=True, + awards_only=True, + provider_opt=(), + ) + + +def test_legacy_no_pp_forwards_to_cash_only(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_no_pp=True, + ) + assert sel.cash_only is True + + +def test_legacy_pp_only_forwards_to_awards_only(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_pp_only=True, + ) + assert sel.awards_only is True + + +def test_legacy_pp_airlines_forwards_to_provider_opts(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_pp_airlines="United,Delta", + ) + assert sel.provider_opts == {"pp": {"airlines": ["United", "Delta"]}} + assert sel.pp_airlines() == "United,Delta" + + +def test_legacy_pp_cabin_forwards_to_provider_opts(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_pp_cabin="Economy,Business", + ) + assert sel.provider_opts == {"pp": {"cabins": ["Economy", "Business"]}} + assert sel.pp_cabins() == "Economy,Business" + + +def test_legacy_no_pp_conflicts_with_new_surface(_config_dir: Path) -> None: + with pytest.raises(typer.Exit): + _resolve_providers( + providers=None, + cash_only=True, + awards_only=False, + provider_opt=(), + legacy_no_pp=True, + ) + + +def test_legacy_pp_only_conflicts_with_providers(_config_dir: Path) -> None: + with pytest.raises(typer.Exit): + _resolve_providers( + providers="pp", + cash_only=False, + awards_only=False, + provider_opt=(), + legacy_pp_only=True, + ) + + +def test_provider_opt_cli_only(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.airlines=United,Delta",), + ) + assert sel.provider_opts == {"pp": {"airlines": ["United", "Delta"]}} + + +def test_config_toml_seeds_provider_opts(_config_dir: Path) -> None: + (_config_dir / "config.toml").write_text( + """\ +[providers.pp] +airlines = ["United"] +cabins = ["Economy"] +""" + ) + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=(), + ) + assert sel.provider_opts == {"pp": {"airlines": ["United"], "cabins": ["Economy"]}} + + +def test_cli_override_wins_over_config(_config_dir: Path) -> None: + (_config_dir / "config.toml").write_text('[providers.pp]\nairlines = ["United"]\n') + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.airlines=Delta",), + ) + assert sel.provider_opts["pp"]["airlines"] == "Delta" + + +def test_cli_provider_opt_preempts_legacy_pp_airlines(_config_dir: Path) -> None: + """If both --provider-opt and --pp-airlines are set, --provider-opt wins. + + Rationale: the new surface is preferred; legacy is only for unmigrated + scripts. Erroring would break the migration path; silent wins-old would + surprise the user who reached for the new flag.""" + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.airlines=Delta",), + legacy_pp_airlines="United,JetBlue", + ) + assert sel.provider_opts["pp"]["airlines"] == "Delta" + + +def test_pp_airlines_helper_handles_list(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.airlines=United,Delta",), + ) + assert sel.pp_airlines() == "United,Delta" + + +def test_pp_cabins_helper_handles_scalar(_config_dir: Path) -> None: + sel = _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.cabins=Economy",), + ) + assert sel.pp_cabins() == "Economy" + + +def test_invalid_provider_opt_format(_config_dir: Path) -> None: + with pytest.raises(typer.Exit): + _resolve_providers( + providers=None, + cash_only=False, + awards_only=False, + provider_opt=("pp.airlines",), # missing = + ) diff --git a/tests/test_registry.py b/tests/test_registry.py index ba86c17..8bf6f0a 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -14,7 +14,7 @@ import pytest from flight_cli.providers.base import AwardFlight, AwardProvider, LegQuery -from flight_cli.providers.registry import _gather_one_leg +from flight_cli.providers.registry import _gather_one_leg, _matches if TYPE_CHECKING: from flight_cli.pp.client import CashFlightHint @@ -105,3 +105,24 @@ async def go() -> list[AwardFlight]: out: list[AwardFlight] = anyio.run(go) assert len(out) == count + + +# ─────────────────────────── provider filter (work-4byx) ───────────────── + + +def test_matches_is_case_insensitive() -> None: + assert _matches(("pp",), "PP") is True + assert _matches(("PP",), "pp") is True + assert _matches(("Pp",), "pp") is True + + +def test_matches_trims_whitespace() -> None: + assert _matches((" pp ",), "pp") is True + + +def test_matches_returns_false_for_unknown_provider() -> None: + assert _matches(("pp",), "seats") is False + + +def test_matches_empty_filter_returns_false() -> None: + assert _matches((), "pp") is False