diff --git a/crates/no-mistakes/src/codebase/dependencies/graph/files_config_prepared.rs b/crates/no-mistakes/src/codebase/dependencies/graph/files_config_prepared.rs index 83d5ac6c2..6d0c0ecd1 100644 --- a/crates/no-mistakes/src/codebase/dependencies/graph/files_config_prepared.rs +++ b/crates/no-mistakes/src/codebase/dependencies/graph/files_config_prepared.rs @@ -141,7 +141,16 @@ fn prepare_graph_config_inner( // `frontend_apps_or_default` never returns empty, so the pre-#624/#625 // zero-signal fallback (`default_frontend_root`) still applies via the // single synthetic entry it produces. - let playwright_settings = if plan.playwright_routes || plan.playwright_selectors { + // + // Resolving apps is skipped entirely when `has_v2_playwright_settings` is + // false: every per-app `settings_from_loaded_v2` call would collapse to + // the same app-agnostic `settings_from_defaults` fallback regardless of + // which app (if any) is named, so resolving the app set first would only + // discard the result unused — wasted work for every non-Playwright repo + // that still configures `type: nextjs` projects for other reasons. + let playwright_settings = if !(plan.playwright_routes || plan.playwright_selectors) { + Vec::new() + } else if crate::playwright::config::has_v2_playwright_settings(config) { let apps = crate::config::v2::frontend_apps_or_default( root, config, @@ -160,7 +169,14 @@ fn prepare_graph_config_inner( }) .collect::>>()? } else { - Vec::new() + vec![crate::playwright::config::settings_from_loaded_v2( + root, + config, + &[], + None, + None, + visible_paths, + )?] }; Ok(PreparedGraphConfig { options, diff --git a/crates/no-mistakes/src/codebase/dependencies/graph/tests/config_path_full_graph.rs b/crates/no-mistakes/src/codebase/dependencies/graph/tests/config_path_full_graph.rs index acdecda1e..779cdc14f 100644 --- a/crates/no-mistakes/src/codebase/dependencies/graph/tests/config_path_full_graph.rs +++ b/crates/no-mistakes/src/codebase/dependencies/graph/tests/config_path_full_graph.rs @@ -322,6 +322,54 @@ fn prepare_graph_config_surfaces_an_unresolvable_frontend_app() { assert!(message.contains("projects.web.root"), "{message}"); } +/// A `type: nextjs` project whose root can't be inferred must NOT fail (or +/// even attempt) frontend-app resolution when the repository has zero +/// `tests.playwright.*` configuration and no rule binds a Playwright project +/// to an app — `has_v2_playwright_settings` being false means every +/// `settings_from_loaded_v2` call collapses to the app-agnostic +/// `settings_from_defaults` fallback regardless of which app (if any) is +/// named, so resolving the (here, unresolvable) app set first would only +/// discard the result unused. Disagreement test for a review finding: before +/// gating on `has_v2_playwright_settings`, `prepare_graph_config` called +/// `frontend_apps_or_default` unconditionally whenever Playwright edges were +/// requested, so this exact repository — real-world shape: `type: nextjs` +/// projects configured for reasons unrelated to Playwright, no Playwright +/// testing set up at all — failed the whole graph build outright. +#[test] +fn prepare_graph_config_skips_app_resolution_when_playwright_is_unconfigured() { + let root = crate::codebase::ts_resolver::normalize_path(&fixture( + "graph-nextjs-project-without-playwright", + )); + let plan = GraphBuildPlan { + playwright_routes: true, + playwright_selectors: true, + ..GraphBuildPlan::default() + }; + let loaded = crate::config::v2::load_v2_config(&root, None).unwrap(); + assert!( + !crate::playwright::config::has_v2_playwright_settings(&loaded), + "sanity check: this fixture must have zero tests.playwright.* signal" + ); + let codebase_config = crate::codebase::config::config_from_loaded_v2(&root, None, &loaded); + let visible = crate::codebase::ts_source::VisiblePathSnapshot::new(&root); + + let prepared = prepare_graph_config(&root, plan, &codebase_config, &loaded, &visible) + .expect("app resolution must be skipped, not attempted and failed"); + let tsconfig = TsConfig { + dir: root.clone(), + paths: Vec::new(), + paths_dir: root.clone(), + base_url: None, + }; + assert!( + prepared + .playwright_fact_plan(&root, &tsconfig, &visible) + .unwrap() + .is_some(), + "the app-agnostic bare-defaults Settings must still build a fact plan" + ); +} + #[test] fn playwright_route_edges_use_explicit_config_path() { let root = diff --git a/crates/no-mistakes/src/playwright/config.rs b/crates/no-mistakes/src/playwright/config.rs index b8746c09b..9ed1ecb16 100644 --- a/crates/no-mistakes/src/playwright/config.rs +++ b/crates/no-mistakes/src/playwright/config.rs @@ -67,6 +67,16 @@ pub(crate) fn settings_from_loaded_v2( ) } +/// Whether `config` opts into per-app Playwright settings resolution at all +/// (a top-level `tests.playwright.*` field, or any rule's `tests.playwright` +/// list). When this is `false`, every Playwright settings call collapses to +/// the same bare-defaults fallback regardless of which frontend app (if any) +/// a caller names, so resolving that app first is wasted work — callers that +/// fan out over multiple frontend apps should check this before doing so. +pub(crate) fn has_v2_playwright_settings(config: &crate::config::v2::NoMistakesConfig) -> bool { + load::has_v2_playwright_settings(config) +} + pub(crate) fn has_configured_html_id_selector(settings: &Settings) -> bool { use crate::playwright::selectors::HTML_ID_ATTRIBUTE; settings diff --git a/crates/no-mistakes/src/playwright/config/load.rs b/crates/no-mistakes/src/playwright/config/load.rs index 535b36fcf..38aa8ccb4 100644 --- a/crates/no-mistakes/src/playwright/config/load.rs +++ b/crates/no-mistakes/src/playwright/config/load.rs @@ -7,7 +7,8 @@ use std::path::{Path, PathBuf}; #[path = "load/helpers.rs"] pub(super) mod helpers; -use helpers::{default_selector_attributes, has_v2_playwright_settings}; +use helpers::default_selector_attributes; +pub(super) use helpers::has_v2_playwright_settings; #[path = "load/loaded_v2.rs"] mod loaded_v2; diff --git a/crates/no-mistakes/src/playwright/config/load/helpers.rs b/crates/no-mistakes/src/playwright/config/load/helpers.rs index 736f9bff6..ea26462cf 100644 --- a/crates/no-mistakes/src/playwright/config/load/helpers.rs +++ b/crates/no-mistakes/src/playwright/config/load/helpers.rs @@ -28,7 +28,7 @@ pub(super) fn playwright_configs_from_v2( find_default_playwright_configs_from_snapshot(root, visible_paths) } -pub(super) fn has_v2_playwright_settings(config: &NoMistakesConfig) -> bool { +pub(crate) fn has_v2_playwright_settings(config: &NoMistakesConfig) -> bool { is_v2_playwright_configured(&config.tests.playwright) || config .rules diff --git a/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/.no-mistakes.yml b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/.no-mistakes.yml new file mode 100644 index 000000000..6ff0699a5 --- /dev/null +++ b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/.no-mistakes.yml @@ -0,0 +1,3 @@ +projects: + web: + type: nextjs diff --git a/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/a/next.config.ts b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/a/next.config.ts new file mode 100644 index 000000000..ff8b4c563 --- /dev/null +++ b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/a/next.config.ts @@ -0,0 +1 @@ +export default {}; diff --git a/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/b/next.config.ts b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/b/next.config.ts new file mode 100644 index 000000000..ff8b4c563 --- /dev/null +++ b/test-cases/codebase-analysis/graph-nextjs-project-without-playwright/fixture/b/next.config.ts @@ -0,0 +1 @@ +export default {};