test: some refactoring (WIP)

deno.json

@@ -39,6 +39,8 @@
     "exclude": ["**/*_test.*", "src/__OLD/**", "*.todo", "**/tests/**"]
   },
   "imports": {
+    "@fresh/internal/test-utils": "./packages/internal/src/test-utils.ts",
+    "@fresh/internal/versions": "./packages/internal/src/versions.ts",
     "@deno/doc": "jsr:@deno/doc@^0.172.0",
     "@deno/esbuild-plugin": "jsr:@deno/esbuild-plugin@^1.2.0",
     "@fresh/build-id": "jsr:@fresh/build-id@^1.0.0",

design/current-test-suite.md

@@ -0,0 +1,137 @@
+# Analysis of the Current Test Suite
+
+This document provides a detailed analysis of the existing test suite in the
+Fresh repository. It outlines the architecture, identifies key problem areas,
+and serves as a foundation for the proposed rewrite.
+
+## 1. Overview
+
+The test suite is spread across multiple packages, primarily `packages/fresh`
+and `packages/plugin-vite`. It uses a combination of Deno's built-in test runner
+(`Deno.test`), the standard library's `asserts` module, and a collection of
+custom utility functions.
+
+The core of the testing strategy revolves around:
+
+- **Fixture-based testing:** Small, self-contained Fresh projects (`fixtures`)
+  are used to test specific features.
+- **Child process execution:** Tests often spawn a Deno process to run a
+  development server (`deno run ... main.ts`).
+- **E2E and DOM inspection:** A headless browser (`@astral/astral`) or a virtual
+  DOM (`linkedom`) is used to interact with the running server and assert the
+  state of the rendered HTML.
+
+## 2. Key Files and Components
+
+- Internal test utilities (canonical source):
+  - `@fresh/internal/test-utils` (from `packages/internal/src/*`)
+    - Provides shared helpers used across the monorepo:
+      - Browser helpers: `withBrowser`, `withBrowserApp`
+      - DOM helpers: `parseHtml`, `assertSelector`, `assertNotSelector`,
+        `assertMetaContent`
+      - Process helpers: `withChildProcessServer`, `getStdOutput`
+      - Server helpers: `FakeServer`, `serveMiddleware`, `MockBuildCache`,
+        `createFakeFs`
+      - FS helpers: `withTmpDir`, `writeFiles`, `delay`, `updateFile`
+      - Misc: `waitFor`, `usingEnv`
+  - `@fresh/internal/versions`
+    - Centralized version constants used in tests/docs.
+
+- `packages/fresh/tests/test_utils.tsx`: Now only contains Fresh-specific test
+  bits:
+  - JSX-based helpers used within Fresh tests (`Doc`, `charset`, `favicon`).
+  - Build and fixtures helpers (`buildProd`, `ALL_ISLAND_DIR`,
+    `ISLAND_GROUP_DIR`).
+  - All generic utilities were removed and should be imported from
+    `@fresh/internal/test-utils`.
+
+- `packages/plugin-vite/tests/test_utils.ts`: Thin wrappers for plugin-vite:
+  - Uses `@fresh/internal/test-utils` primitives under the hood.
+  - Adds Vite-specific helpers like `prepareDevServer`, `launchDevServer`,
+    `spawnDevServer`, and `buildVite`.
+
+- **Fixture Directories**:
+  - `packages/fresh/tests/fixtures_islands/`
+  - `packages/fresh/tests/fixture_island_groups/`
+  - `packages/plugin-vite/tests/fixtures/`
+  - These contain the small Fresh projects that are run during tests.
+
+## 3. Problematic Areas (pre-refactor)
+
+### 3.1. Temporary Directory Management
+
+- **The Problem:** The `prepareDevServer` function in
+  `plugin-vite/tests/test_utils.ts` calls `withTmpDir` from
+  `fresh/src/test_utils.ts`. The `withTmpDir` function is configured to create
+  temporary directories with a prefix like `tmp_vite_` **inside the
+  `packages/plugin-vite` directory itself**.
+- **Impact:** This pollutes the project's source tree with temporary test
+  artifacts. This is messy for local development and can cause issues in CI
+  environments, as untracked files may be present during builds or other steps.
+  It also makes it harder to reason about the state of the repository. The user
+  has noted that moving this outside the repository breaks things due to path
+  dependencies (e.g., finding `deno.json`), indicating a tight coupling between
+  the test fixtures and the repository root.
+
+### 3.2. Redundant and Convoluted Logic
+
+- **The Problem:** There is significant overlap in the logic for starting a
+  server. `fresh` has `withChildProcessServer`, and `plugin-vite` has
+  `launchDevServer` and `spawnDevServer`, which are wrappers around
+  `withChildProcessServer`. This creates a confusing, multi-layered system.
+- **Impact:** It's difficult for a developer to understand the exact sequence of
+  events when a test is run. Debugging is complicated because the root cause of
+  a failure could be in any of the layers of abstraction. The separation of
+  concerns is unclear.
+
+### 3.3. Brittle Server Readiness Check
+
+- **The Problem:** The `withChildProcessServer` function determines if the
+  server is ready by reading the process's `stdout` line by line and looking for
+  a URL (`http://...`).
+- **Impact:** This is extremely fragile. It can fail if:
+  - The server's startup log message changes.
+  - The output is buffered differently across OSes or Deno versions.
+  - The log output is colored or contains other ANSI escape codes (though
+    `stripAnsiCode` is used, it's an extra step that can fail).
+  - The server logs an unrelated URL before it's actually ready to accept
+    connections.
+- This is a likely source of the test flakiness observed across different
+  environments.
+
+### 3.4. Cross-Package Dependencies
+
+- Pre-refactor, `plugin-vite` imported helpers from
+  `fresh/tests/test_utils.tsx`. This tight coupling has been removed by
+  introducing `@fresh/internal/test-utils`.
+
+### 3.5. JSX in Utility Files
+
+- Generic utilities were moved out into `@fresh/internal/test-utils`. The
+  remaining `.tsx` file is intentionally limited to JSX-only test helpers used
+  by the Fresh tests (e.g., `Doc`, `favicon`).
+
+### 3.6. Lack of Abstraction
+
+- **The Problem:** The primitives of the test suite (creating a temporary
+  fixture, starting a server, running an E2E test) are not well-abstracted. They
+  are implemented as a series of standalone functions that are chained together
+  within the test files themselves.
+- **Impact:** This leads to boilerplate code in the test files and makes it
+  difficult to see the "what" (the test's intent) because of all the "how" (the
+  setup and teardown mechanics).
+
+## 4. Current State (post-refactor)
+
+- Shared, generic test utilities live in `@fresh/internal/test-utils` inside
+  `packages/internal`. They are allowed to import Fresh internals as needed.
+- Fresh-specific JSX helpers remain in `packages/fresh/tests/test_utils.tsx`.
+- `packages/fresh/src/test_utils.ts` has been removed; runtime code no longer
+  imports test utilities. Where a fallback is needed (e.g., in `app.ts`), an
+  inline minimal `BuildCache` is used.
+- `plugin-vite` tests consume shared primitives from
+  `@fresh/internal/test-utils` and wrap them with Vite-specific helpers where
+  appropriate.
+
+This layout removes cross-package coupling, eliminates duplicated helpers, and
+keeps the repo green under `deno lint` and `deno check`.

packages/fresh/src/app.ts

@@ -8,6 +8,7 @@ import {
   runMiddlewares,
 } from "./middlewares/mod.ts";
 import { Context } from "./context.ts";
+import type { ServerIslandRegistry } from "./context.ts";
 import { mergePath, type Method, UrlPatternRouter } from "./router.ts";
 import type { FreshConfig, ResolvedFreshConfig } from "./config.ts";
 import type { BuildCache } from "./build_cache.ts";
@@ -28,7 +29,48 @@ import {
   newNotFoundCmd,
   newRouteCmd,
 } from "./commands.ts";
-import { MockBuildCache } from "./test_utils.ts";
+// Minimal fallback BuildCache for handler() when no build cache is present in dev/test.
+//
+// Why this exists
+// - In normal operation, a Builder populates a BuildCache and associates it
+//   with an App via setBuildCache(app, cache, mode). That cache powers features
+//   like island discovery, client entry resolution, entry assets, and the dev
+//   error overlay toggle.
+// - In unit tests or simple dev scenarios where an App is constructed directly
+//   and no Builder is involved, getBuildCache() returns null. Historically some
+//   tests relied on a MockBuildCache from test utils. We removed that runtime
+//   dependency from this module, so we provide a tiny internal fallback here to
+//   keep App.handler() usable without requiring test utilities.
+//
+// Behavior
+// - The fallback aims to be minimal and safe: it does not provide client assets
+//   or islands (no hydration). It only exposes empty registries/arrays and a
+//   features flag for the dev error overlay. This is sufficient for server-side
+//   tests and middleware behavior checks.
+// - To preserve previous dev/test behavior, the error overlay is enabled when
+//   the app runs in "development" mode. In production with a deployment id
+//   (i.e. on Deploy), we never use this fallback (see handler()) and instead
+//   throw with guidance to run a build.
+class __InlineFallbackBuildCache<State> implements BuildCache<State> {
+  root = "";
+  clientEntry = "";
+  islandRegistry: ServerIslandRegistry = new Map();
+  features: { errorOverlay: boolean };
+
+  constructor(mode: "development" | "production" = "production") {
+    this.features = { errorOverlay: mode === "development" };
+  }
+
+  getEntryAssets(): string[] {
+    return [];
+  }
+  getFsRoutes(): Command<State>[] {
+    return [];
+  }
+  readFile(): Promise<import("./build_cache.ts").StaticFile | null> {
+    return Promise.resolve(null);
+  }
+}
 
 // TODO: Completed type clashes in older Deno versions
 // deno-lint-ignore no-explicit-any
@@ -371,7 +413,9 @@ export class App<State> {
           `Could not find _fresh directory. Maybe you forgot to run "deno task build" or maybe you're trying to run "main.ts" directly instead of "_fresh/server.js"?`,
         );
       } else {
-        buildCache = new MockBuildCache([], this.config.mode);
+        // In dev/test fallback, enable error overlay feature to keep behavior
+        // consistent with MockBuildCache used in tests.
+        buildCache = new __InlineFallbackBuildCache<State>(this.config.mode);
       }
     }
 

