From 365dd1b2cb4d9ddf528360c0bf8bd84d5345f953 Mon Sep 17 00:00:00 2001 From: Miles Johnson Date: Thu, 23 Jan 2025 17:13:14 -0800 Subject: [PATCH] Fix target state. --- crates/task-runner/src/task_runner.rs | 42 +++++++----- crates/task-runner/tests/task_runner_test.rs | 69 +++++--------------- 2 files changed, 42 insertions(+), 69 deletions(-) diff --git a/crates/task-runner/src/task_runner.rs b/crates/task-runner/src/task_runner.rs index e4b5353a2be..b01779a4dc7 100644 --- a/crates/task-runner/src/task_runner.rs +++ b/crates/task-runner/src/task_runner.rs @@ -43,6 +43,7 @@ pub struct TaskRunner<'task> { pub operations: OperationList, pub remote_state: Option>, pub report_item: TaskReportItem, + pub target_state: Option, } impl<'task> TaskRunner<'task> { @@ -76,6 +77,7 @@ impl<'task> TaskRunner<'task> { output_style: task.options.output_style, ..Default::default() }, + target_state: None, task, app, operations: OperationList::default(), @@ -93,7 +95,7 @@ impl<'task> TaskRunner<'task> { ) -> miette::Result> { // If a dependency has failed or been skipped, we should skip this task if !self.is_dependencies_complete(context)? { - self.skip(context)?; + self.skip()?; return Ok(None); } @@ -108,7 +110,7 @@ impl<'task> TaskRunner<'task> { let hash = self.generate_hash(context, node).await?; // Exit early if this build has already been cached/hashed - if self.hydrate(context, &hash).await? { + if self.hydrate(&hash).await? { return Ok(Some(hash)); } @@ -147,6 +149,11 @@ impl<'task> TaskRunner<'task> { match result { Ok(maybe_hash) => { + context.set_target_state( + &self.task.target, + self.target_state.take().unwrap_or(TargetState::Passthrough), + ); + self.report_item.hash = maybe_hash.clone(); self.app.console.reporter.on_task_completed( @@ -163,6 +170,11 @@ impl<'task> TaskRunner<'task> { }) } Err(error) => { + context.set_target_state( + &self.task.target, + self.target_state.take().unwrap_or(TargetState::Failed), + ); + self.inject_failed_task_execution(Some(&error))?; self.app.console.reporter.on_task_completed( @@ -485,7 +497,7 @@ impl<'task> TaskRunner<'task> { ) -> miette::Result<()> { // If the task is a no-operation, we should exit early if self.task.is_no_op() { - self.skip_no_op(context)?; + self.skip_no_op()?; return Ok(()); } @@ -546,7 +558,7 @@ impl<'task> TaskRunner<'task> { self.operations.merge(result.attempts); // Update the action state based on the result - context.set_target_state(&self.task.target, result.run_state); + self.target_state = Some(result.run_state); // If the execution as a whole failed, return the error. // We do this here instead of in `execute` so that we can @@ -572,8 +584,8 @@ impl<'task> TaskRunner<'task> { Ok(()) } - #[instrument(skip_all)] - pub fn skip(&mut self, context: &ActionContext) -> miette::Result<()> { + #[instrument(skip(self))] + pub fn skip(&mut self) -> miette::Result<()> { debug!(task_target = self.task.target.as_str(), "Skipping task"); self.operations.push(Operation::new_finished( @@ -581,13 +593,13 @@ impl<'task> TaskRunner<'task> { ActionStatus::Skipped, )); - context.set_target_state(&self.task.target, TargetState::Skipped); + self.target_state = Some(TargetState::Skipped); Ok(()) } - #[instrument(skip(self, context))] - pub fn skip_no_op(&mut self, context: &ActionContext) -> miette::Result<()> { + #[instrument(skip(self))] + pub fn skip_no_op(&mut self) -> miette::Result<()> { debug!( task_target = self.task.target.as_str(), "Skipping task as its a no-operation" @@ -598,10 +610,7 @@ impl<'task> TaskRunner<'task> { ActionStatus::Passed, )); - context.set_target_state( - &self.task.target, - TargetState::from_hash(self.report_item.hash.as_deref()), - ); + self.target_state = Some(TargetState::from_hash(self.report_item.hash.as_deref())); Ok(()) } @@ -648,8 +657,8 @@ impl<'task> TaskRunner<'task> { Ok(archived) } - #[instrument(skip(self, context))] - pub async fn hydrate(&mut self, context: &ActionContext, hash: &str) -> miette::Result { + #[instrument(skip(self))] + pub async fn hydrate(&mut self, hash: &str) -> miette::Result { let mut operation = Operation::output_hydration(); // Not cached @@ -745,9 +754,8 @@ impl<'task> TaskRunner<'task> { self.persist_state(&operation)?; - context.set_target_state(&self.task.target, TargetState::Passed(hash.to_owned())); - self.operations.push(operation); + self.target_state = Some(TargetState::Passed(hash.to_owned())); Ok(true) } diff --git a/crates/task-runner/tests/task_runner_test.rs b/crates/task-runner/tests/task_runner_test.rs index 78ac83abcc7..2109bf539a3 100644 --- a/crates/task-runner/tests/task_runner_test.rs +++ b/crates/task-runner/tests/task_runner_test.rs @@ -895,9 +895,8 @@ mod task_runner { async fn creates_an_operation() { let container = TaskRunnerContainer::new("runner", "base").await; let mut runner = container.create_runner(); - let context = ActionContext::default(); - runner.skip(&context).unwrap(); + runner.skip().unwrap(); let operation = runner.operations.last().unwrap(); @@ -909,18 +908,10 @@ mod task_runner { async fn sets_skipped_state() { let container = TaskRunnerContainer::new("runner", "base").await; let mut runner = container.create_runner(); - let context = ActionContext::default(); - runner.skip(&context).unwrap(); + runner.skip().unwrap(); - assert_eq!( - context - .target_states - .get(&runner.task.target) - .unwrap() - .get(), - &TargetState::Skipped - ); + assert_eq!(runner.target_state.as_ref().unwrap(), &TargetState::Skipped); } } @@ -931,9 +922,8 @@ mod task_runner { async fn creates_an_operation() { let container = TaskRunnerContainer::new("runner", "base").await; let mut runner = container.create_runner(); - let context = ActionContext::default(); - runner.skip_no_op(&context).unwrap(); + runner.skip_no_op().unwrap(); let operation = runner.operations.last().unwrap(); @@ -945,16 +935,11 @@ mod task_runner { async fn sets_passthrough_state() { let container = TaskRunnerContainer::new("runner", "base").await; let mut runner = container.create_runner(); - let context = ActionContext::default(); - runner.skip_no_op(&context).unwrap(); + runner.skip_no_op().unwrap(); assert_eq!( - context - .target_states - .get(&runner.task.target) - .unwrap() - .get(), + runner.target_state.as_ref().unwrap(), &TargetState::Passthrough ); } @@ -962,17 +947,12 @@ mod task_runner { async fn sets_completed_state() { let container = TaskRunnerContainer::new("runner", "base").await; let mut runner = container.create_runner(); - let context = ActionContext::default(); runner.report_item.hash = Some("hash123".into()); - runner.skip_no_op(&context).unwrap(); + runner.skip_no_op().unwrap(); assert_eq!( - context - .target_states - .get(&runner.task.target) - .unwrap() - .get(), + runner.target_state.as_ref().unwrap(), &TargetState::Passed("hash123".into()) ); } @@ -1046,8 +1026,7 @@ mod task_runner { let mut runner = container.create_runner(); - let context = ActionContext::default(); - let result = runner.hydrate(&context, "hash123").await.unwrap(); + let result = runner.hydrate("hash123").await.unwrap(); assert!(!result); @@ -1077,8 +1056,7 @@ mod task_runner { setup_previous_state(&container, &mut runner); - let context = ActionContext::default(); - let result = runner.hydrate(&context, "hash123").await.unwrap(); + let result = runner.hydrate("hash123").await.unwrap(); assert!(result); @@ -1095,15 +1073,10 @@ mod task_runner { setup_previous_state(&container, &mut runner); - let context = ActionContext::default(); - runner.hydrate(&context, "hash123").await.unwrap(); + runner.hydrate("hash123").await.unwrap(); assert_eq!( - context - .target_states - .get(&runner.task.target) - .unwrap() - .get(), + runner.target_state.as_ref().unwrap(), &TargetState::Passed("hash123".into()) ); } @@ -1127,8 +1100,7 @@ mod task_runner { setup_local_state(&container, &mut runner); - let context = ActionContext::default(); - let result = runner.hydrate(&context, "hash123").await.unwrap(); + let result = runner.hydrate("hash123").await.unwrap(); assert!(result); @@ -1145,15 +1117,10 @@ mod task_runner { setup_local_state(&container, &mut runner); - let context = ActionContext::default(); - runner.hydrate(&context, "hash123").await.unwrap(); + runner.hydrate("hash123").await.unwrap(); assert_eq!( - context - .target_states - .get(&runner.task.target) - .unwrap() - .get(), + runner.target_state.as_ref().unwrap(), &TargetState::Passed("hash123".into()) ); } @@ -1165,8 +1132,7 @@ mod task_runner { setup_local_state(&container, &mut runner); - let context = ActionContext::default(); - runner.hydrate(&context, "hash123").await.unwrap(); + runner.hydrate("hash123").await.unwrap(); let output_file = container.sandbox.path().join("project/file.txt"); @@ -1181,8 +1147,7 @@ mod task_runner { setup_local_state(&container, &mut runner); - let context = ActionContext::default(); - let result = runner.hydrate(&context, "hash123").await.unwrap(); + let result = runner.hydrate("hash123").await.unwrap(); assert!(result);