From e78d9fcecd669bff0695b42131cdd5531ce8a0a7 Mon Sep 17 00:00:00 2001 From: hermes Date: Wed, 14 Oct 2026 09:31:12 +0000 Subject: [PATCH] refactor(core): compile the mock provider only with a testing feature Phase 3a of docs/DESIGN.md. MockProvider moves to api/mock.rs, and it, the embedded MSN fixture bodies, and the IDX_MOCK_* hooks compile only with idx-core's new `testing` feature. idx-cli enables it as a dev-dependency, so `cargo test` keeps the fixture-backed provider while release binaries no longer embed fixture data or honor IDX_USE_MOCK_PROVIDER. Parser helpers used only by the mock are gated the same way. The packaged-cli flake check no longer relies on the mock; it checks an offline cache miss (JSON CACHEMISS, exit 3) instead. Goldens unchanged; clippy is clean with and without `testing`. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 +- crates/idx-cli/Cargo.toml | 2 + crates/idx-core/Cargo.toml | 2 + crates/idx-core/src/api/mock.rs | 119 +++++++++++++++ crates/idx-core/src/api/mod.rs | 195 ++++++------------------- crates/idx-core/src/api/msn/client.rs | 13 ++ crates/idx-core/src/api/msn/mod.rs | 1 + crates/idx-core/src/api/yahoo/mod.rs | 1 + crates/idx-core/src/api/yahoo/parse.rs | 15 +- docs/CONVENTIONS.md | 2 +- docs/DESIGN.md | 5 +- docs/SMOKE.md | 2 +- flake.nix | 15 +- 13 files changed, 214 insertions(+), 161 deletions(-) create mode 100644 crates/idx-core/src/api/mock.rs diff --git a/AGENTS.md b/AGENTS.md index 122a90a..aedb82f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,8 @@ Docs: ## Rules 1. Keep data access provider-driven; no ad hoc fetches in `cli/`. 2. Keep parse/normalize functions pure. -3. Tests use fixtures and `IDX_USE_MOCK_PROVIDER=1`; never hit the live network. +3. Tests use fixtures and `IDX_USE_MOCK_PROVIDER=1` (needs idx-core's `testing` + feature, on under `cargo test`); never hit the live network. 4. Output contract: tables on stdout, one JSON document with `-o json`, errors on stderr (JSON-shaped in JSON mode), stdout empty on failure. 5. Keep ownership queries (`releases`, `ticker`, `changes`, ...) working across diff --git a/crates/idx-cli/Cargo.toml b/crates/idx-cli/Cargo.toml index 53fa329..debf2a1 100644 --- a/crates/idx-cli/Cargo.toml +++ b/crates/idx-cli/Cargo.toml @@ -36,6 +36,8 @@ sha2 = { workspace = true, optional = true } ureq = { workspace = true, optional = true } [dev-dependencies] +# Tests drive the binary with the mock provider; release builds exclude it. +idx-core = { workspace = true, features = ["testing"] } assert_cmd = "2" predicates = "3" rusqlite.workspace = true diff --git a/crates/idx-core/Cargo.toml b/crates/idx-core/Cargo.toml index 70b9b25..1ac22af 100644 --- a/crates/idx-core/Cargo.toml +++ b/crates/idx-core/Cargo.toml @@ -17,6 +17,8 @@ default = ["ownership"] ownership = ["dep:rusqlite", "dep:quick-xml", "dep:sha2", "dep:zip"] # Derives clap::ValueEnum on enums frontends expose as CLI arguments. clap = ["dep:clap"] +# Fixture-backed mock provider (IDX_USE_MOCK_PROVIDER) for tests only. +testing = [] [dependencies] clap = { workspace = true, optional = true } diff --git a/crates/idx-core/src/api/mock.rs b/crates/idx-core/src/api/mock.rs new file mode 100644 index 0000000..9959599 --- /dev/null +++ b/crates/idx-core/src/api/mock.rs @@ -0,0 +1,119 @@ +//! Fixture-backed provider for tests, enabled by the `testing` feature and +//! `IDX_USE_MOCK_PROVIDER`. Release builds do not contain it. + +use super::types::{Bar, Fundamentals, Interval, Period, Quote}; +use super::{FundamentalsProvider, HistoryProvider, QuoteProvider, msn, yahoo}; +use crate::config::ProviderKind; +use crate::error::IdxError; + +pub(crate) fn enabled() -> bool { + std::env::var("IDX_USE_MOCK_PROVIDER").is_ok() +} + +pub struct MockProvider { + quote: Result, + fundamentals: Result, + history: Result, IdxError>, +} + +impl MockProvider { + pub fn from_fixtures(provider: ProviderKind) -> Self { + Self::from_fixtures_with_history_verbose(provider, false) + } + + pub fn from_fixtures_with_history_verbose( + provider: ProviderKind, + history_verbose: bool, + ) -> Self { + if std::env::var("IDX_MOCK_ERROR").is_ok() { + return Self::with_error(IdxError::ProviderUnavailable); + } + + match provider { + ProviderKind::Yahoo => Self::from_yahoo_fixtures(history_verbose), + ProviderKind::Msn => Self::from_msn_fixtures(), + } + } + + fn from_yahoo_fixtures(history_verbose: bool) -> Self { + let quote_raw = std::fs::read_to_string("tests/fixtures/chart_bbca_1d.json") + .unwrap_or_else(|_| "{}".to_string()); + let history_path = std::env::var("IDX_MOCK_YAHOO_HISTORY_FIXTURE") + .unwrap_or_else(|_| "tests/fixtures/chart_bbca_3mo.json".to_string()); + let history_raw = + std::fs::read_to_string(&history_path).unwrap_or_else(|_| "{}".to_string()); + let fundamentals_raw = std::fs::read_to_string("tests/fixtures/quotesummary_bbca.json") + .unwrap_or_else(|_| "{}".to_string()); + + let quote = yahoo::parse_quote_from_str("BBCA.JK", "e_raw) + .map_err(|e| IdxError::ParseError(e.to_string())); + let fundamentals = yahoo::parse_fundamentals_from_str("BBCA.JK", &fundamentals_raw) + .map_err(|e| IdxError::ParseError(e.to_string())); + let history = + yahoo::parse_history_from_str_with_verbose("BBCA.JK", &history_raw, history_verbose) + .map_err(|e| IdxError::ParseError(e.to_string())); + + Self { + quote, + fundamentals, + history, + } + } + + fn from_msn_fixtures() -> Self { + let quote_raw = std::fs::read_to_string("tests/fixtures/msn_quote_bbca.json") + .unwrap_or_else(|_| "[]".to_string()); + let fundamentals_path = std::env::var("IDX_MOCK_MSN_KEYRATIOS_FIXTURE") + .unwrap_or_else(|_| "tests/fixtures/msn_keyratios_bbca.json".to_string()); + let fundamentals_raw = + std::fs::read_to_string(&fundamentals_path).unwrap_or_else(|_| "[]".to_string()); + + let quote = msn::parse_quote_from_str("BBCA.JK", "e_raw) + .map_err(|e| IdxError::ParseError(e.to_string())); + let fundamentals = msn::parse_fundamentals_from_str(&fundamentals_raw, Some("e_raw)) + .map_err(|e| IdxError::ParseError(e.to_string())); + let history_raw = std::fs::read_to_string("tests/fixtures/msn_chart_bbca_3m.json") + .unwrap_or_else(|_| "[]".to_string()); + let history = msn::parse_history_from_str("BBCA.JK", &history_raw) + .map_err(|e| IdxError::ParseError(e.to_string())); + + Self { + quote, + fundamentals, + history, + } + } + + pub fn with_error(err: IdxError) -> Self { + Self { + quote: Err(err.clone()), + fundamentals: Err(err.clone()), + history: Err(err), + } + } +} + +impl QuoteProvider for MockProvider { + fn quote(&self, symbol: &str) -> Result { + let mut q = self.quote.clone()?; + q.symbol = symbol.to_string(); + Ok(q) + } +} + +impl FundamentalsProvider for MockProvider { + fn fundamentals(&self, _symbol: &str) -> Result { + self.fundamentals.clone() + } +} + +impl HistoryProvider for MockProvider { + fn history( + &self, + _symbol: &str, + _period: &Period, + _interval: &Interval, + ) -> Result, IdxError> { + self.history.clone() + } +} diff --git a/crates/idx-core/src/api/mod.rs b/crates/idx-core/src/api/mod.rs index 9fad9cb..624942e 100644 --- a/crates/idx-core/src/api/mod.rs +++ b/crates/idx-core/src/api/mod.rs @@ -1,3 +1,5 @@ +#[cfg(feature = "testing")] +pub mod mock; pub mod msn; pub mod types; pub mod yahoo; @@ -171,11 +173,36 @@ pub fn resolve_symbol(symbol: &str, exchange: &str) -> Result } pub fn default_provider(provider: ProviderKind, verbose: bool) -> SelectedProvider { - build_selected_provider( - provider, - verbose, - std::env::var("IDX_USE_MOCK_PROVIDER").is_ok(), - ) + build_selected_provider(provider, verbose, mock_enabled()) +} + +fn mock_enabled() -> bool { + #[cfg(feature = "testing")] + return mock::enabled(); + #[cfg(not(feature = "testing"))] + false +} + +#[cfg(feature = "testing")] +fn mock_market(provider: ProviderKind) -> Option> { + Some(Box::new(mock::MockProvider::from_fixtures(provider))) +} + +#[cfg(not(feature = "testing"))] +fn mock_market(_provider: ProviderKind) -> Option> { + None +} + +#[cfg(feature = "testing")] +fn mock_history(provider: ProviderKind, verbose: bool) -> Option> { + Some(Box::new( + mock::MockProvider::from_fixtures_with_history_verbose(provider, verbose), + )) +} + +#[cfg(not(feature = "testing"))] +fn mock_history(_provider: ProviderKind, _verbose: bool) -> Option> { + None } fn build_selected_provider( @@ -183,10 +210,15 @@ fn build_selected_provider( verbose: bool, use_mock: bool, ) -> SelectedProvider { - match (provider, use_mock) { - (ProviderKind::Yahoo, true) => SelectedProvider { + let mock = if use_mock { + mock_market(provider) + } else { + None + }; + match provider { + ProviderKind::Yahoo => SelectedProvider { kind: ProviderKind::Yahoo, - market: Box::new(MockProvider::from_fixtures(ProviderKind::Yahoo)), + market: mock.unwrap_or_else(|| Box::new(yahoo::YahooProvider::new(verbose))), profile: None, financials: None, earnings: None, @@ -195,31 +227,9 @@ fn build_selected_provider( news: None, screener: None, }, - (ProviderKind::Yahoo, false) => SelectedProvider { - kind: ProviderKind::Yahoo, - market: Box::new(yahoo::YahooProvider::new(verbose)), - profile: None, - financials: None, - earnings: None, - sentiment: None, - insights: None, - news: None, - screener: None, - }, - (ProviderKind::Msn, true) => SelectedProvider { + ProviderKind::Msn => SelectedProvider { kind: ProviderKind::Msn, - market: Box::new(MockProvider::from_fixtures(ProviderKind::Msn)), - profile: Some(Box::new(msn::MsnProvider::new(verbose))), - financials: Some(Box::new(msn::MsnProvider::new(verbose))), - earnings: Some(Box::new(msn::MsnProvider::new(verbose))), - sentiment: Some(Box::new(msn::MsnProvider::new(verbose))), - insights: Some(Box::new(msn::MsnProvider::new(verbose))), - news: Some(Box::new(msn::MsnProvider::new(verbose))), - screener: Some(Box::new(msn::MsnProvider::new(verbose))), - }, - (ProviderKind::Msn, false) => SelectedProvider { - kind: ProviderKind::Msn, - market: Box::new(msn::MsnProvider::new(verbose)), + market: mock.unwrap_or_else(|| Box::new(msn::MsnProvider::new(verbose))), profile: Some(Box::new(msn::MsnProvider::new(verbose))), financials: Some(Box::new(msn::MsnProvider::new(verbose))), earnings: Some(Box::new(msn::MsnProvider::new(verbose))), @@ -254,13 +264,10 @@ pub fn history_provider( }, }; - if std::env::var("IDX_USE_MOCK_PROVIDER").is_ok() { - return Ok(( - resolved, - Box::new(MockProvider::from_fixtures_with_history_verbose( - resolved, verbose, - )), - )); + if mock_enabled() + && let Some(mock) = mock_history(resolved, verbose) + { + return Ok((resolved, mock)); } match resolved { @@ -269,114 +276,6 @@ pub fn history_provider( } } -pub struct MockProvider { - quote: Result, - fundamentals: Result, - history: Result, IdxError>, -} - -impl MockProvider { - pub fn from_fixtures(provider: ProviderKind) -> Self { - Self::from_fixtures_with_history_verbose(provider, false) - } - - pub fn from_fixtures_with_history_verbose( - provider: ProviderKind, - history_verbose: bool, - ) -> Self { - if std::env::var("IDX_MOCK_ERROR").is_ok() { - return Self::with_error(IdxError::ProviderUnavailable); - } - - match provider { - ProviderKind::Yahoo => Self::from_yahoo_fixtures(history_verbose), - ProviderKind::Msn => Self::from_msn_fixtures(), - } - } - - fn from_yahoo_fixtures(history_verbose: bool) -> Self { - let quote_raw = std::fs::read_to_string("tests/fixtures/chart_bbca_1d.json") - .unwrap_or_else(|_| "{}".to_string()); - let history_path = std::env::var("IDX_MOCK_YAHOO_HISTORY_FIXTURE") - .unwrap_or_else(|_| "tests/fixtures/chart_bbca_3mo.json".to_string()); - let history_raw = - std::fs::read_to_string(&history_path).unwrap_or_else(|_| "{}".to_string()); - let fundamentals_raw = std::fs::read_to_string("tests/fixtures/quotesummary_bbca.json") - .unwrap_or_else(|_| "{}".to_string()); - - let quote = yahoo::parse_quote_from_str("BBCA.JK", "e_raw) - .map_err(|e| IdxError::ParseError(e.to_string())); - let fundamentals = yahoo::parse_fundamentals_from_str("BBCA.JK", &fundamentals_raw) - .map_err(|e| IdxError::ParseError(e.to_string())); - let history = - yahoo::parse_history_from_str_with_verbose("BBCA.JK", &history_raw, history_verbose) - .map_err(|e| IdxError::ParseError(e.to_string())); - - Self { - quote, - fundamentals, - history, - } - } - - fn from_msn_fixtures() -> Self { - let quote_raw = std::fs::read_to_string("tests/fixtures/msn_quote_bbca.json") - .unwrap_or_else(|_| "[]".to_string()); - let fundamentals_path = std::env::var("IDX_MOCK_MSN_KEYRATIOS_FIXTURE") - .unwrap_or_else(|_| "tests/fixtures/msn_keyratios_bbca.json".to_string()); - let fundamentals_raw = - std::fs::read_to_string(&fundamentals_path).unwrap_or_else(|_| "[]".to_string()); - - let quote = msn::parse_quote_from_str("BBCA.JK", "e_raw) - .map_err(|e| IdxError::ParseError(e.to_string())); - let fundamentals = msn::parse_fundamentals_from_str(&fundamentals_raw, Some("e_raw)) - .map_err(|e| IdxError::ParseError(e.to_string())); - let history_raw = std::fs::read_to_string("tests/fixtures/msn_chart_bbca_3m.json") - .unwrap_or_else(|_| "[]".to_string()); - let history = msn::parse_history_from_str("BBCA.JK", &history_raw) - .map_err(|e| IdxError::ParseError(e.to_string())); - - Self { - quote, - fundamentals, - history, - } - } - - pub fn with_error(err: IdxError) -> Self { - Self { - quote: Err(err.clone()), - fundamentals: Err(err.clone()), - history: Err(err), - } - } -} - -impl QuoteProvider for MockProvider { - fn quote(&self, symbol: &str) -> Result { - let mut q = self.quote.clone()?; - q.symbol = symbol.to_string(); - Ok(q) - } -} - -impl FundamentalsProvider for MockProvider { - fn fundamentals(&self, _symbol: &str) -> Result { - self.fundamentals.clone() - } -} - -impl HistoryProvider for MockProvider { - fn history( - &self, - _symbol: &str, - _period: &Period, - _interval: &Interval, - ) -> Result, IdxError> { - self.history.clone() - } -} - #[cfg(test)] mod tests { use super::{ProviderKind, build_selected_provider, resolve_symbol}; @@ -410,6 +309,7 @@ mod tests { assert!(provider.screener_provider("screen").is_ok()); } + #[cfg(feature = "testing")] #[test] fn selected_provider_exposes_expected_capabilities_for_mock_msn() { let provider = build_selected_provider(ProviderKind::Msn, false, true); @@ -438,6 +338,7 @@ mod tests { ); } + #[cfg(feature = "testing")] #[test] fn selected_provider_rejects_msn_only_capabilities_for_mock_yahoo() { let provider = build_selected_provider(ProviderKind::Yahoo, false, true); diff --git a/crates/idx-core/src/api/msn/client.rs b/crates/idx-core/src/api/msn/client.rs index fde0bf5..a62bff0 100644 --- a/crates/idx-core/src/api/msn/client.rs +++ b/crates/idx-core/src/api/msn/client.rs @@ -34,6 +34,17 @@ impl MsnClient { Self { agent } } + #[cfg(not(feature = "testing"))] + fn mock_error() -> Option { + None + } + + #[cfg(not(feature = "testing"))] + fn mock_body(_endpoint: &str) -> Option<&'static str> { + None + } + + #[cfg(feature = "testing")] fn mock_error() -> Option { if std::env::var("IDX_USE_MOCK_PROVIDER").is_ok() && std::env::var("IDX_MOCK_ERROR").is_ok() { @@ -43,6 +54,7 @@ impl MsnClient { } } + #[cfg(feature = "testing")] fn mock_body(endpoint: &str) -> Option<&'static str> { if std::env::var("IDX_USE_MOCK_PROVIDER").is_err() { return None; @@ -271,6 +283,7 @@ impl MsnClient { url: &str, request: &ScreenerRequest, ) -> Result { + #[cfg(feature = "testing")] if std::env::var("IDX_USE_MOCK_PROVIDER").is_ok() { if let Some(err) = Self::mock_error() { return Err(err); diff --git a/crates/idx-core/src/api/msn/mod.rs b/crates/idx-core/src/api/msn/mod.rs index 2278e6a..37bf2a3 100644 --- a/crates/idx-core/src/api/msn/mod.rs +++ b/crates/idx-core/src/api/msn/mod.rs @@ -23,6 +23,7 @@ use map::{ parse_news, parse_profile, parse_quote, parse_screener_results, parse_sentiment, }; +#[cfg(feature = "testing")] pub(crate) use parse::{parse_fundamentals_from_str, parse_history_from_str, parse_quote_from_str}; pub struct MsnProvider { diff --git a/crates/idx-core/src/api/yahoo/mod.rs b/crates/idx-core/src/api/yahoo/mod.rs index ca8fd7c..387ae07 100644 --- a/crates/idx-core/src/api/yahoo/mod.rs +++ b/crates/idx-core/src/api/yahoo/mod.rs @@ -11,6 +11,7 @@ use client::YahooClient; use map::{parse_fundamentals, parse_quote}; use parse::parse_history_with_verbose; +#[cfg(feature = "testing")] pub(crate) use parse::{ parse_fundamentals_from_str, parse_history_from_str_with_verbose, parse_quote_from_str, }; diff --git a/crates/idx-core/src/api/yahoo/parse.rs b/crates/idx-core/src/api/yahoo/parse.rs index c5a7008..6f0e80e 100644 --- a/crates/idx-core/src/api/yahoo/parse.rs +++ b/crates/idx-core/src/api/yahoo/parse.rs @@ -1,9 +1,16 @@ -use crate::api::types::{Fundamentals, Ohlc, Quote}; +use crate::api::types::Ohlc; +#[cfg(any(test, feature = "testing"))] +use crate::api::types::{Fundamentals, Quote}; use crate::error::IdxError; -use super::map::{parse_fundamentals, parse_history, parse_quote}; -use super::raw_types::{ChartResponse, QuoteSummaryResponse}; +use super::map::parse_history; +#[cfg(any(test, feature = "testing"))] +use super::map::{parse_fundamentals, parse_quote}; +use super::raw_types::ChartResponse; +#[cfg(any(test, feature = "testing"))] +use super::raw_types::QuoteSummaryResponse; +#[cfg(any(test, feature = "testing"))] pub(crate) fn parse_quote_from_str(symbol: &str, raw: &str) -> Result { let chart: ChartResponse = serde_json::from_str(raw).map_err(|e| IdxError::ParseError(e.to_string()))?; @@ -15,6 +22,7 @@ pub(crate) fn parse_history_from_str(symbol: &str, raw: &str) -> Result_` (e.g., `test_parse_id_number_with_dots`) - **Coverage over counts:** replacing redundant unit assertions with stronger integration scenarios must retain meaningful error, boundary, and data-integrity coverage. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 56615f8..497872f 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -94,8 +94,9 @@ Key decisions: - **Explicit configuration.** Core reads no environment variables. `IDX_*` env parsing, `config.toml` loading, and XDG resolution move to a `CoreConfig::load()` helper that frontends call; tests construct `CoreConfig` directly. -- **Mock provider behind a `testing` feature.** Today `MockProvider` ships in the - release binary and reads fixtures relative to the working directory. +- **Mock provider behind a `testing` feature.** Done: `MockProvider` and the + embedded MSN fixtures compile only with idx-core's `testing` feature, which + idx-cli enables as a dev-dependency. - **Typed inputs.** Screener filters, regions, statement and earnings filters become enums with `FromStr`, so validation is identical across frontends. diff --git a/docs/SMOKE.md b/docs/SMOKE.md index b8f97d3..6512288 100644 --- a/docs/SMOKE.md +++ b/docs/SMOKE.md @@ -10,7 +10,7 @@ offline with pinned tools: | `fmt`, `clippy` | formatting and lints (`--all-targets`) | | `test` | unit/module tests, `tests/cli.rs` CLI integration tests, and `tests/golden.rs` output parity | | `default`, `package` | release build and `cargo package` verification | -| `packaged-cli` | the wrapped Nix binary starts, emits JSON, and generates completions | +| `packaged-cli` | the wrapped Nix release binary emits JSON, generates completions, and returns a JSON `CACHEMISS` error with exit 3 offline | | `ownership-publisher`, `publisher-test` | packaged publisher entry point and its regression scenarios | | `install-script-sh`, `install-script-dash`, `shellcheck` | installer regressions and shell lint | diff --git a/flake.nix b/flake.nix index c83465c..2ede4f4 100644 --- a/flake.nix +++ b/flake.nix @@ -217,14 +217,17 @@ packaged-cli = pkgs.runCommand "idx-packaged-cli" { nativeBuildInputs = with pkgs; [ bash jq ]; } '' - # The mock provider reads fixtures relative to the working dir. - mkdir -p tests - cp -r ${./crates/idx-core/tests/fixtures} tests/fixtures - export HOME="$TMPDIR" + export HOME="$TMPDIR" XDG_CACHE_HOME="$TMPDIR/cache" XDG_CONFIG_HOME="$TMPDIR/config" ${idxPackage}/bin/idx -o json version | jq -e '.version | type == "string"' ${idxPackage}/bin/idx completions bash | bash -n - IDX_USE_MOCK_PROVIDER=1 ${idxPackage}/bin/idx -o json stocks quote BBCA \ - | jq -e '.[0].price | type == "number"' + # Release builds have no mock provider; an offline cache miss + # exercises the JSON error contract and exit-code classes. + set +e + ${idxPackage}/bin/idx --offline -o json stocks quote BBCA 2>err.json + status=$? + set -e + test "$status" -eq 3 + jq -e '.code == "CACHEMISS"' err.json touch "$out" ''; install-script-sh = installerCheck "sh";