gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cleanup: strip stale TODO markers from data integration tests

Browse Source 
Removes commented-out ProviderMetrics tests (struct was replaced by
ConnectionStatus long ago) and reword the Parquet-reader integration
tests so they stop claiming the reader is a "placeholder" — the Parquet
reader is fully implemented and surfaces `File::open` errors via
anyhow. Also tightens test_12_invalid_file_handling to assert the real
Err behaviour rather than the stale "Ok(vec![])" expectation.

No production code change; tests still compile and run under the same
ignore gates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-04-23 08:27:33 +02:00
parent 7f92fa242c
commit 8251a3bf67
3 changed files with 22 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
crates/data/tests/comprehensive_coverage_tests.rs
View File

@@ -425,38 +425,6 @@ fn test_connection_state_transitions() {
}
}
// TODO: ProviderMetrics has been removed from the API - needs rewrite for new ConnectionStatus
// #[test]
// fn test_provider_metrics_initialization() {
// let metrics = ProviderMetrics {
// messages_received: 0,
// messages_sent: 0,
// errors_count: 0,
// reconnections: 0,
// last_heartbeat: None,
// uptime_seconds: 0,
// };
//
// assert_eq!(metrics.messages_received, 0);
// assert!(metrics.last_heartbeat.is_none());
// }
// TODO: ProviderMetrics has been removed from the API - needs rewrite for new ConnectionStatus
// #[test]
// fn test_provider_metrics_edge_values() {
// let metrics = ProviderMetrics {
// messages_received: u64::MAX,
// messages_sent: u64::MAX,
// errors_count: u64::MAX,
// reconnections: u64::MAX,
// last_heartbeat: Some(Utc::now()),
// uptime_seconds: u64::MAX,
// };
//
// assert_eq!(metrics.messages_received, u64::MAX);
// assert_eq!(metrics.uptime_seconds, u64::MAX);
// }
// ============================================================================
// Utils Module Tests - Edge Cases
// ============================================================================
@@ -646,18 +614,3 @@ fn test_empty_string_errors() {
assert!(matches!(err, DataError::Configuration { .. }));
}
// TODO: ProviderMetrics has been removed from the API - needs rewrite for new ConnectionStatus
// #[test]
// fn test_zero_values() {
// let metrics = ProviderMetrics {
// messages_received: 0,
// messages_sent: 0,
// errors_count: 0,
// reconnections: 0,
// last_heartbeat: None,
// uptime_seconds: 0,
// };
//
// assert_eq!(metrics.messages_received, 0);
// assert_eq!(metrics.uptime_seconds, 0);
// }

45
crates/data/tests/provider_error_path_tests.rs
View File

@@ -1,11 +1,9 @@
//! Provider-specific error path and edge case tests
//!
//! Targets uncovered error handling in databento and benzinga providers
//!
//! NOTE: Many tests commented out due to API changes:
//! - ProviderMetrics removed (now using ConnectionStatus)
//! - Databento types::Dataset and types::Schema need conditional compilation
//! TODO: Rewrite tests to match current APIs
//! Targets uncovered error handling in databento and benzinga providers.
//! ProviderMetrics-based tests have been removed after the API switch to
//! `ConnectionStatus`; the remaining tests exercise error categories,
//! state-machine transitions, and parse/validation edge cases directly.
#![allow(
clippy::absurd_extreme_comparisons,
clippy::assertions_on_constants,
@@ -68,8 +66,6 @@ use chrono::{Duration, Utc};
use data::error::DataError;
// Import ConnectionState from providers module which re-exports from traits
use data::providers::ConnectionState;
// TODO: ProviderMetrics removed - use ConnectionStatus instead
// use data::providers::common::ProviderMetrics;
#[cfg(feature = "databento")]
use data::providers::databento::types::{DatabentoDataset as Dataset, DatabentoSchema as Schema};
use std::collections::HashMap;
@@ -286,24 +282,6 @@ fn test_streaming_buffer_overflow() {
}
}
// TODO: ProviderMetrics has been removed - needs rewrite for new ConnectionStatus
// #[test]
// fn test_streaming_backpressure() {
// // Test backpressure handling
// let metrics = ProviderMetrics {
// messages_received: 1_000_000,
// messages_sent: 500_000, // Backlog
// errors_count: 0,
// reconnections: 0,
// last_heartbeat: Some(Utc::now()),
// uptime_seconds: 3600,
// };
//
// let backlog = metrics.messages_received - metrics.messages_sent;
// assert!(backlog > 0);
// assert_eq!(backlog, 500_000);
// }
// ============================================================================
// Reconnection Logic Tests
// ============================================================================
@@ -371,21 +349,6 @@ fn test_heartbeat_timeout_detection() {
assert!(is_timeout);
}
// TODO: ProviderMetrics has been removed - needs rewrite for new ConnectionStatus
// #[test]
// fn test_heartbeat_none_case() {
// let metrics = ProviderMetrics {
// messages_received: 0,
// messages_sent: 0,
// errors_count: 0,
// reconnections: 0,
// last_heartbeat: None, // Never received heartbeat
// uptime_seconds: 0,
// };
//
// assert!(metrics.last_heartbeat.is_none());
// }
// ============================================================================
// Data Format Conversion Tests
// ============================================================================

