From 7cb4f751a75470e42af12e2a864cdceb478883b2 Mon Sep 17 00:00:00 2001 From: Nicholas Yang Date: Wed, 16 Oct 2024 15:31:39 -0400 Subject: [PATCH] fix(query): remove engine validation for query (#9271) ### Description We don't want to validate certain issues with graphs with `turbo query`, because it should be still usable even with an invalid graph. ### Testing Instructions Added a test that shows that `turbo query` is tolerant of issues that `turbo run` is not. --- crates/turborepo-lib/src/commands/query.rs | 4 +- crates/turborepo-lib/src/engine/builder.rs | 9 +- crates/turborepo-lib/src/run/builder.rs | 14 +- .../invalid-dependency/app-a/package.json | 10 ++ .../invalid-dependency/app-b/package.json | 11 ++ .../invalid-dependency/lib-a/package.json | 10 ++ .../invalid-dependency/lib-b/package.json | 10 ++ .../invalid-dependency/lib-c/package.json | 6 + .../invalid-dependency/lib-d/package.json | 7 + .../invalid-dependency/package.json | 11 ++ .../invalid-dependency/turbo.json | 32 ++++ .../integration/tests/query/validation.t | 148 ++++++++++++++++++ 12 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-a/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-b/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-a/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-b/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-c/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-d/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/package.json create mode 100644 turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/turbo.json create mode 100644 turborepo-tests/integration/tests/query/validation.t diff --git a/crates/turborepo-lib/src/commands/query.rs b/crates/turborepo-lib/src/commands/query.rs index 83cb64fc25ca4..6d367bd562688 100644 --- a/crates/turborepo-lib/src/commands/query.rs +++ b/crates/turborepo-lib/src/commands/query.rs @@ -68,7 +68,9 @@ pub async fn run( execution_args: Box::default(), }); - let run_builder = RunBuilder::new(base)?.add_all_tasks(); + let run_builder = RunBuilder::new(base)? + .add_all_tasks() + .do_not_validate_engine(); let run = run_builder.build(&handler, telemetry).await?; if let Some(query) = query { diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index dd958fcf127c0..acd2dbb4efa3b 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -101,6 +101,7 @@ pub struct EngineBuilder<'a> { root_enabled_tasks: HashSet>, tasks_only: bool, add_all_tasks: bool, + should_validate_engine: bool, } impl<'a> EngineBuilder<'a> { @@ -120,6 +121,7 @@ impl<'a> EngineBuilder<'a> { root_enabled_tasks: HashSet::new(), tasks_only: false, add_all_tasks: false, + should_validate_engine: true, } } @@ -157,6 +159,11 @@ impl<'a> EngineBuilder<'a> { self } + pub fn do_not_validate_engine(mut self) -> Self { + self.should_validate_engine = false; + self + } + // Returns the set of allowed tasks that can be run if --only is used // The set is exactly the product of the packages in filter and tasks specified // by CLI @@ -502,7 +509,7 @@ impl<'a> EngineBuilder<'a> { } } - if task_definitions.is_empty() { + if task_definitions.is_empty() && self.should_validate_engine { let (span, text) = task_id.span_and_text("turbo.json"); return Err(Error::MissingPackageTask { span, diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 7f10b64ab1cc5..dbfaa33f8324f 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -67,6 +67,8 @@ pub struct RunBuilder { should_print_prelude_override: Option, allow_missing_package_manager: bool, allow_no_turbo_json: bool, + // In query, we don't want to validate the engine. Defaults to `true` + should_validate_engine: bool, // If true, we will add all tasks to the graph, even if they are not specified add_all_tasks: bool, } @@ -111,6 +113,7 @@ impl RunBuilder { allow_missing_package_manager, root_turbo_json_path, allow_no_turbo_json, + should_validate_engine: true, add_all_tasks: false, }) } @@ -130,6 +133,11 @@ impl RunBuilder { self } + pub fn do_not_validate_engine(mut self) -> Self { + self.should_validate_engine = false; + self + } + fn connect_process_manager(&self, signal_subscriber: SignalSubscriber) { let manager = self.processes.clone(); tokio::spawn(async move { @@ -496,6 +504,10 @@ impl RunBuilder { builder = builder.add_all_tasks(); } + if !self.should_validate_engine { + builder = builder.do_not_validate_engine(); + } + let mut engine = builder.build()?; // If we have an initial task, we prune out the engine to only @@ -504,7 +516,7 @@ impl RunBuilder { engine = engine.create_engine_for_subgraph(entrypoint_packages); } - if !self.opts.run_opts.parallel { + if !self.opts.run_opts.parallel && self.should_validate_engine { engine .validate( pkg_dep_graph, diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-a/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-a/package.json new file mode 100644 index 0000000000000..1b1065a49f07a --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-a/package.json @@ -0,0 +1,10 @@ +{ + "name": "app-a", + "scripts": { + "build": "echo 'build app-a'", + "test": "echo 'test app-a'" + }, + "dependencies": { + "lib-a": "*" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-b/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-b/package.json new file mode 100644 index 0000000000000..5b28eec0f17f7 --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/app-b/package.json @@ -0,0 +1,11 @@ +{ + "name": "app-b", + "scripts": { + "build": "echo 'build app-b'", + "test": "echo 'test app-b'" + }, + "dependencies": { + "lib-b": "*", + "lib-c": "*" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-a/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-a/package.json new file mode 100644 index 0000000000000..7507e85ce1b0b --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-a/package.json @@ -0,0 +1,10 @@ +{ + "name": "lib-a", + "scripts": { + "build": "echo 'build lib-a'", + "test": "echo 'test lib-a'" + }, + "dependencies": { + "lib-b": "*" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-b/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-b/package.json new file mode 100644 index 0000000000000..5288e96887e92 --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-b/package.json @@ -0,0 +1,10 @@ +{ + "name": "lib-b", + "scripts": { + "build": "echo 'build lib-b'", + "test": "echo 'test lib-b'" + }, + "dependencies": { + "lib-d": "*" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-c/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-c/package.json new file mode 100644 index 0000000000000..d52930425c5d3 --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-c/package.json @@ -0,0 +1,6 @@ +{ + "name": "lib-c", + "scripts": { + "build": "echo 'build lib-c'" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-d/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-d/package.json new file mode 100644 index 0000000000000..94d86aa0d4a5b --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/lib-d/package.json @@ -0,0 +1,7 @@ +{ + "name": "lib-d", + "scripts": { + "build": "echo 'build lib-d'", + "test": "echo 'test lib-d'" + } +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/package.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/package.json new file mode 100644 index 0000000000000..ea0a7089dc497 --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/package.json @@ -0,0 +1,11 @@ +{ + "name": "unknown-dependency", + "workspaces": [ + "app-a", + "app-b", + "lib-a", + "lib-b", + "lib-c", + "lib-d" + ] +} diff --git a/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/turbo.json b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/turbo.json new file mode 100644 index 0000000000000..c2391603086a4 --- /dev/null +++ b/turborepo-tests/integration/fixtures/task_dependencies/invalid-dependency/turbo.json @@ -0,0 +1,32 @@ +{ + "tasks": { + "build0": { + "dependsOn": [ + "^build0", + "prepare" + ] + }, + "test": { + "dependsOn": [ + "^build0", + "prepare" + ] + }, + "prepare": {}, + "side-quest": { + "dependsOn": [ + "prepare" + ] + }, + "build1": { + "dependsOn": [ + "^build1" + ] + }, + "build2": { + "dependsOn": [ + "app-a#custom" + ] + } + } +} diff --git a/turborepo-tests/integration/tests/query/validation.t b/turborepo-tests/integration/tests/query/validation.t new file mode 100644 index 0000000000000..424005304fdaf --- /dev/null +++ b/turborepo-tests/integration/tests/query/validation.t @@ -0,0 +1,148 @@ +Setup + $ . ${TESTDIR}/../../../helpers/setup_integration_test.sh persistent_dependencies/10-too-many + +Validate that we get an error when we try to run multiple persistent tasks with concurrency 1 + $ ${TURBO} run build --concurrency=1 + x invalid task configuration + + Error: x You have 2 persistent tasks but `turbo` is configured for concurrency of + | 1. Set --concurrency to at least 3 + + [1] + +However on query, we ignore this validation + $ ${TURBO} query "query { packages { items { tasks { items { fullName } } } } }" | jq + WARNING query command is experimental and may change in the future + { + "data": { + "packages": { + "items": [ + { + "tasks": { + "items": [] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "one#build" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "two#build" + } + ] + } + } + ] + } + } + } + +Setup + $ . ${TESTDIR}/../../../helpers/setup_integration_test.sh task_dependencies/invalid-dependency + warning: re-init: ignored --initial-branch=main + +Validate that we get an error when trying to depend on a task that doesn't exist + $ ${TURBO} run build2 + x Could not find "app-a#custom" in root turbo.json or "custom" in package + ,-[turbo.json:27:1] + 27 | "dependsOn": [ + 28 | "app-a#custom" + : ^^^^^^^^^^^^^^ + 29 | ] + `---- + + [1] + +However, we don't get an error when we query + $ ${TURBO} query "query { packages { items { tasks { items { fullName } } } } }" | jq + WARNING query command is experimental and may change in the future + { + "data": { + "packages": { + "items": [ + { + "tasks": { + "items": [] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "app-a#build" + }, + { + "fullName": "app-a#test" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "app-b#build" + }, + { + "fullName": "app-b#test" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "lib-a#build" + }, + { + "fullName": "lib-a#test" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "lib-b#build" + }, + { + "fullName": "lib-b#test" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "lib-c#build" + } + ] + } + }, + { + "tasks": { + "items": [ + { + "fullName": "lib-d#build" + }, + { + "fullName": "lib-d#test" + } + ] + } + } + ] + } + } + } \ No newline at end of file