Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 1 addition & 24 deletions core/exec/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,17 +434,7 @@ fn execute_flat<T: Executable>(

// Acquire any required tools (capability-based pattern)
// This runs the upsert (check/install) and adds ToolHandle to inputs.
// Skip acquisition when a node override is present — the node's outputs
// will be replaced wholesale, so running real tool checks/installs is
// wasted I/O that can fail in CI or mutate the environment.
let has_node_override = match mode {
ExecutionMode::DryRun(ref m) => m.get_node_override(&node_id.0).is_some(),
ExecutionMode::Simulate(ref config) => {
config.boundary_mocks.get_node_override(&node_id.0).is_some()
}
_ => false,
};
if node.has_tool_requirements() && !has_node_override {
if node.has_tool_requirements() {
acquire_node_tools(node, &mut acquired_tools, &mut inputs)?;
}

Expand All @@ -460,18 +450,6 @@ fn execute_flat<T: Executable>(
.collect();
(outputs, false)
} else {
// Check for explicit node override first (for non-transport I/O nodes).
// Node overrides force-mock a node regardless of its port types,
// used by flow tests to mock CLI tool operations etc.
let node_override = match mode {
ExecutionMode::DryRun(ref m) => m.get_node_override(&node_id.0),
ExecutionMode::Simulate(ref config) => config.boundary_mocks.get_node_override(&node_id.0),
_ => None,
};

if let Some(override_outputs) = node_override {
(override_outputs.clone(), true)
} else {
// Check if this is a transport execution node (consumes TransportRequest)
// Transport execution nodes are intercepted in dry-run/simulate mode
// This follows the design principle: intercept where I/O happens, not boundaries
Expand Down Expand Up @@ -524,7 +502,6 @@ fn execute_flat<T: Executable>(
}
}
}
}
};

// Mask any secret values in CI context so that CI runners
Expand Down
21 changes: 0 additions & 21 deletions core/exec/src/intercept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ pub struct BoundaryMocks {
mocks: HashMap<(String, String), BoundaryMock>,
/// Default mock to use when no specific mock is defined
default_mock: BoundaryMock,
/// Override all outputs for specific nodes (regardless of transport executor status).
/// Used by flow tests to mock non-transport I/O nodes (e.g., CLI tool ops).
node_overrides: HashMap<String, HashMap<String, Value>>,
}

