Skip to content

Commit

Permalink
Fix bottom-up build tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
Gohla committed Oct 27, 2023
1 parent 14260a9 commit 39a4724
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 46 deletions.
2 changes: 1 addition & 1 deletion dev_ext/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"

[dependencies]
pie = { path = "../pie" }
dev_util = { path = "../dev_util" }

[dev-dependencies]
dev_util = { path = "../dev_util" }
assert_matches = "1"
37 changes: 37 additions & 0 deletions dev_ext/src/downcast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use std::any::Any;

use dev_util::downcast_ref_or_panic;
use pie::trait_object::{KeyObj, ValueObj};

/// Downcast trait objects to specific types for testing purposes.
pub trait Downcast {
fn downcast<T: Any>(&self) -> &T;

fn as_str(&self) -> &'static str {
*self.downcast::<&'static str>()
}
fn as_result_str<E: Any>(&self) -> &Result<&'static str, E> {
self.downcast::<Result<&'static str, E>>()
}

fn as_string(&self) -> &String {
self.downcast::<String>()
}
fn as_result_string<E: Any>(&self) -> &Result<String, E> {
self.downcast::<Result<String, E>>()
}
}

impl Downcast for dyn ValueObj {
fn downcast<T: Any>(&self) -> &T { downcast_ref_or_panic::<T>(self.as_any()) }
}
impl Downcast for Box<dyn ValueObj> {
fn downcast<T: Any>(&self) -> &T { downcast_ref_or_panic::<T>(self.as_ref().as_any()) }
}

impl Downcast for dyn KeyObj {
fn downcast<T: Any>(&self) -> &T { downcast_ref_or_panic::<T>(self.as_any()) }
}
impl Downcast for Box<dyn KeyObj> {
fn downcast<T: Any>(&self) -> &T { downcast_ref_or_panic::<T>(self.as_ref().as_any()) }
}
2 changes: 1 addition & 1 deletion dev_ext/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
pub mod task;

pub mod downcast;
8 changes: 8 additions & 0 deletions dev_util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::any::Any;
use std::fs::{metadata, write};
use std::io;
use std::path::Path;
Expand Down Expand Up @@ -49,3 +50,10 @@ pub fn wait_until_modified_time_changes() -> Result<SystemTime, io::Error> {
write(&file, "123")?;
write_until_modified(&file, "123")
}

/// Downcast `&any` to `&T`, or panic if downcasting fails.
#[inline]
pub fn downcast_ref_or_panic<T: Any>(any: &dyn Any) -> &T {
any.downcast_ref::<T>()
.unwrap_or_else(|| panic!("can't downcast `{:?}` to `{}`", any, std::any::type_name::<T>()))
}
6 changes: 0 additions & 6 deletions pie/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,3 @@ required-features = ["file_hash_checker"]
[[test]]
name = "file_checker"
required-features = ["file_hash_checker"]


