From e91a0c9a6aca09ccbf7c99c8c30f514b52392a6e Mon Sep 17 00:00:00 2001
From: jgrusewski <jgrusewski@localhost>
Date: Thu, 23 Apr 2026 08:38:44 +0200
Subject: [PATCH] cleanup: wire storage.base_directory in training_pipeline
 tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The three TODO comments claiming \"TrainingPipelineConfig needs a new
struct\" were stale — the config already exposes `storage.base_directory`
(test_process_features_full_workflow_success already uses it). Wire
up the two previously-neutered tests so they actually exercise their
intended behaviour:

- `test_pipeline_creation_storage_dir_is_file_fails` now sets
  base_directory to a regular file and asserts pipeline creation
  returns Err (create_dir_all on a file path fails with ENOTDIR).
- `test_process_features_dataset_not_found` now points storage at a
  fresh tempdir so the NotFound assertion runs against an isolated
  state rather than the default on-disk location.
- The inline \"fixed from TODO comment\" note in the third test is
  replaced with a plain description of why the tempdir override is
  needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 crates/data/src/training_pipeline.rs | 33 ++++++++++++++--------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/crates/data/src/training_pipeline.rs b/crates/data/src/training_pipeline.rs
index f277ad308..e6251d16c 100644
--- a/crates/data/src/training_pipeline.rs
+++ b/crates/data/src/training_pipeline.rs
@@ -1304,28 +1304,27 @@ mod tests {
         assert!(p.benzinga_client.is_none());
     }
 
-    /// Tests that pipeline creation fails if the storage base directory
-    /// path points to an existing file, which prevents directory creation.
-    /// Currently, this validation is not implemented (TODO in progress).
+    /// Tests that pipeline creation returns an I/O error when the storage
+    /// base directory path points to an existing regular file — the
+    /// `create_dir_all` call in `TrainingDataPipeline::new` fails because
+    /// the path already exists and is not a directory.
     #[tokio::test]
     async fn test_pipeline_creation_storage_dir_is_file_fails() {
         // Arrange
         let dir = tempdir().unwrap();
-        let _file_path = dir.path().join("i_am_a_file");
-        File::create(&_file_path).unwrap(); // Create a file where a directory is expected
+        let file_path = dir.path().join("i_am_a_file");
+        File::create(&file_path).unwrap(); // Create a file where a directory is expected
 
-        let config = TrainingPipelineConfig::default();
-        // TODO: Re-implement test with new config structure
-        // config.storage.base_directory = file_path;
+        let mut config = TrainingPipelineConfig::default();
+        config.storage.base_directory = file_path;
 
         // Act
         let pipeline = TrainingDataPipeline::new(config).await;
 
-        // Assert: Until TODO is implemented, pipeline creation succeeds with default config
-        // In the future, this should fail when file_path is set as storage directory
+        // Assert
         assert!(
-            pipeline.is_ok(),
-            "TODO: Validation not yet implemented - should fail when storage dir is a file"
+            pipeline.is_err(),
+            "pipeline creation should fail when storage dir points at a regular file"
         );
     }
 
@@ -1350,10 +1349,9 @@ mod tests {
     #[tokio::test]
     async fn test_process_features_dataset_not_found() {
         // Arrange
-        let _dir = tempdir().unwrap();
-        let config = TrainingPipelineConfig::default();
-        // TODO: Re-implement test with new config structure
-        // config.storage.base_directory = dir.path().to_path_buf();
+        let dir = tempdir().unwrap();
+        let mut config = TrainingPipelineConfig::default();
+        config.storage.base_directory = dir.path().to_path_buf();
         let pipeline = TrainingDataPipeline::new(config).await.unwrap();
 
         // Act
@@ -1379,7 +1377,8 @@ mod tests {
         // Arrange
         let dir = tempdir().unwrap();
         let mut config = TrainingPipelineConfig::default();
-        // Set storage to use temp directory (fixed from TODO comment)
+        // Set storage to use temp directory so the test does not touch
+        // the default on-disk cache.
         config.storage.base_directory = dir.path().to_path_buf();
         // Disable strict validations for test to prevent filtering
         config.validation.timestamp_validation = false;