Skip to content

Commit

Permalink
fix: Fix internal tasks showing up in queries. (#1592)
Browse files Browse the repository at this point in the history
* Add method.

* Fix tests.
  • Loading branch information
milesj authored Jul 29, 2024
1 parent 3763f2c commit 9b6075f
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Fixed an issue where token expansion would clobber variable replacement when multiple variables
are used.
- Fixed internal tasks being displayed in `moon query` results.

## 1.27.4

Expand Down
1 change: 1 addition & 0 deletions crates/action-graph/src/action_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ impl<'app> ActionGraphBuilder<'app> {
}

for project in projects_to_build {
// Include internal tasks!
for dep_task in project.tasks.values() {
// Don't skip internal tasks, since they are a dependency of the parent
// task, and must still run! They just can't be ran manually.
Expand Down
4 changes: 2 additions & 2 deletions crates/app/src/commands/docker/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub async fn file(session: CliSession, args: DockerFileArgs) -> AppResult {
} else if args.defaults {
project.config.docker.file.build_task.as_ref()
} else {
let mut ids = project.tasks.keys().collect::<Vec<_>>();
let mut ids = project.get_task_ids()?;
ids.sort();

let starting_cursor = project
Expand Down Expand Up @@ -120,7 +120,7 @@ pub async fn file(session: CliSession, args: DockerFileArgs) -> AppResult {
} else if args.defaults {
project.config.docker.file.start_task.as_ref()
} else {
let mut ids = project.tasks.keys().collect::<Vec<_>>();
let mut ids = project.get_task_ids()?;
ids.sort();

let starting_cursor = project
Expand Down
2 changes: 1 addition & 1 deletion crates/app/src/commands/graph/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub async fn action_graph(session: CliSession, args: ActionGraphArgs) -> AppResu
// Show all targets and actions
} else {
for project in project_graph.get_all()? {
for task in project.tasks.values() {
for task in project.get_tasks()? {
action_graph_builder.run_task(&project, task, &requirements)?;
}
}
Expand Down
14 changes: 5 additions & 9 deletions crates/app/src/commands/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,13 @@ pub async fn project(session: CliSession, args: ProjectArgs) -> AppResult {
}
}

if !project.tasks.is_empty() {
console.print_entry_header("Tasks")?;

for name in project.tasks.keys() {
let task = project.tasks.get(name).unwrap();
let tasks = project.get_tasks()?;

if task.is_internal() {
continue;
}
if !tasks.is_empty() {
console.print_entry_header("Tasks")?;

console.print_entry(name, "")?;
for task in tasks {
console.print_entry(&task.id, "")?;

console.write_line(format!(
" {} {}",
Expand Down
8 changes: 4 additions & 4 deletions crates/app/src/commands/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,11 @@ pub async fn tasks(session: CliSession, args: QueryTasksArgs) -> AppResult {

for project in projects {
let filtered_tasks = project
.tasks
.iter()
.filter_map(|(task_id, task)| {
.get_tasks()?
.into_iter()
.filter_map(|task| {
if !args.affected || task.is_affected(&touched_files).is_ok_and(|v| v) {
Some((task_id.to_owned(), task.to_owned()))
Some((task.id.to_owned(), task.to_owned()))
} else {
None
}
Expand Down
5 changes: 4 additions & 1 deletion crates/app/src/queries/projects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ fn load_with_regex(
}

if let Some(regex) = &tasks_regex {
let has_task = project.tasks.keys().any(|task_id| regex.is_match(task_id));
let has_task = project
.get_task_ids()?
.iter()
.any(|task_id| regex.is_match(&task_id));

if !has_task {
continue;
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/tests/query_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ mod tasks {

projects.sort();

assert_eq!(tasks, string_vec!["internal", "lint", "test"]);
assert_eq!(tasks, string_vec!["lint", "test"]);
assert_eq!(projects, string_vec!["metadata", "platforms", "tasks"]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@ platforms
:node | node
:system | noop
tasks
:internal | build
:lint | eslint
:test | jest
1 change: 1 addition & 0 deletions crates/project-expander/src/tasks_expander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl<'graph, 'query> TasksExpander<'graph, 'query> {
dep: &TaskDependencyConfig,
skip_if_missing: bool|
-> miette::Result<()> {
// Allow internal tasks!
let Some(dep_task) = dep_project.tasks.get(&dep.target.task_id) else {
if skip_if_missing {
return Ok(());
Expand Down
17 changes: 15 additions & 2 deletions crates/project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,25 @@ impl Project {
Ok(task)
}

/// Return all tasks within the project.
/// Return a list of all visible task IDs.
pub fn get_task_ids(&self) -> miette::Result<Vec<&Id>> {
Ok(self
.get_tasks()?
.iter()
.map(|task| &task.id)
.collect::<Vec<_>>())
}

/// Return all visible tasks within the project. Does not include internal tasks!
pub fn get_tasks(&self) -> miette::Result<Vec<&Task>> {
let mut tasks = vec![];

for task_id in self.tasks.keys() {
tasks.push(self.get_task(task_id)?);
let task = self.get_task(task_id)?;

if !task.is_internal() {
tasks.push(task);
}
}

Ok(tasks)
Expand Down

0 comments on commit 9b6075f

Please sign in to comment.