impl BoundaryMocks {
Expand Down Expand Up @@ -93,24 +90,6 @@ impl BoundaryMocks {
mocks.set_default_value(value);
mocks
}

/// Override all outputs for a specific node.
///
/// Unlike transport executor interception (which is structural), node overrides
/// force-mock any node regardless of its port types. This is used by flow tests
/// to mock non-transport I/O nodes like CLI tool operations.
pub fn set_node_override(
&mut self,
node_id: impl Into<String>,
outputs: HashMap<String, Value>,
) {
self.node_overrides.insert(node_id.into(), outputs);
}

/// Get the override outputs for a node, if any.
pub fn get_node_override(&self, node_id: &str) -> Option<&HashMap<String, Value>> {
self.node_overrides.get(node_id)
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion core/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub use composition::{assert_types_compatible, TypeCompatibility};
pub use mock::{MockBehavior, MockOp, ScriptedDagBuilder};
pub use mock_spec::{
validate_chain, BoundaryMock, ChainError, ChainValidationResult, ExpectedOutput,
InputConstraint, InputExpectation, MockSpec, NodeOverride, ResourceAcquireResult,
InputConstraint, InputExpectation, MockSpec, ResourceAcquireResult,
ResourceBehavior, ResourceMocks, ResourceSimulation, ResourceType, TransportMock,
};
pub use mockable::{
Expand Down
38 changes: 1 addition & 37 deletions core/test/src/mock_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ pub struct MockSpec {
/// Expected outputs at terminal/boundary nodes (for flow test assertions).
/// After DryRun execution, these are verified against actual outputs.
pub expected_outputs: Vec<ExpectedOutput>,

/// Override outputs for non-transport I/O nodes (e.g., CLI tool ops).
/// These force-mock nodes that aren't transport executors but still do I/O.
pub node_overrides: Vec<NodeOverride>,
}

impl MockSpec {
Expand All @@ -70,7 +66,6 @@ impl MockSpec {
resource_mocks: ResourceMocks::new(),
transport_mocks: Vec::new(),
expected_outputs: Vec::new(),
node_overrides: Vec::new(),
}
}

Expand Down Expand Up @@ -156,36 +151,14 @@ impl MockSpec {
self
}

/// Add a node override (force-mock a non-transport I/O node).
pub fn node_override(
mut self,
node: impl Into<String>,
outputs: Vec<(impl Into<String>, Value)>,
) -> Self {
self.node_overrides.push(NodeOverride {
node: node.into(),
outputs: outputs.into_iter().map(|(k, v)| (k.into(), v)).collect(),
});
self
}

/// Convert this MockSpec into BoundaryMocks suitable for `execute_with_mode`.
///
/// Maps transport_mocks to port-level mocks and node_overrides to
/// full-node overrides in the resulting BoundaryMocks.
/// Maps transport_mocks to port-level mocks in the resulting BoundaryMocks.
pub fn to_boundary_mocks(&self) -> BoundaryMocks {
let mut mocks = BoundaryMocks::new();
for tm in &self.transport_mocks {
mocks.set_value(&tm.node, &tm.port, tm.value.clone());
}
for no in &self.node_overrides {
let outputs: HashMap<String, Value> = no
.outputs
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
mocks.set_node_override(&no.node, outputs);
}
mocks
}

Expand Down Expand Up @@ -254,15 +227,6 @@ pub struct ExpectedOutput {
pub expected: Value,
}

/// An override for a non-transport I/O node (force-mocked in DryRun).
#[derive(Debug, Clone)]
pub struct NodeOverride {
/// Node ID to override (e.g., "clippy_lint")
pub node: String,
/// Output port → value pairs
pub outputs: Vec<(String, Value)>,
}

/// An expectation about input from upstream.
#[derive(Debug, Clone)]
pub struct InputExpectation {
Expand Down
12 changes: 5 additions & 7 deletions gunbc-dag/src/ci/graph_mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,11 @@ pub fn ci_mock_spec() -> MockSpec {
)
.transport_mock("execute_test", "skip", Value::Bool(false))
.transport_mock("execute_test", "skip_reason", Value::Str(String::new()))
// -- Lint: clippy_lint is a CliToolOp (not transport), needs node override
.node_override("clippy_lint", vec![
("success", Value::Bool(true)),
("stdout", Value::Str(String::new())),
("stderr", Value::Str(String::new())),
("skip", Value::Bool(false)),
])
// -- Lint: clippy succeeds
.transport_mock("clippy_lint", "success", Value::Bool(true))
.transport_mock("clippy_lint", "stdout", Value::Str(String::new()))
.transport_mock("clippy_lint", "stderr", Value::Str(String::new()))
.transport_mock("clippy_lint", "skip", Value::Bool(false))
Comment on lines +92 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid transport mocks for non-transport clippy node

These transport_mock entries won’t be used because DryRun interception only applies to transport executor nodes (inputs include TransportRequest; see is_transport_execution_node in core/exec/src/execute.rs), while clippy_lint is a CliToolOp with only a skip input. As a result, execute_with_mode(DryRun) will still acquire/run clippy, so the flow tests (and any DryRun runs) will perform real tool I/O or fail if clippy isn’t installed. With node overrides removed, this mock needs to move to a transport node or another mocking mechanism for CLI ops.

Useful? React with 👍 / 👎.

// Expected outputs: verified after DryRun execution
.expected_output("report", "overall_success", Value::Bool(true))
}
Expand Down