Skip to content

Commit

Permalink
fix: Fix more CI affected dependent issues. (#1625)
Browse files Browse the repository at this point in the history
* Update builder.

* Add tests.

* Bump version.
  • Loading branch information
milesj authored Aug 22, 2024
1 parent 0c1fe42 commit 40d9df9
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 10 deletions.
9 changes: 9 additions & 0 deletions .yarn/versions/4722c693.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
releases:
"@moonrepo/cli": patch
"@moonrepo/core-linux-arm64-gnu": patch
"@moonrepo/core-linux-arm64-musl": patch
"@moonrepo/core-linux-x64-gnu": patch
"@moonrepo/core-linux-x64-musl": patch
"@moonrepo/core-macos-arm64": patch
"@moonrepo/core-macos-x64": patch
"@moonrepo/core-windows-x64-msvc": patch
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

#### 🐞 Fixes

- Fixed an issue around running dependents when a dependency is affected in CI.

## 1.27.9

#### 🐞 Fixes
Expand Down
22 changes: 13 additions & 9 deletions crates/action-graph/src/action_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,19 @@ impl<'app> ActionGraphBuilder<'app> {
color::property("runInCI"),
);

// Dependents may still want to run though!
// Dependents may still want to run though,
// but only if this task was affected
if reqs.dependents {
self.run_task_dependents(task, reqs, true)?;
if let Some(touched) = &reqs.touched_files {
debug!(
task = task.target.as_str(),
"But will run all dependent tasks if affected"
);

if task.is_affected(touched)? {
self.run_task_dependents(task, reqs)?;
}
}
}

return Ok(None);
Expand Down Expand Up @@ -300,7 +310,7 @@ impl<'app> ActionGraphBuilder<'app> {

// And possibly dependents
if reqs.dependents {
self.run_task_dependents(task, reqs, false)?;
self.run_task_dependents(task, reqs)?;
}

Ok(Some(index))
Expand Down Expand Up @@ -348,7 +358,6 @@ impl<'app> ActionGraphBuilder<'app> {
&mut self,
task: &Task,
parent_reqs: &RunRequirements<'app>,
with_touched: bool,
) -> miette::Result<Vec<NodeIndex>> {
let mut indices = vec![];

Expand All @@ -357,11 +366,6 @@ impl<'app> ActionGraphBuilder<'app> {
let reqs = RunRequirements {
ci: parent_reqs.ci,
interactive: parent_reqs.interactive,
touched_files: if with_touched {
parent_reqs.touched_files
} else {
None
},
..Default::default()
};

Expand Down
8 changes: 8 additions & 0 deletions crates/action-graph/tests/__fixtures__/tasks/ci/moon.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
tasks:
ci1-dependency:
inputs:
- 'input.txt'
options:
runInCI: false
ci1-dependant:
Expand All @@ -9,6 +11,8 @@ tasks:
runInCI: true

ci2-dependency:
inputs:
- 'input.txt'
options:
runInCI: false
ci2-dependant:
Expand All @@ -18,6 +22,8 @@ tasks:
runInCI: false

ci3-dependency:
inputs:
- 'input.txt'
options:
runInCI: true
ci3-dependant:
Expand All @@ -27,6 +33,8 @@ tasks:
runInCI: false

ci4-dependency:
inputs:
- 'input.txt'
options:
runInCI: true
ci4-dependant:
Expand Down
35 changes: 34 additions & 1 deletion crates/action-graph/tests/action_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,39 @@ mod action_graph {
}

#[tokio::test]
async fn runs_dependents_if_dependency_is_ci_false() {
async fn runs_dependents_if_dependency_is_ci_false_but_affected() {
let sandbox = create_sandbox("tasks");
let container = ActionGraphContainer::new(sandbox.path()).await;
let mut builder = container.create_builder();

let project = container.project_graph.get("ci").unwrap();
let task = project.get_task("ci1-dependency").unwrap();

// Must be affected to run the dependent
let touched_files =
FxHashSet::from_iter([WorkspaceRelativePathBuf::from("ci/input.txt")]);

builder
.run_task(
&project,
task,
&RunRequirements {
ci: true,
ci_check: true,
dependents: true,
touched_files: Some(&touched_files),
..RunRequirements::default()
},
)
.unwrap();

let graph = builder.build();

assert_snapshot!(graph.to_dot());
}

#[tokio::test]
async fn doesnt_run_dependents_if_dependency_is_ci_false_and_not_affected() {
let sandbox = create_sandbox("tasks");
let container = ActionGraphContainer::new(sandbox.path()).await;
let mut builder = container.create_builder();
Expand All @@ -1145,6 +1177,7 @@ mod action_graph {
ci: true,
ci_check: true,
dependents: true,
touched_files: None,
..RunRequirements::default()
},
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/action-graph/tests/action_graph_test.rs
expression: graph.to_dot()
---
digraph {
}

0 comments on commit 40d9df9

Please sign in to comment.