diff --git a/tutorial/src/3_min_sound/6_hidden_dep/index.md b/tutorial/src/3_min_sound/6_hidden_dep/index.md index 64b1611..e6a5000 100644 --- a/tutorial/src/3_min_sound/6_hidden_dep/index.md +++ b/tutorial/src/3_min_sound/6_hidden_dep/index.md @@ -15,7 +15,7 @@ In this section, we will: 1) Create tests to showcase the hidden dependency problem. 2) Prevent hidden dependencies by checking for them at runtime, fixing the issue. -3) Improve and add additional tests +3) Improve and add additional tests. ## Test to showcase the issue diff --git a/tutorial/src/3_min_sound/7_cycle/a_task.rs b/tutorial/src/3_min_sound/7_cycle/a_task.rs new file mode 100644 index 0000000..e9d5eb3 --- /dev/null +++ b/tutorial/src/3_min_sound/7_cycle/a_task.rs @@ -0,0 +1,142 @@ +use std::io::{BufWriter, ErrorKind, Read, Stdout}; +use std::path::PathBuf; + +use dev_shared::write_until_modified; +use pie::{Context, Pie, Task}; +use pie::stamp::FileStamper; +use pie::tracker::CompositeTracker; +use pie::tracker::event::EventTracker; +use pie::tracker::writing::WritingTracker; + +/// Testing tracker composed of an [`EventTracker`] for testing and stdout [`WritingTracker`] for debugging. +pub type TestTracker = CompositeTracker::Output>, WritingTracker>>; +pub fn test_tracker() -> TestTracker { + CompositeTracker(EventTracker::default(), WritingTracker::with_stdout()) +} + +/// Testing [`Pie`] using [`TestTracker`]. +pub type TestPie = Pie::Output, TestTracker>; +pub fn test_pie() -> TestPie { + TestPie::with_tracker(test_tracker()) +} + +/// Testing extensions for [`TestPie`]. +pub trait TestPieExt { + /// Require `task` in a new session, assert that there are no dependency check errors, then runs `test_assert_func` + /// on the event tracker for test assertion purposes. + fn require_then_assert( + &mut self, + task: &T, + test_assert_func: impl FnOnce(&EventTracker), + ) -> T::Output; + + /// Require `task` in a new session, asserts that there are no dependency check errors. + fn require(&mut self, task: &T) -> T::Output { + self.require_then_assert(task, |_| {}) + } + + /// Require `task` in a new session, then assert that it is not executed. + fn require_then_assert_no_execute(&mut self, task: &T) -> T::Output { + self.require_then_assert(task, |t| + assert!(!t.any_execute_of(task), "expected no execution of task {:?}, but it was executed", task), + ) + } + /// Require `task` in a new session, then assert that it is executed exactly once. + fn require_then_assert_one_execute(&mut self, task: &T) -> T::Output { + self.require_then_assert(task, |t| + assert!(t.one_execute_of(task), "expected one execution of task {:?}, but it was not executed, or was executed more than once", task), + ) + } +} +impl TestPieExt for TestPie { + fn require_then_assert(&mut self, task: &T, test_assert_func: impl FnOnce(&EventTracker)) -> T::Output { + let mut session = self.new_session(); + let output = session.require(task); + assert!(session.dependency_check_errors().is_empty(), "expected no dependency checking errors, but there are \ + dependency checking errors: {:?}", session.dependency_check_errors()); + test_assert_func(&self.tracker().0); + output + } +} + +/// Testing tasks enumeration. +#[derive(Clone, Eq, PartialEq, Hash, Debug)] +pub enum TestTask { + Return(&'static str), + ReadFile(PathBuf, FileStamper, Option>), + WriteFile(Box, PathBuf, FileStamper), + ToLower(Box), + ToUpper(Box), + Sequence(Vec), + RequireSelf, + RequireA, + RequireB, +} +impl Task for TestTask { + type Output = Result; + fn execute>(&self, context: &mut C) -> Self::Output { + match self { + TestTask::Return(string) => Ok(string.to_string().into()), + TestTask::ReadFile(path, stamper, origin) => { + if let Some(origin) = origin { + context.require_task(origin)?; + } + let mut string = String::new(); + if let Some(mut file) = context.require_file_with_stamper(path, *stamper).map_err(|e| e.kind())? { + file.read_to_string(&mut string).map_err(|e| e.kind())?; + } + Ok(string.into()) + } + TestTask::WriteFile(string_provider_task, path, stamper) => { + let string = context.require_task(string_provider_task.as_ref())?.into_string(); + write_until_modified(path, string.as_bytes()).map_err(|e| e.kind())?; + context.provide_file_with_stamper(path, *stamper).map_err(|e| e.kind())?; + Ok(TestOutput::Unit) + } + TestTask::ToLower(string_provider_task) => { + let string = context.require_task(string_provider_task)?.into_string(); + Ok(string.to_lowercase().into()) + } + TestTask::ToUpper(string_provider_task) => { + let string = context.require_task(string_provider_task)?.into_string(); + Ok(string.to_uppercase().into()) + } + TestTask::Sequence(tasks) => { + for task in tasks { + context.require_task(task)?; + } + Ok(TestOutput::Unit) + } + TestTask::RequireSelf => context.require_task(&TestTask::RequireSelf), + TestTask::RequireA => context.require_task(&TestTask::RequireB), + TestTask::RequireB => context.require_task(&TestTask::RequireA), + } + } +} + +/// [`TestTask`] output enumeration. +#[derive(Clone, Eq, PartialEq, Hash, Debug)] +pub enum TestOutput { + String(String), + Unit, +} +impl From for TestOutput { + fn from(value: String) -> Self { Self::String(value) } +} +impl From<()> for TestOutput { + fn from(_: ()) -> Self { Self::Unit } +} +impl TestOutput { + pub fn as_str(&self) -> &str { + match self { + Self::String(s) => &s, + _ => panic!("{:?} does not contain a string", self), + } + } + pub fn into_string(self) -> String { + match self { + Self::String(s) => s, + _ => panic!("{:?} does not contain a string", self), + } + } +} diff --git a/tutorial/src/3_min_sound/7_cycle/b_test.rs b/tutorial/src/3_min_sound/7_cycle/b_test.rs new file mode 100644 index 0000000..f6aff5c --- /dev/null +++ b/tutorial/src/3_min_sound/7_cycle/b_test.rs @@ -0,0 +1,24 @@ + + +// Cycle tests + +#[test] +#[should_panic(expected = "Cyclic task dependency")] +fn require_self_panics() { + let mut pie = test_pie(); + pie.require(&RequireSelf).unwrap(); +} + +#[test] +#[should_panic(expected = "Cyclic task dependency")] +fn require_cycle_a_panics() { + let mut pie = test_pie(); + pie.require(&RequireA).unwrap(); +} + +#[test] +#[should_panic(expected = "Cyclic task dependency")] +fn require_cycle_b_panics() { + let mut pie = test_pie(); + pie.require(&RequireB).unwrap(); +} diff --git a/tutorial/src/3_min_sound/7_cycle/index.md b/tutorial/src/3_min_sound/7_cycle/index.md new file mode 100644 index 0000000..c7cc82e --- /dev/null +++ b/tutorial/src/3_min_sound/7_cycle/index.md @@ -0,0 +1,72 @@ +# Prevent Cycles + +```admonish warning title="Under construction" +This page is under construction. +``` + +In this section, we will fix the remaining correctness issue with cyclic tasks. + +Didn't we already catch dependency graph cycles in the Incremental Top-Down Context section? +Yes, you remembered right! +However, there is a corner case that we didn't handle. +The issue is that we add a task dependency to the dependency graph only _after the task has finished executing_. +We do this because we need the output from executing the task to create the dependency. + +But what would happen if we made a task that just requires itself? +Let's figure that out in this section, in which we will: + +1) Add cyclic tasks to the testing tasks. +2) Create tests to showcase the cyclic task execution problem. +3) Prevent cycles by _reserving_ a task dependency before executing the task. +4) Improve and add additional tests. + +## Add cyclic testing tasks + +We don't have any testing tasks to easily construct different kinds of cycles yet, so we will add those first. + +Modify `pie/tests/common/mod.rs`: + +```diff2html linebyline +{{#include ../../gen/3_min_sound/7_cycle/a_task.rs.diff}} +``` + +We add the `RequireSelf` task which directly requires itself. +We also add the `RequireA` and `RequireB` tasks which require each other in a cycle. +We want to prevent both of these kinds of cycles. + +## Add cycle tests + +Now add tests that check whether requiring these tasks (correctly) panics due to cycles. + +Modify `pie/tests/top_down.rs`: + +```rust, +{{#include b_test.rs:3:}} +``` + +These test are simple: require the task and that's it. +Which of these tests will correctly result in a cyclic task dependency panic? + +```admonish warning title="Infinite Recursion" +Running these tests will result in infinite recursion, but should quickly cause a stack overflow. +However, be sure to save everything in the event your computer becomes unresponsive. +``` + +```admonish failure title="Expected Test Failure" +Tests `require_self_panics`, `require_cycle_a_panics`, and `require_cycle_b_panics` will fail as expected, which we will fix in this section! +``` + +Run the tests with `cargo test`, or skip running them (and comment them out) if you don't want to waste battery life running infinite recursions. +These tests will infinitely recurse and thus fail. + +The issue is that we only add a dependency to the dependency graph _after the task has executed_. +We do this because we need the output from the executing task to create the dependency. +Therefore, no dependencies are ever added to the dependency graph in these tests, because a task never finishes executing! +This in turn causes the cycle detection to never trigger, because there is no cycle in the dependency graph. + +To fix this, we need to add task dependencies to the dependency graph _before we execute the task_. +But we cannot do this, because we need the output of the task to create the task dependency. +Therefore, we need to first _reserve_ a task dependency in the dependency graph, which creates an edge in the dependency graph without any attached data. + +## Reserving task dependencies + diff --git a/tutorial/src/SUMMARY.md b/tutorial/src/SUMMARY.md index 35fb7fc..3e0045d 100644 --- a/tutorial/src/SUMMARY.md +++ b/tutorial/src/SUMMARY.md @@ -17,7 +17,7 @@ - [Fix Superfluous Task Dependency](./3_min_sound/4_fix_task_dep/index.md) - [Prevent Overlapping File Writes](./3_min_sound/5_overlap/index.md) - [Prevent Hidden Dependencies](./3_min_sound/6_hidden_dep/index.md) - - [Prevent Cycles]() + - [Prevent Cycles](./3_min_sound/7_cycle/index.md) - [Improvements]() - [Serialization]() - [More Stampers]() diff --git a/tutorial/stepper/src/app.rs b/tutorial/stepper/src/app.rs index 0db22bf..73e3596 100644 --- a/tutorial/stepper/src/app.rs +++ b/tutorial/stepper/src/app.rs @@ -378,5 +378,14 @@ pub fn step_all( SourceArchive::new("source.zip") ); }); + + stepper.with_path("7_cycle", |stepper| { + stepper.apply([ + create_diff_from_destination_file("a_task.rs", "../tests/common/mod.rs"), + ]); + stepper.apply_failure([ + add("b_test.rs", "../tests/top_down.rs"), + ]); + }); }); }