From b07f1858f268086388f10825d1dc036a9088ab1a Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 28 Oct 2024 12:36:21 -0400 Subject: [PATCH 1/5] chore(engine): add failing unit test --- crates/turborepo-lib/src/engine/builder.rs | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index acd2dbb4efa3b..6623ae2a02607 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -1245,4 +1245,55 @@ mod test { .err(); assert_eq!(result.as_deref(), reason); } + + #[test] + fn test_run_package_task_exact() { + let repo_root_dir = TempDir::with_prefix("repo").unwrap(); + let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap(); + let package_graph = mock_package_graph( + &repo_root, + package_jsons! { + repo_root, + "app1" => ["libA"], + "app2" => ["libA"], + "libA" => [] + }, + ); + let turbo_jsons = vec![ + ( + PackageName::Root, + turbo_json(json!({ + "tasks": { + "build": { "dependsOn": ["^build"] }, + "special": { "dependsOn": ["^build"] }, + } + })), + ), + ( + PackageName::from("app2"), + turbo_json(json!({ + "tasks": { + "special": { "dependsOn": ["^build"] }, + } + })), + ), + ] + .into_iter() + .collect(); + let loader = TurboJsonLoader::noop(turbo_jsons); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) + .with_tasks(vec![ + Spanned::new(TaskName::from("app1#special")), + Spanned::new(TaskName::from("app2#special")), + ]) + .with_workspaces(vec![PackageName::from("app1"), PackageName::from("app2")]) + .build() + .unwrap(); + + let expected = deps! { + "app1#special" => ["libA#build"], + "libA#build" => ["___ROOT___"] + }; + assert_eq!(all_dependencies(&engine), expected); + } } From 0845a2e12444f3eef5620eccb47414ef25fe2674 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 28 Oct 2024 17:11:26 -0400 Subject: [PATCH 2/5] feat(engine): allow for users to specify tasks via task id --- crates/turborepo-lib/src/engine/builder.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 6623ae2a02607..caadb1c13da6b 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -433,8 +433,16 @@ impl<'a> EngineBuilder<'a> { }; let task_id_as_name = task_id.as_task_name(); - if turbo_json.tasks.contains_key(&task_id_as_name) + if + // See if pkg#task is defined e.g. `docs#build`. This can only happen in root turbo.json + turbo_json.tasks.contains_key(&task_id_as_name) + // See if task is defined e.g. `build`. This can happen in root or workspace turbo.json + // This will fail if the user provided a task id e.g. turbo `docs#build` || turbo_json.tasks.contains_key(task_name) + // If user provided a task id, then we see if the task is defined + // e.g. `docs#build` should resolve if there's a `build` in root turbo.json or docs workspace level turbo.json + || (matches!(workspace, PackageName::Root) && turbo_json.tasks.contains_key(&TaskName::from(task_name.task()))) + || (workspace == &PackageName::from(task_id.package()) && turbo_json.tasks.contains_key(&TaskName::from(task_name.task()))) { Ok(true) } else if !matches!(workspace, PackageName::Root) { @@ -680,6 +688,10 @@ mod test { #[test_case(PackageName::from("b"), "build", "b#build", true ; "workspace task in workspace")] #[test_case(PackageName::from("b"), "test", "b#test", true ; "task missing from workspace")] #[test_case(PackageName::from("c"), "missing", "c#missing", false ; "task missing")] + #[test_case(PackageName::from("c"), "c#curse", "c#curse", true ; "root defined task")] + #[test_case(PackageName::from("b"), "c#curse", "c#curse", true ; "non-workspace root defined task")] + #[test_case(PackageName::from("b"), "b#special", "b#special", true ; "workspace defined task")] + #[test_case(PackageName::from("c"), "b#special", "b#special", false ; "non-workspace defined task")] fn test_task_definition( workspace: PackageName, task_name: &'static str, @@ -694,6 +706,7 @@ mod test { "test": { "inputs": ["testing"] }, "build": { "inputs": ["primary"] }, "a#build": { "inputs": ["special"] }, + "c#curse": {}, } })), ), @@ -702,6 +715,7 @@ mod test { turbo_json(json!({ "tasks": { "build": { "inputs": ["outer"]}, + "special": {}, } })), ), @@ -1272,8 +1286,9 @@ mod test { ( PackageName::from("app2"), turbo_json(json!({ + "extends": ["//"], "tasks": { - "special": { "dependsOn": ["^build"] }, + "another": { "dependsOn": ["^build"] }, } })), ), @@ -1284,7 +1299,7 @@ mod test { let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_tasks(vec![ Spanned::new(TaskName::from("app1#special")), - Spanned::new(TaskName::from("app2#special")), + Spanned::new(TaskName::from("app2#another")), ]) .with_workspaces(vec![PackageName::from("app1"), PackageName::from("app2")]) .build() @@ -1292,6 +1307,7 @@ mod test { let expected = deps! { "app1#special" => ["libA#build"], + "app2#another" => ["libA#build"], "libA#build" => ["___ROOT___"] }; assert_eq!(all_dependencies(&engine), expected); From 616bdb4d8227f1d907dc2900d2ee65b2aa9ec44e Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 29 Oct 2024 13:54:43 -0400 Subject: [PATCH 3/5] chore(engine): add tests for errors regarding missing task def --- Cargo.lock | 1 + crates/turborepo-lib/Cargo.toml | 1 + crates/turborepo-lib/src/engine/builder.rs | 58 ++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ae5ea50a5fe37..2c946a7e5bca2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6361,6 +6361,7 @@ dependencies = [ "human_format", "humantime", "ignore", + "insta", "itertools 0.10.5", "jsonc-parser 0.21.0", "lazy_static", diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index 56816b40a639c..b7a6b4234034c 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -19,6 +19,7 @@ daemon-file-hashing = [] anyhow = { workspace = true, features = ["backtrace"] } assert_cmd = { workspace = true } async-stream = "0.3.4" +insta = { workspace = true } itertools = { workspace = true } port_scanner = { workspace = true } pretty_assertions = { workspace = true } diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index caadb1c13da6b..ff636fdba8204 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -561,6 +561,7 @@ fn validate_task_name(task: Spanned<&str>) -> Result<(), Error> { mod test { use std::assert_matches::assert_matches; + use insta::assert_snapshot; use pretty_assertions::assert_eq; use serde_json::json; use tempfile::TempDir; @@ -1312,4 +1313,61 @@ mod test { }; assert_eq!(all_dependencies(&engine), expected); } + + #[test] + fn test_run_package_task_exact_error() { + let repo_root_dir = TempDir::with_prefix("repo").unwrap(); + let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap(); + let package_graph = mock_package_graph( + &repo_root, + package_jsons! { + repo_root, + "app1" => ["libA"], + "libA" => [] + }, + ); + let turbo_jsons = vec![ + ( + PackageName::Root, + turbo_json(json!({ + "tasks": { + "build": { "dependsOn": ["^build"] }, + } + })), + ), + ( + PackageName::from("app1"), + turbo_json(json!({ + "extends": ["//"], + "tasks": { + "another": { "dependsOn": ["^build"] }, + } + })), + ), + ] + .into_iter() + .collect(); + let loader = TurboJsonLoader::noop(turbo_jsons); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader.clone(), false) + .with_tasks(vec![Spanned::new(TaskName::from("app1#special"))]) + .with_workspaces(vec![PackageName::from("app1")]) + .build(); + assert!(engine.is_err()); + assert_snapshot!(format!("{:?}", miette::Report::new(engine.unwrap_err())), @r###" + × missing tasks in project + + Error: × could not find task `app1#special` in project + "###); + + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) + .with_tasks(vec![Spanned::new(TaskName::from("app1#another"))]) + .with_workspaces(vec![PackageName::from("libA")]) + .build(); + assert!(engine.is_err()); + assert_snapshot!(format!("{:?}", miette::Report::new(engine.unwrap_err())), @r###" + × missing tasks in project + + Error: × could not find task `app1#another` in project + "###); + } } From 406f1ff7a843e21df9cad8ae927feeb094be9f45 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 29 Oct 2024 14:00:20 -0400 Subject: [PATCH 4/5] chore(engine): add integration test --- .../tests/workspace-configs/cross-workspace.t | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/turborepo-tests/integration/tests/workspace-configs/cross-workspace.t b/turborepo-tests/integration/tests/workspace-configs/cross-workspace.t index 17af56d6dabe9..dbe0c150f016b 100644 --- a/turborepo-tests/integration/tests/workspace-configs/cross-workspace.t +++ b/turborepo-tests/integration/tests/workspace-configs/cross-workspace.t @@ -21,3 +21,24 @@ Setup Cached: 0 cached, 2 total Time:\s*[\.0-9]+m?s (re) + $ ${TURBO} run cross-workspace#cross-workspace-task + \xe2\x80\xa2 Packages in scope: add-keys, add-tasks, bad-json, blank-pkg, cached, config-change, cross-workspace, invalid-config, missing-workspace-config, omit-keys, override-values, persistent (esc) + \xe2\x80\xa2 Running cross-workspace#cross-workspace-task in 12 packages (esc) + \xe2\x80\xa2 Remote caching disabled (esc) + blank-pkg:cross-workspace-underlying-task: cache hit, replaying logs 39566f6362976823 + blank-pkg:cross-workspace-underlying-task: + blank-pkg:cross-workspace-underlying-task: > cross-workspace-underlying-task + blank-pkg:cross-workspace-underlying-task: > echo cross-workspace-underlying-task from blank-pkg + blank-pkg:cross-workspace-underlying-task: + blank-pkg:cross-workspace-underlying-task: cross-workspace-underlying-task from blank-pkg + cross-workspace:cross-workspace-task: cache hit, replaying logs bce507a110930f07 + cross-workspace:cross-workspace-task: + cross-workspace:cross-workspace-task: > cross-workspace-task + cross-workspace:cross-workspace-task: > echo cross-workspace-task + cross-workspace:cross-workspace-task: + cross-workspace:cross-workspace-task: cross-workspace-task + + Tasks: 2 successful, 2 total + Cached: 2 cached, 2 total + Time:\s*[\.0-9]+m?s >>> FULL TURBO (re) + From 6f0e43181d81a4ad39f4517d8d1b0ac4e0fe0bca Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 29 Oct 2024 15:09:41 -0400 Subject: [PATCH 5/5] chore(engine): avoid hitting tty based error output --- crates/turborepo-lib/src/engine/builder.rs | 24 ++++++++++--------- ..._test__run_package_task_exact_error-2.snap | 5 ++++ ...r__test__run_package_task_exact_error.snap | 5 ++++ 3 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error-2.snap create mode 100644 crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error.snap diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index ff636fdba8204..7dfe7b987c16c 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -561,7 +561,7 @@ fn validate_task_name(task: Spanned<&str>) -> Result<(), Error> { mod test { use std::assert_matches::assert_matches; - use insta::assert_snapshot; + use insta::assert_json_snapshot; use pretty_assertions::assert_eq; use serde_json::json; use tempfile::TempDir; @@ -1353,21 +1353,23 @@ mod test { .with_workspaces(vec![PackageName::from("app1")]) .build(); assert!(engine.is_err()); - assert_snapshot!(format!("{:?}", miette::Report::new(engine.unwrap_err())), @r###" - × missing tasks in project - - Error: × could not find task `app1#special` in project - "###); + let report = miette::Report::new(engine.unwrap_err()); + let mut msg = String::new(); + miette::JSONReportHandler::new() + .render_report(&mut msg, report.as_ref()) + .unwrap(); + assert_json_snapshot!(msg); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_tasks(vec![Spanned::new(TaskName::from("app1#another"))]) .with_workspaces(vec![PackageName::from("libA")]) .build(); assert!(engine.is_err()); - assert_snapshot!(format!("{:?}", miette::Report::new(engine.unwrap_err())), @r###" - × missing tasks in project - - Error: × could not find task `app1#another` in project - "###); + let report = miette::Report::new(engine.unwrap_err()); + let mut msg = String::new(); + miette::JSONReportHandler::new() + .render_report(&mut msg, report.as_ref()) + .unwrap(); + assert_json_snapshot!(msg); } } diff --git a/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error-2.snap b/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error-2.snap new file mode 100644 index 0000000000000..23933b974c392 --- /dev/null +++ b/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error-2.snap @@ -0,0 +1,5 @@ +--- +source: crates/turborepo-lib/src/engine/builder.rs +expression: msg +--- +"{\"message\": \"missing tasks in project\",\"severity\": \"error\",\"causes\": [],\"labels\": [],\"related\": [{\"message\": \"could not find task `app1#another` in project\",\"severity\": \"error\",\"causes\": [],\"filename\": \"\",\"labels\": [],\"related\": []}]}" diff --git a/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error.snap b/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error.snap new file mode 100644 index 0000000000000..24e22689cdfb9 --- /dev/null +++ b/crates/turborepo-lib/src/engine/snapshots/turborepo_lib__engine__builder__test__run_package_task_exact_error.snap @@ -0,0 +1,5 @@ +--- +source: crates/turborepo-lib/src/engine/builder.rs +expression: msg +--- +"{\"message\": \"missing tasks in project\",\"severity\": \"error\",\"causes\": [],\"labels\": [],\"related\": [{\"message\": \"could not find task `app1#special` in project\",\"severity\": \"error\",\"causes\": [],\"filename\": \"\",\"labels\": [],\"related\": []}]}"