49
crates/data/tests/real_data_integration_tests.rs
View File

@@ -143,14 +143,9 @@ async fn test_01_load_btc_parquet() {
let reader = ParquetMarketDataReader::new(base_path);
let events = reader.read_file(BTC_FILE).await;
// Note: Current implementation returns empty Vec (placeholder)
// This test validates the reader can be called without panic
// Smoke-level assertion: the reader is expected to succeed on the fixture.
// Row-count / symbol assertions live in `test_03_btc_schema_validation`.
assert!(events.is_ok(), "Failed to read BTC Parquet file");
// TODO: Once reader is fully implemented, validate:
// let events = events.unwrap();
// assert_eq!(events.len(), BTC_EXPECTED_ROWS);
// assert!(events[0].symbol.contains("BTC"));
}
#[tokio::test]
@@ -171,12 +166,9 @@ async fn test_02_load_eth_parquet() {
let reader = ParquetMarketDataReader::new(base_path);
let events = reader.read_file(ETH_FILE).await;
// Smoke-level assertion; row-count / symbol checks live in
// `test_04_eth_schema_validation`.
assert!(events.is_ok(), "Failed to read ETH Parquet file");
// TODO: Once reader is fully implemented, validate:
// let events = events.unwrap();
// assert_eq!(events.len(), ETH_EXPECTED_ROWS);
// assert!(events[0].symbol.contains("ETH"));
}
// ============================================================================
@@ -480,13 +472,10 @@ async fn test_09_backtesting_integration() {
return;
}
// TODO: Once backtesting service integration is ready:
// 1. Load Parquet data
// 2. Create backtest config using real data
// 3. Run 1-day backtest
// 4. Validate results (PnL calculated, no errors)
// Placeholder test that verifies files are accessible
// This test currently only verifies that the Parquet fixture can be
// opened by the reader. End-to-end backtest wiring (config construction,
// 1-day replay, PnL validation) is exercised by the backtesting crate's
// own integration tests; repeating it here would duplicate that coverage.
let base_path = Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.unwrap()
@@ -511,13 +500,10 @@ async fn test_10_feature_extraction() {
return;
}
// TODO: Once feature extraction is integrated:
// 1. Load BTC data
// 2. Extract features using FeatureProcessor
// 3. Verify 32 dimensions
// 4. Check OHLCV + 27 technical indicators
// Placeholder test
// Feature-extraction correctness (32-dim OHLCV + 27 technical indicators)
// is covered by the ml_features crate's own tests. This test only
// verifies that the Parquet fixture the feature processor consumes is
// readable from the data crate side.
let base_path = Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.unwrap()
@@ -593,12 +579,13 @@ async fn test_12_invalid_file_handling() {
let reader = ParquetMarketDataReader::new(base_path);
// Test with non-existent file
// A non-existent file surfaces the `File::open` error through anyhow's
// context chain, so the reader returns `Err` rather than an empty Vec.
let result = reader.read_file("nonexistent.parquet").await;
// Current implementation returns Ok(vec![]) for placeholder
// TODO: Once reader is fully implemented, this should return Err
assert!(result.is_ok(), "Should handle non-existent file gracefully");
assert!(
result.is_err(),
"Non-existent Parquet file should yield Err from the reader"
);
}
// ============================================================================