packages/fresh/src/app_test.tsx

@@ -1,6 +1,6 @@
 import { expect } from "@std/expect";
 import { App } from "./app.ts";
-import { FakeServer } from "./test_utils.ts";
+import { FakeServer } from "@fresh/internal/test-utils";
 import { HttpError } from "./error.ts";
 
 Deno.test("App - .use()", async () => {

packages/fresh/src/context_test.tsx

@@ -2,9 +2,8 @@ import { expect } from "@std/expect";
 import { Context } from "./context.ts";
 import { App } from "fresh";
 import { asset } from "fresh/runtime";
-import { FakeServer } from "./test_utils.ts";
+import { FakeServer, parseHtml } from "@fresh/internal/test-utils";
 import { BUILD_ID } from "@fresh/build-id";
-import { parseHtml } from "../tests/test_utils.tsx";
 
 Deno.test("FreshReqContext.prototype.redirect", () => {
   let res = Context.prototype.redirect("/");

packages/fresh/src/dev/builder_test.ts

@@ -4,11 +4,12 @@ import { Builder, specToName } from "./builder.ts";
 import { App } from "../app.ts";
 import { DEV_ERROR_OVERLAY_URL } from "../constants.ts";
 import { BUILD_ID } from "@fresh/build-id";
-import { withTmpDir, writeFiles } from "../test_utils.ts";
 import {
   getStdOutput,
   withChildProcessServer,
-} from "../../tests/test_utils.tsx";
+  withTmpDir,
+  writeFiles,
+} from "@fresh/internal/test-utils";
 import { staticFiles } from "../middlewares/static_files.ts";
 
 Deno.test({

packages/fresh/src/dev/dev_build_cache_test.ts

@@ -1,7 +1,7 @@
 import { expect } from "@std/expect";
 import { MemoryBuildCache } from "./dev_build_cache.ts";
 import { FileTransformer } from "./file_transformer.ts";
-import { createFakeFs, withTmpDir } from "../test_utils.ts";
+import { createFakeFs, withTmpDir } from "@fresh/internal/test-utils";
 import type { ResolvedBuildConfig } from "./builder.ts";
 
 Deno.test({

packages/fresh/src/dev/file_transformer_test.ts

@@ -1,7 +1,7 @@
 import { expect } from "@std/expect";
 import type { FsAdapter } from "../fs.ts";
 import { FileTransformer, type ProcessedFile } from "./file_transformer.ts";
-import { delay } from "../test_utils.ts";
+import { delay } from "@fresh/internal/test-utils";
 
 function testTransformer(files: Record<string, string>, root = "/") {
   const mockFs: FsAdapter = {

packages/fresh/src/dev/fs_crawl_test.ts

@@ -1,5 +1,5 @@
 import { expect } from "@std/expect/expect";
-import { createFakeFs } from "../test_utils.ts";
+import { createFakeFs } from "@fresh/internal/test-utils";
 import { walkDir } from "./fs_crawl.ts";
 
 Deno.test("walkDir - ", async () => {

packages/fresh/src/dev/middlewares/error_overlay/middleware_test.tsx

@@ -1,6 +1,6 @@
 import { expect } from "@std/expect";
 import { App } from "../../../app.ts";
-import { FakeServer } from "../../../test_utils.ts";
+import { FakeServer } from "@fresh/internal/test-utils";
 import { devErrorOverlay } from "./middleware.tsx";
 import { HttpError } from "../../../error.ts";
 

packages/fresh/src/dev/update_check_test.ts

@@ -1,8 +1,7 @@
 import * as path from "@std/path";
 import denoJson from "../../deno.json" with { type: "json" };
-import { getStdOutput } from "../../tests/test_utils.tsx";
+import { getStdOutput, withTmpDir } from "@fresh/internal/test-utils";
 import { expect } from "@std/expect";
-import { withTmpDir } from "../test_utils.ts";
 import type { CheckFile } from "./update_check.ts";
 import { WEEK } from "../constants.ts";
 import { retry } from "@std/async/retry";

packages/fresh/src/fs_routes_test.tsx

@@ -1,12 +1,16 @@
 import { App, setBuildCache } from "./app.ts";
 import { type FreshFsMod, sortRoutePaths } from "./fs_routes.ts";
-import { delay, FakeServer, MockBuildCache } from "./test_utils.ts";
-import { createFakeFs } from "./test_utils.ts";
+import {
+  createFakeFs,
+  delay,
+  FakeServer,
+  MockBuildCache,
+  parseHtml,
+} from "@fresh/internal/test-utils";
 import { expect, fn } from "@std/expect";
 import { stub } from "@std/testing/mock";
 import { type HandlerByMethod, type HandlerFn, page } from "./handlers.ts";
 import type { Method } from "./router.ts";
-import { parseHtml } from "../tests/test_utils.tsx";
 import type { Context } from "./context.ts";
 import { HttpError } from "./error.ts";
 import { crawlRouteDir } from "./dev/fs_crawl.ts";

packages/fresh/src/middlewares/mod_test.ts

@@ -1,6 +1,6 @@
 import { runMiddlewares } from "./mod.ts";
 import { expect } from "@std/expect";
-import { serveMiddleware } from "../test_utils.ts";
+import { serveMiddleware } from "@fresh/internal/test-utils";
 import type { Middleware } from "./mod.ts";
 import type { Lazy, MaybeLazy } from "../types.ts";
 

packages/fresh/src/middlewares/static_files_test.ts

@@ -1,5 +1,5 @@
 import { staticFiles } from "./static_files.ts";
-import { serveMiddleware } from "../test_utils.ts";
+import { serveMiddleware } from "@fresh/internal/test-utils";
 import type { BuildCache, StaticFile } from "../build_cache.ts";
 import { expect } from "@std/expect";
 import { ASSET_CACHE_BUST_KEY } from "../constants.ts";

packages/fresh/src/middlewares/trailing_slashes_test.ts

@@ -1,7 +1,7 @@
 // deno-lint-ignore-file require-await
 import { trailingSlashes } from "./trailing_slashes.ts";
 import { expect } from "@std/expect";
-import { serveMiddleware } from "../test_utils.ts";
+import { serveMiddleware } from "@fresh/internal/test-utils";
 
 Deno.test("trailingSlashes - always", async () => {
   const middleware = trailingSlashes("always");

packages/fresh/tests/active_links_test.tsx

@@ -1,15 +1,12 @@
 import { App, staticFiles } from "fresh";
 import {
-  ALL_ISLAND_DIR,
   assertNotSelector,
   assertSelector,
-  buildProd,
-  Doc,
+  FakeServer,
   parseHtml,
   withBrowserApp,
-} from "./test_utils.tsx";
-
-import { FakeServer } from "../src/test_utils.ts";
+} from "@fresh/internal/test-utils";
+import { ALL_ISLAND_DIR, buildProd, Doc } from "./test_utils.tsx";
 import { Partial } from "fresh/runtime";
 
 const allIslandCache = await buildProd({ islandDir: ALL_ISLAND_DIR });
@@ -118,7 +115,7 @@ Deno.test({
         return ctx.render(<PartialPage />);
       });
 
-    await withBrowserApp(app, async (page, address) => {
+    await withBrowserApp(app.handler(), async (page, address) => {
       await page.goto(`${address}/active_nav_partial`);
 
       let doc = parseHtml(await page.content());

packages/fresh/tests/doc_examples_test.tsx

@@ -1,12 +1,14 @@
-import twDenoJson from "../../plugin-tailwindcss/deno.json" with {
-  type: "json",
-};
+// Versions and plugin-tailwind version are provided via @fresh/internal
 import * as Marked from "marked";
 import { ensureDir, walk } from "@std/fs";
 import { dirname, join, relative } from "@std/path";
 // import { expect } from "@std/expect/expect";
-import { withTmpDir } from "../src/test_utils.ts";
-import { FRESH_VERSION, PREACT_VERSION } from "../../update/src/update.ts";
+import { withTmpDir } from "@fresh/internal/test-utils";
+import {
+  FRESH_VERSION,
+  PREACT_VERSION,
+  TAILWIND_PLUGIN_VERSION,
+} from "@fresh/internal/versions";
 
 Deno.test("Docs Code example checks", async () => {
   await using tmp = await withTmpDir();
@@ -22,9 +24,9 @@ Deno.test("Docs Code example checks", async () => {
     imports: {
       fresh: `jsr:@fresh/core@${FRESH_VERSION}`,
       "@fresh/plugin-tailwind-v3":
-        `jsr:@fresh/plugin-tailwind@^${twDenoJson.version}`,
+        `jsr:@fresh/plugin-tailwind@^${TAILWIND_PLUGIN_VERSION}`,
       "@fresh/plugin-tailwind":
-        `jsr:@fresh/plugin-tailwind@^${twDenoJson.version}`,
+        `jsr:@fresh/plugin-tailwind@^${TAILWIND_PLUGIN_VERSION}`,
       preact: `npm:preact@^${PREACT_VERSION}`,
       "@deno/gfm": "jsr:@deno/gfm@^0.11.0",
       "@std/expect": "jsr:@std/expect@^1.0.16",