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

Conversation

SamuelMarks
Copy link
Contributor

@SamuelMarks SamuelMarks commented Jul 26, 2024

WiP (works but needs tests)


Use cases this facilitates:

…onfigSection` `struct` ; [src/lib/execution_plan.rs] Implement `before_each_task` and `after_each_task` via interspersal of steps
…er_each` in `cli::descriptor::load` ; [src/lib/execution_plan.rs] Return to using the already parsed out before|after each
Comment on lines +641 to +646
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()),
_ => {}
}
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).

src/lib/execution_plan.rs Outdated Show resolved Hide resolved
src/lib/execution_plan.rs Outdated Show resolved Hide resolved
src/lib/execution_plan.rs Outdated Show resolved Hide resolved
src/lib/execution_plan.rs Outdated Show resolved Hide resolved
let end_special = HashSet::from(["end", "end_task"]);
interspersed_steps.extend(steps.into_iter().flat_map(|e| -> Vec<Step> {
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants