Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add before_each_task and after_each_task support #1127

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
52 changes: 26 additions & 26 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions src/lib/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,5 +638,13 @@ pub fn load(
}
}

for (name, _) in config.tasks.iter() {
match name.as_str() {
"before_each" => config.config.before_each_task = Some(name.to_string()),
"after_each" => config.config.after_each_task = Some(name.to_string()),
_ => {}
}
Comment on lines +641 to +646
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this loop actually do? what does it mutate?
meaning are you setting config.before_each_task="before_each"? if so why loop? why not just configure it? why the naming magic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out how to else get the before_each_task and after_each_task to be populated. Tried a bunch of solutions, this is the only one that seemed to make it accessible to future locations (specifically src/lib/execution_plan.rs).

}

Ok(config)
}
68 changes: 65 additions & 3 deletions src/lib/execution_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use fsio::path::{get_basename, get_parent_directory};
use glob::Pattern;
use indexmap::IndexMap;
use regex::Regex;
use std::borrow::Cow;
use std::collections::HashSet;
use std::env;
use std::path::Path;
Expand Down Expand Up @@ -481,8 +482,8 @@ impl<'a> ExecutionPlanBuilder<'a> {
skip_tasks_pattern,
skip_init_end_tasks,
} = *self;
let mut task_names = HashSet::new();
let mut steps = Vec::new();
let mut task_names = HashSet::<String>::new();
let mut steps = Vec::<Step>::new();
let default_crate_info = CrateInfo::new();
let crate_info = crate_info.unwrap_or(&default_crate_info);
let skip_init_end_tasks = skip_init_end_tasks || sub_flow;
Expand Down Expand Up @@ -549,6 +550,67 @@ impl<'a> ExecutionPlanBuilder<'a> {
};
}

Ok(ExecutionPlan { steps })
Ok(ExecutionPlan {
steps: Self::handle_before_each_and_after_each_tasks(&config, &steps)?.into_owned(),
})
}

/// `before_each` and `after_each` interspersal for the steps vector
fn handle_before_each_and_after_each_tasks<'b>(
config: &Config,
steps: &'b Vec<Step>,
) -> Result<Cow<'b, Vec<Step>>, CargoMakeError> {
let mut after_and_before_each = Vec::<Step>::with_capacity(2);
let mut handle_some = |name: &String| -> Result<u8, CargoMakeError> {
let task_config = get_normalized_task(config, name, false)?;
if !task_config.disabled.unwrap_or(false) {
after_and_before_each.push(Step {
name: name.to_string(),
config: task_config,
});
Ok(1)
} else {
Ok(0)
}
};
let has_after_each: u8 = match &config.config.after_each_task {
None => 0,
Some(after_each) => handle_some(&after_each)?,
};
let has_before_each: u8 = match &config.config.before_each_task {
None => 0,
Some(before_each) => handle_some(&before_each)?,
};
let scale: u8 = has_before_each + has_after_each;
if scale > 0 {
let before_and_after_each_len = scale as usize + 1;
let mut interspersed_steps =
Vec::<Step>::with_capacity(steps.len() * before_and_after_each_len);
let before_special = HashSet::from(["init", "init_task"]);
let end_special = HashSet::from(["end", "end_task"]);
SamuelMarks marked this conversation as resolved.
Show resolved Hide resolved
interspersed_steps.extend(steps.into_iter().flat_map(|e| -> Vec<Step> {
SamuelMarks marked this conversation as resolved.
Show resolved Hide resolved
let mut _steps = Vec::<Step>::with_capacity(before_and_after_each_len + 1);
if before_special.contains(e.name.as_str()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe i'm not following this right but you are only pushing an additional task for the init/end tasks but not for other tasks instead of the opposite?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is:

  • add a before_each (if exists) before each task; followed by an after_each (if exists) after each task

With two addenda:

  • if the task is the init task and the before_each task exists, don't prepend the before_each
  • else if the task is the end task and the after_each task exists, don't append the after_each

if has_after_each == 1 {
_steps.push(
unsafe { after_and_before_each.last().unwrap_unchecked() }.to_owned(),
SamuelMarks marked this conversation as resolved.
Show resolved Hide resolved
);
}
} else if end_special.contains(e.name.as_str()) {
if has_before_each == 1 {
_steps.push(
unsafe { after_and_before_each.first().unwrap_unchecked() }.to_owned(),
SamuelMarks marked this conversation as resolved.
Show resolved Hide resolved
);
}
} else {
_steps.extend(after_and_before_each.clone());
}
_steps.push(e.to_owned());
_steps
}));
Ok(Cow::Owned(interspersed_steps))
} else {
Ok(Cow::Borrowed(&steps))
}
}
}
26 changes: 26 additions & 0 deletions src/lib/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,10 @@ pub struct ConfigSection {
pub init_task: Option<String>,
/// End task name which will be invoked at the end of every run
pub end_task: Option<String>,
/// before_each task name which will be invoked before each task (except `init_task`)
pub before_each_task: Option<String>,
/// after_each task name which will be invoked after each task (except `end_task`)
pub after_each_task: Option<String>,
/// The name of the task to run in case of any error during the invocation of the flow
pub on_error_task: Option<String>,
/// The name of the task which runs legacy migration flows
Expand Down Expand Up @@ -2210,6 +2214,20 @@ impl ConfigSection {
));
}

if self.before_each_task.is_some() {
self.before_each_task = Some(get_namespaced_task_name(
namespace,
&self.before_each_task.clone().unwrap(),
));
}

if self.after_each_task.is_some() {
self.after_each_task = Some(get_namespaced_task_name(
namespace,
&self.after_each_task.clone().unwrap(),
));
}

if self.on_error_task.is_some() {
self.on_error_task = Some(get_namespaced_task_name(
namespace,
Expand Down Expand Up @@ -2250,6 +2268,14 @@ impl ConfigSection {
self.end_task = extended.end_task.clone();
}

if let Some(before_each_task) = &extended.before_each_task {
self.before_each_task = Some(before_each_task.clone());
}

if let Some(after_each_task) = &extended.after_each_task {
self.after_each_task = Some(after_each_task.clone());
}

if extended.on_error_task.is_some() {
self.on_error_task = extended.on_error_task.clone();
}
Expand Down