Snapshot issues in next-dev-tests (#3774)

This:
* Adds issue snapshot support to next-dev-tests. This will allow us to
assert that certain issues are raised in tests that require next-dev.
* Extracts common snapshot code into a new crates,
`turbopack-test-utils`, which is shared between snapshot tests and
next-dev-tests.
* Implements an issue reporter that emits issues in a channel to the the
integration test code, where they are snapshotted.
* Fixes an issue where next-dev tests that were not Next.js apps would
emit non-fatal issues for a missing `pages/` directory.

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

crates/next-core/src/next_config.rs

@@ -8,7 +8,7 @@ use turbo_tasks::{
     CompletionVc, Value,
 };
 use turbo_tasks_env::EnvMapVc;
-use turbo_tasks_fs::json::parse_json_rope_with_source_context;
+use turbo_tasks_fs::{json::parse_json_rope_with_source_context, FileSystemPathVc};
 use turbopack::evaluate_context::node_evaluate_asset_context;
 use turbopack_core::{
     asset::Asset,
@@ -603,3 +603,11 @@ pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<N
         }
     }
 }
+
+#[turbo_tasks::function]
+pub async fn has_next_config(context: FileSystemPathVc) -> Result<BoolVc> {
+    Ok(BoolVc::cell(!matches!(
+        *find_context_file(context, next_configs()).await?,
+        FindContextFileResult::NotFound(_)
+    )))
+}

crates/next-core/src/router_source.rs