# Disable bottom-up context tests until they are fixed
[[test]]
name = "bottom_up"
test = false
10 changes: 7 additions & 3 deletions pie/src/context/bottom_up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ impl<'p, 's> BottomUpContext<'p, 's> {
//let requiring_task = self.session.store.get_task(&requiring_task_node);
//self.session.tracker.check_affected_by_required_task_start(requiring_task, dependency);
//self.session.tracker.check_affected_by_required_task_end(requiring_task, dependency, inconsistent.clone());
if !dependency.is_consistent_with(&output) {
// Note: use `output.as_ref()` instead of `&output`, because `&output` results in a `&Box<dyn ValueObj>` which also
// implements `dyn ValueObj`, but cannot be downcasted to the concrete unboxed type!
if !dependency.is_consistent_with(output.as_ref()) {
// Schedule task; can't extract method due to self borrow above.
//self.session.tracker.schedule_task(requiring_task);
self.scheduled.add(requiring_task_node);
Expand Down Expand Up @@ -153,7 +155,9 @@ impl<'p, 's> BottomUpContext<'p, 's> {
let previous_executing_task = self.session.current_executing_task.replace(node);
let track_end = self.session.tracker.execute(task.as_key_obj());
let output = task.execute_bottom_up(self);
track_end(&mut self.session.tracker, &output);
// Note: use `output.as_ref()` instead of `&output`, because `&output` results in a `&Box<dyn ValueObj>` which also
// implements `dyn ValueObj`, but cannot be downcasted to the concrete unboxed type!
track_end(&mut self.session.tracker, output.as_ref());
self.session.current_executing_task = previous_executing_task;
self.session.store.set_task_output(&node, output.clone());
output
Expand All @@ -168,7 +172,7 @@ impl<'p, 's> BottomUpContext<'p, 's> {
if let Some(min_task_node) = self.scheduled.pop_least_task_with_dependency_from(node, &self.session.store) {
let output = self.execute_and_schedule(min_task_node);
if min_task_node == *node {
let output = output.as_box_any().downcast::<T::Output>()
let output = output.into_box_any().downcast::<T::Output>()
.expect("BUG: non-matching task output type");
return Some(*output);
}
Expand Down
24 changes: 13 additions & 11 deletions pie/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ mod test {

use assert_matches::assert_matches;

use dev_util::downcast_ref_or_panic;

use crate::Context;
use crate::dependency::{ResourceDependency, TaskDependency};
use crate::resource::file::{ExistsChecker, ModifiedChecker};
Expand All @@ -314,33 +316,33 @@ mod test {
}
}

/// Downcast trait objects to references used in these tests.
trait ObjExt {
/// Cast trait objects to types used in tests.
trait Cast {
fn as_path(&self) -> &PathBuf;
fn as_str(&self) -> &'static str;
}
impl ObjExt for dyn KeyObj {
impl Cast for dyn KeyObj {
fn as_path(&self) -> &PathBuf {
self.as_any().downcast_ref().expect("expected `&PathBuf`")
downcast_ref_or_panic(self.as_any())
}
fn as_str(&self) -> &'static str {
self.as_any().downcast_ref::<&'static str>().expect("expected `&'static str`")
downcast_ref_or_panic::<&'static str>(self.as_any())
}
}
impl ObjExt for dyn ValueObj {
impl Cast for dyn ValueObj {
fn as_path(&self) -> &PathBuf {
self.as_any().downcast_ref().expect("expected `&PathBuf`")
downcast_ref_or_panic(self.as_any())
}
fn as_str(&self) -> &'static str {
self.as_any().downcast_ref::<&'static str>().expect("expected `&'static str`")
downcast_ref_or_panic::<&'static str>(self.as_any())
}
}
impl ObjExt for dyn TaskObj {
impl Cast for dyn TaskObj {
fn as_path(&self) -> &PathBuf {
self.as_any().downcast_ref().expect("expected `&PathBuf`")
downcast_ref_or_panic(self.as_any())
}
fn as_str(&self) -> &'static str {
self.as_any().downcast_ref::<&'static str>().expect("expected `&'static str`")
downcast_ref_or_panic::<&'static str>(self.as_any())
}
}

Expand Down
4 changes: 2 additions & 2 deletions pie/src/trait_object/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ use std::hash::{Hash, Hasher};
/// Conversion into [`&dyn Any`](Any). Implies `'static` because [`Any`] requires `'static`.
pub trait AsAny: 'static {
fn as_any(&self) -> &dyn Any;
fn as_box_any(self: Box<Self>) -> Box<dyn Any>;
fn into_box_any(self: Box<Self>) -> Box<dyn Any>;
}
impl<T: Any> AsAny for T {
#[inline]
fn as_any(&self) -> &dyn Any { self as &dyn Any }
#[inline]
fn as_box_any(self: Box<Self>) -> Box<dyn Any> { self as Box<dyn Any> }
fn into_box_any(self: Box<Self>) -> Box<dyn Any> { self as Box<dyn Any> }
}

/// Object safe proxy of [`Eq`], comparing against [`&dyn Any`](Any).
Expand Down
43 changes: 21 additions & 22 deletions pie/tests/bottom_up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::PathBuf;
use assert_matches::assert_matches;
use testresult::TestResult;

use dev_ext::downcast::Downcast;
use dev_ext::task::*;
use dev_util::{create_temp_dir, write_until_modified};
use pie::{Context, Task};
Expand All @@ -15,17 +16,16 @@ use crate::util::{new_test_pie, TestPieExt};

mod util;

/// Downcast trait objects to references.
trait ObjExt {
fn as_str(&self) -> &'static str;
/// Cast trait objects to types used in tests.
trait Cast {
fn cast(&self) -> Result<&str, &FsError>;
}
impl ObjExt for Box<dyn ValueObj> {
fn as_str(&self) -> &'static str {
self.as_ref().as_any().downcast_ref::<&'static str>().expect("expected `&'static str`")
impl Cast for Box<dyn ValueObj> {
fn cast(&self) -> Result<&str, &FsError> {
self.as_result_string::<FsError>().as_deref()
}
}


#[test]
fn test_nothing_affected() {
let mut pie = new_test_pie();
Expand All @@ -52,7 +52,7 @@ fn test_directly_affected_task() -> TestResult {
write_until_modified(&path, "hello world!")?;
pie.bottom_up_build_then_assert(|b| b.changed_resource(&path), |tracker| {
assert_matches!(tracker.first_execute_end(&task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!");
assert_eq!(d.output.cast(), Ok("hello world!"));
});
});

Expand All @@ -73,18 +73,17 @@ fn test_indirectly_affected_tasks() -> TestResult {
let output = pie.require(&to_lowercase_task)?;
assert_eq!(output.as_str(), "hello world!");

// Change the file that ReadFile requires, directly affecting it, indirectly affecting ToLower,
// indirectly affecting CombineA.
// Change the file that ReadFile requires, directly affecting it, indirectly affecting ToLower.
write_until_modified(&path, "HELLO WORLD!!")?;
pie.bottom_up_build_then_assert(|b| b.changed_resource(&path), |tracker| {
// ReadFile
let read_task_end = assert_matches!(tracker.first_execute_end(&read_task), Some(d) => {
assert_eq!(d.output.as_str(), "HELLO WORLD!!");
assert_eq!(d.output.cast(), Ok("HELLO WORLD!!"));
d.index
});
// ToLower
let to_lowercase_task_end = assert_matches!(tracker.first_execute_end(&to_lowercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!!");
assert_eq!(d.output.cast(), Ok("hello world!!"));
d.index
});
assert!(to_lowercase_task_end > read_task_end);
Expand All @@ -108,18 +107,18 @@ fn test_indirectly_affected_tasks_early_cutoff() -> TestResult {
// Initially require the tasks.
pie.require(&write_task)?;

// Change the file that ReadFile requires, directly affecting it, indirectly affecting ToLower, but not
// affecting WriteFile because the output from ToLower does not change.
// Change the file that ReadFile requires, directly affecting it, indirectly affecting ToLower, but not affecting
// WriteFile because the output from ToLower does not change.
write_until_modified(&read_path, "hello world!")?;
pie.bottom_up_build_then_assert(|b| b.changed_resource(&read_path), |tracker| {
// ReadFile
let read_task_end = assert_matches!(tracker.first_execute_end(&read_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!");
assert_eq!(d.output.cast(), Ok("hello world!"));
d.index
});
// ToLower
let to_lowercase_task_end = assert_matches!(tracker.first_execute_end(&to_lowercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!");
assert_eq!(d.output.cast(), Ok("hello world!"));
d.index
});
assert!(to_lowercase_task_end > read_task_end);
Expand Down Expand Up @@ -157,20 +156,20 @@ fn test_indirectly_affected_multiple_tasks() -> TestResult {
pie.bottom_up_build_then_assert(|b| b.changed_resource(&read_path), |tracker| {
// ReadFile
let read_task_end = assert_matches!(tracker.first_execute_end(&read_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!");
assert_eq!(d.output.cast(), Ok("hello world!"));
d.index
});
// ToLower
let to_lowercase_task_end = assert_matches!(tracker.first_execute_end(&to_lowercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!");
assert_eq!(d.output.cast(), Ok("hello world!"));
d.index
});
assert!(to_lowercase_task_end > read_task_end);
// WriteFile(ToLower)
assert!(!tracker.any_execute_of(&write_lowercase_task));
// ToUpper
let to_uppercase_task_end = assert_matches!(tracker.first_execute_end(&to_uppercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "HELLO WORLD!");
assert_eq!(d.output.cast(), Ok("HELLO WORLD!"));
d.index
});
assert!(to_uppercase_task_end > read_task_end);
Expand All @@ -183,12 +182,12 @@ fn test_indirectly_affected_multiple_tasks() -> TestResult {
pie.bottom_up_build_then_assert(|b| b.changed_resource(&read_path), |tracker| {
// ReadFile
let read_task_end = assert_matches!(tracker.first_execute_end(&read_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!!");
assert_eq!(d.output.cast(), Ok("hello world!!"));
d.index
});
// ToLower
let to_lowercase_task_end = assert_matches!(tracker.first_execute_end(&to_lowercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "hello world!!");
assert_eq!(d.output.cast(), Ok("hello world!!"));
d.index
});
assert!(to_lowercase_task_end > read_task_end);
Expand All @@ -197,7 +196,7 @@ fn test_indirectly_affected_multiple_tasks() -> TestResult {
assert!(*write_lowercase_task_end > to_lowercase_task_end);
// ToUpper
let to_uppercase_task_end = assert_matches!(tracker.first_execute_end(&to_uppercase_task), Some(d) => {
assert_eq!(d.output.as_str(), "HELLO WORLD!!");
assert_eq!(d.output.cast(), Ok("HELLO WORLD!!"));
d.index
});
assert!(to_uppercase_task_end > read_task_end);
Expand Down

0 comments on commit 39a4724

Please sign in to comment.