@@ -12,7 +12,7 @@ use turbopack_dev_server::source::{
 use turbopack_node::execution_context::ExecutionContextVc;
 
 use crate::{
-    next_config::NextConfigVc,
+    next_config::{has_next_config, NextConfigVc},
     router::{route, RouterRequest, RouterResult},
 };
 
@@ -74,6 +74,16 @@ impl ContentSource for NextRouterContentSource {
     ) -> Result<ContentSourceResultVc> {
         let this = self_vc.await?;
 
+        // The next-dev server can currently run against projects as simple as
+        // `index.js`. If this isn't a Next.js project, don't try to use the Next.js
+        // router.
+        let project_root = this.execution_context.await?.project_root;
+        if !(*has_next_config(project_root).await?) {
+            return Ok(this
+                .inner
+                .get(path, Value::new(ContentSourceData::default())));
+        }
+
         let ContentSourceData {
             method: Some(method),
             raw_headers: Some(raw_headers),

crates/next-dev-tests/Cargo.toml

@@ -30,7 +30,6 @@ lazy_static = "1.4.0"
 mime = "0.3.16"
 next-core = { path = "../next-core" }
 next-dev = { path = "../next-dev" }
-once_cell = "1.13.0"
 owo-colors = "3"
 parking_lot = "0.12.1"
 rand = "0.8.5"
@@ -53,6 +52,7 @@ turbopack-cli-utils = { path = "../turbopack-cli-utils" }
 turbopack-core = { path = "../turbopack-core" }
 turbopack-dev-server = { path = "../turbopack-dev-server" }
 turbopack-node = { path = "../turbopack-node" }
+turbopack-test-utils = { path = "../turbopack-test-utils" }
 url = "2.2.2"
 webbrowser = "0.7.1"
 

crates/next-dev-tests/build.rs

@@ -1,6 +1,7 @@
-use turbo_tasks_build::rerun_if_glob;
+use turbo_tasks_build::{generate_register, rerun_if_glob};
 
 fn main() {
+    generate_register();
     // The test/integration crate need to be rebuilt if any test input is changed.
     // Unfortunately, we can't have the build.rs file operate differently on
     // each file, so the entire next-dev crate needs to be rebuilt.

crates/next-dev-tests/tests/integration.rs

@@ -1,3 +1,4 @@
+#![feature(min_specialization)]
 #![cfg(test)]
 extern crate test_generator;
 
@@ -25,16 +26,31 @@ use chromiumoxide::{
 };
 use futures::StreamExt;
 use lazy_static::lazy_static;
-use next_dev::{register, EntryRequest, NextDevServerBuilder};
+use next_dev::{EntryRequest, NextDevServerBuilder};
 use owo_colors::OwoColorize;
 use serde::Deserialize;
 use test_generator::test_resources;
-use tokio::{net::TcpSocket, task::JoinSet};
+use tokio::{
+    net::TcpSocket,
+    sync::mpsc::{channel, Sender},
+    task::JoinSet,
+};
 use tungstenite::{error::ProtocolError::ResetWithoutClosingHandshake, Error::Protocol};
-use turbo_tasks::TurboTasks;
-use turbo_tasks_fs::util::sys_to_unix;
+use turbo_tasks::{
+    debug::{ValueDebug, ValueDebugStringReadRef},
+    primitives::BoolVc,
+    NothingVc, RawVc, ReadRef, State, TransientInstance, TransientValue, TurboTasks,
+};
+use turbo_tasks_fs::{DiskFileSystemVc, FileSystem};
 use turbo_tasks_memory::MemoryBackend;
 use turbo_tasks_testing::retry::retry_async;
+use turbopack_core::issue::{CapturedIssues, IssueReporter, IssueReporterVc, PlainIssueReadRef};
+use turbopack_test_utils::snapshot::snapshot_issues;
+
+fn register() {
+    next_dev::register();
+    include!(concat!(env!("OUT_DIR"), "/register_test_integration.rs"));
+}
 
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -155,18 +171,27 @@ async fn run_test(resource: &str) -> JestRunResult {
     );
 
     let package_root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    let workspace_root = package_root.parent().unwrap().parent().unwrap();
-    let project_dir = workspace_root.join(resource).join("input");
+    let workspace_root = package_root
+        .parent()
+        .unwrap()
+        .parent()
+        .unwrap()
+        .to_path_buf();
+    let test_dir = workspace_root.join(resource);
+    let project_dir = test_dir.join("input");
     let requested_addr = get_free_local_addr().unwrap();
 
     let mock_dir = path.join("__httpmock__");
     let mock_server_future = get_mock_server_future(&mock_dir);
 
-    let turbo_tasks = TurboTasks::new(MemoryBackend::default());
+    let (issue_tx, mut issue_rx) = channel(u16::MAX as usize);
+    let issue_tx = TransientInstance::new(issue_tx);
+
+    let tt = TurboTasks::new(MemoryBackend::default());
     let server = NextDevServerBuilder::new(
-        turbo_tasks,
-        sys_to_unix(&project_dir.to_string_lossy()).to_string(),
-        sys_to_unix(&workspace_root.to_string_lossy()).to_string(),
+        tt.clone(),
+        project_dir.to_string_lossy().to_string(),
+        workspace_root.to_string_lossy().to_string(),
     )
     .entry_request(EntryRequest::Module(
         "@turbo/pack-test-harness".to_string(),
@@ -178,6 +203,9 @@ async fn run_test(resource: &str) -> JestRunResult {
     .port(requested_addr.port())
     .log_level(turbopack_core::issue::IssueSeverity::Warning)
     .log_detail(true)
+    .issue_reporter(Box::new(move || {
+        TestIssueReporterVc::new(issue_tx.clone()).into()
+    }))
     .show_all(true)
     .build()
     .await
@@ -198,6 +226,29 @@ async fn run_test(resource: &str) -> JestRunResult {
 
     env::remove_var("TURBOPACK_TEST_ONLY_MOCK_SERVER");
 
+    let task = tt.spawn_once_task(async move {
+        let issues_fs = DiskFileSystemVc::new(
+            "issues".to_string(),
+            test_dir.join("issues").to_string_lossy().to_string(),
+        )
+        .as_file_system();
+
+        let mut issues = vec![];
+        while let Ok(issue) = issue_rx.try_recv() {
+            issues.push(issue);
+        }
+
+        snapshot_issues(
+            issues.iter().cloned(),
+            issues_fs.root(),
+            &workspace_root.to_string_lossy(),
+        )
+        .await?;
+
+        Ok(NothingVc::new().into())
+    });
+    tt.wait_task_completion(task, true).await.unwrap();
+
     result
 }
 
@@ -414,3 +465,40 @@ async fn get_mock_server_future(mock_dir: &Path) -> Result<(), String> {
         std::future::pending::<Result<(), String>>().await
     }
 }
+
+#[turbo_tasks::value(shared)]
+struct TestIssueReporter {
+    #[turbo_tasks(trace_ignore, debug_ignore)]
+    pub issue_tx: State<Sender<(PlainIssueReadRef, ValueDebugStringReadRef)>>,
+}
+
+#[turbo_tasks::value_impl]
+impl TestIssueReporterVc {
+    #[turbo_tasks::function]
+    fn new(
+        issue_tx: TransientInstance<Sender<(PlainIssueReadRef, ValueDebugStringReadRef)>>,
+    ) -> Self {
+        TestIssueReporter {
+            issue_tx: State::new((*issue_tx).clone()),
+        }
+        .cell()
+    }
+}
+
+#[turbo_tasks::value_impl]
+impl IssueReporter for TestIssueReporter {
+    #[turbo_tasks::function]
+    async fn report_issues(
+        &self,
+        captured_issues: TransientInstance<ReadRef<CapturedIssues>>,
+        _source: TransientValue<RawVc>,
+    ) -> Result<BoolVc> {
+        let issue_tx = self.issue_tx.get_untracked().clone();
+        for issue in captured_issues.iter() {
+            let plain = issue.into_plain();
+            issue_tx.send((plain.await?, plain.dbg().await?)).await?;
+        }
+
+        Ok(BoolVc::cell(false))
+    }
+}

crates/next-dev-tests/tests/integration/next/tailwind/basic/issues/lint TP1004 fs.existsSync(__q____q____q____star__0__star__) is very dynamic-c19e79.txt

@@ -0,0 +1,25 @@
+PlainIssue {
+    severity: Warning,
+    context: "[project-with-next]/node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/previous-map.js",
+    category: "parse",
+    title: "lint TP1004 fs.existsSync(???*0*) is very dynamic",
+    description: "- *0* arguments[0]\n  ⚠\u{fe0f}  function calls are not analysed yet",
+    detail: "",
+    documentation_link: "",
+    source: Some(
+        PlainIssueSource {
+            asset: PlainAsset {
+                path: "node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/previous-map.js",
+            },
+            start: SourcePos {
+                line: 87,
+                column: 8,
+            },
+            end: SourcePos {
+                line: 87,
+                column: 8,
+            },
+        },
+    ),
+    sub_issues: [],
+}

crates/next-dev-tests/tests/integration/next/tailwind/basic/issues/lint TP1004 fs.readFileSync(__q____q____q____star__0__star__, __quo__utf-8__quo__) is very dynamic-58d7af.txt

@@ -0,0 +1,25 @@
+PlainIssue {
+    severity: Warning,
+    context: "[project-with-next]/node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/previous-map.js",
+    category: "parse",
+    title: "lint TP1004 fs.readFileSync(???*0*, \"utf-8\") is very dynamic",
+    description: "- *0* arguments[0]\n  ⚠\u{fe0f}  function calls are not analysed yet",
+    detail: "",
+    documentation_link: "",
+    source: Some(
+        PlainIssueSource {
+            asset: PlainAsset {
+                path: "node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/previous-map.js",
+            },
+            start: SourcePos {
+                line: 89,
+                column: 13,
+            },
+            end: SourcePos {
+                line: 89,
+                column: 13,
+            },
+        },
+    ),
+    sub_issues: [],
+}

crates/next-dev-tests/tests/integration/next/tailwind/basic/issues/lint TP1006 path.resolve(__q____q____q____star__0__star__) is very dynamic-667243.txt

@@ -0,0 +1,25 @@
+PlainIssue {
+    severity: Warning,
+    context: "[project-with-next]/node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/input.js",
+    category: "parse",
+    title: "lint TP1006 path.resolve(???*0*) is very dynamic",
+    description: "- *0* ???*1*[\"from\"]\n  ⚠\u{fe0f}  unknown object\n- *1* opts\n  ⚠\u{fe0f}  pattern without value",
+    detail: "",
+    documentation_link: "",
+    source: Some(
+        PlainIssueSource {
+            asset: PlainAsset {
+                path: "node_modules/.pnpm/postcss@8.4.20/node_modules/postcss/lib/input.js",
+            },
+            start: SourcePos {
+                line: 43,
+                column: 20,
+            },
+            end: SourcePos {
+                line: 43,
+                column: 20,
+            },
+        },
+    ),
+    sub_issues: [],
+}

crates/next-dev-tests/tests/integration/next/webpack-loaders/basic-options/input/pages/index.js

@@ -0,0 +1,17 @@
+import { useEffect } from "react";
+import source from "./hello.replace";
+
+export default function Home() {
+  useEffect(() => {
+    // Only run on client
+    import("@turbo/pack-test-harness").then(runTests);
+  });
+
+  return null;
+}
+
+function runTests() {
+  it("runs a loader with basic options", () => {
+    expect(source).toBe(3);
+  });
+}