diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..1274e02 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,6 @@ +# Set the default behavior, in case people don't have core.autocrlf set. +* text=auto + +# Make everything in the e2e-tests/snapshots directory use LF line endings. +# Otherwise we'll get diffs in the snapshots when running on Windows. +e2e-tests/snapshots/** text eol=lf \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5259b9e..956d7c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,11 +17,19 @@ defaults: jobs: test: - # Why Mac? + # Why Mac and Windows? # I can't run electron playwright on ubuntu-latest and # Linux support for Dyad is experimental so not as important # as Mac + Windows - runs-on: macos-latest + strategy: + fail-fast: false + matrix: + os: + [ + { name: "windows", image: "windows-latest" }, + { name: "macos", image: "macos-latest" }, + ] + runs-on: ${{ matrix.os.image }} steps: - name: Checkout code uses: actions/checkout@v4 @@ -34,8 +42,12 @@ jobs: - name: Install node modules run: npm install - name: Presubmit check (e.g. lint, format) + # do not run this on Windows (it fails and not necessary) + if: contains(matrix.os.name, 'macos') run: npm run presubmit - name: Type-checking + # do not run this on windows (it's redunant) + if: contains(matrix.os.name, 'macos') run: npm run ts - name: Install Chromium browser for Playwright run: npx playwright install chromium --with-deps @@ -44,8 +56,9 @@ jobs: - name: Prep test server run: cd testing/fake-llm-server && npm install && npm run build && cd - - name: E2E tests - # Add debug logging to make it easier to see what's failing - run: DEBUG=pw:browser npm run e2e + # You can add debug logging to make it easier to see what's failing + # by adding "DEBUG=pw:browser" in front. + run: npm run e2e - uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 if: failure() with: diff --git a/e2e-tests/approve.spec.ts b/e2e-tests/approve.spec.ts index 5624ab7..f5d686c 100644 --- a/e2e-tests/approve.spec.ts +++ b/e2e-tests/approve.spec.ts @@ -1,4 +1,4 @@ -import { test } from "./helpers/test_helper"; +import { test, Timeout } from "./helpers/test_helper"; import { expect } from "@playwright/test"; test("write to index, approve, check preview", async ({ po }) => { @@ -11,6 +11,8 @@ test("write to index, approve, check preview", async ({ po }) => { await po.snapshotMessages(); // This can be pretty slow because it's waiting for the app to build. - await expect(po.getPreviewIframeElement()).toBeVisible({ timeout: 15_000 }); + await expect(po.getPreviewIframeElement()).toBeVisible({ + timeout: Timeout.LONG, + }); await po.snapshotPreview(); }); diff --git a/e2e-tests/auto_approve.spec.ts b/e2e-tests/auto_approve.spec.ts index d7c628c..2d4aec2 100644 --- a/e2e-tests/auto_approve.spec.ts +++ b/e2e-tests/auto_approve.spec.ts @@ -1,4 +1,4 @@ -import { test } from "./helpers/test_helper"; +import { test, Timeout } from "./helpers/test_helper"; import { expect } from "@playwright/test"; test("auto-approve", async ({ po }) => { @@ -7,6 +7,8 @@ test("auto-approve", async ({ po }) => { await po.snapshotMessages(); // This can be pretty slow because it's waiting for the app to build. - await expect(po.getPreviewIframeElement()).toBeVisible({ timeout: 15_000 }); + await expect(po.getPreviewIframeElement()).toBeVisible({ + timeout: Timeout.LONG, + }); await po.snapshotPreview(); }); diff --git a/e2e-tests/dump_messages.spec.ts b/e2e-tests/dump_messages.spec.ts index a2dd3ef..c715160 100644 --- a/e2e-tests/dump_messages.spec.ts +++ b/e2e-tests/dump_messages.spec.ts @@ -1,7 +1,10 @@ -import { test } from "./helpers/test_helper"; +import { testSkipIfWindows } from "./helpers/test_helper"; // This is useful to make sure the messages are being sent correctly. -test("dump messages", async ({ po }) => { +// +// Why skip on Windows? The file ordering is not stable between runs +// but unclear why. +testSkipIfWindows("dump messages", async ({ po }) => { await po.setUp(); await po.sendPrompt("[dump]"); await po.snapshotServerDump(); diff --git a/e2e-tests/fixtures/import-app/minimal-with-ai-rules/AI_RULES.md b/e2e-tests/fixtures/import-app/minimal-with-ai-rules/AI_RULES.md index 34abae8..e3ced03 100644 --- a/e2e-tests/fixtures/import-app/minimal-with-ai-rules/AI_RULES.md +++ b/e2e-tests/fixtures/import-app/minimal-with-ai-rules/AI_RULES.md @@ -1 +1,3 @@ +[[beginning of AI_RULES.md]] There's already AI rules... +[[end of AI_RULES.md]] diff --git a/e2e-tests/helpers/test_helper.ts b/e2e-tests/helpers/test_helper.ts index cf41a69..c46a43b 100644 --- a/e2e-tests/helpers/test_helper.ts +++ b/e2e-tests/helpers/test_helper.ts @@ -3,9 +3,16 @@ import { findLatestBuild, parseElectronApp } from "electron-playwright-helpers"; import { ElectronApplication, _electron as electron } from "playwright"; import fs from "fs"; import path from "path"; +import os from "os"; const showDebugLogs = process.env.DEBUG_LOGS === "true"; +export const Timeout = { + // Why make this a constant? In some platforms, perhaps locally, + // we may want to shorten this. + LONG: 30_000, +}; + class PageObject { private userDataDir: string; @@ -99,7 +106,7 @@ class PageObject { async snapshotPreview() { const iframe = this.getPreviewIframeElement(); await expect(iframe.contentFrame().locator("body")).toMatchAriaSnapshot({ - timeout: 30_000, + timeout: Timeout.LONG, }); } @@ -469,19 +476,31 @@ export const test = base.extend<{ }); await use(electronApp); - await electronApp.close(); + // Why are we doing a force kill on Windows? + // + // Otherwise, Playwright will just hang on the test cleanup + // because the electron app does NOT ever fully quit due to + // Windows' strict resource locking (e.g. file locking). + if (os.platform() === "win32") { + electronApp.process().kill(); + } else { + await electronApp.close(); + } }, { auto: true }, ], }); +// Wrapper that skips tests on Windows platform +export const testSkipIfWindows = os.platform() === "win32" ? test.skip : test; + function prettifyDump( dumpContent: string, { onlyLastMessage = false }: { onlyLastMessage?: boolean } = {}, ) { const parsedDump = JSON.parse(dumpContent) as Array<{ role: string; - content: string; + content: string | Array<{}>; }>; const messages = onlyLastMessage ? parsedDump.slice(-1) : parsedDump; @@ -491,6 +510,8 @@ function prettifyDump( const content = Array.isArray(message.content) ? JSON.stringify(message.content) : message.content + // Normalize line endings to always use \n + .replace(/\r\n/g, "\n") // We remove package.json because it's flaky. // Depending on whether pnpm install is run, it will be modified, // and the contents and timestamp (thus affecting order) will be affected. diff --git a/e2e-tests/rebuild.spec.ts b/e2e-tests/rebuild.spec.ts index e2a1e48..1e717e4 100644 --- a/e2e-tests/rebuild.spec.ts +++ b/e2e-tests/rebuild.spec.ts @@ -1,4 +1,4 @@ -import { test } from "./helpers/test_helper"; +import { test, Timeout } from "./helpers/test_helper"; import { expect } from "@playwright/test"; import fs from "fs"; import path from "path"; @@ -15,7 +15,7 @@ test("rebuild app", async ({ po }) => { await po.clickRebuild(); await expect(po.locateLoadingAppPreview()).toBeVisible(); await expect(po.locateLoadingAppPreview()).not.toBeVisible({ - timeout: 15_000, + timeout: Timeout.LONG, }); // Check that the file is removed with the rebuild diff --git a/e2e-tests/restart.spec.ts b/e2e-tests/restart.spec.ts index f8408d6..d3dbc48 100644 --- a/e2e-tests/restart.spec.ts +++ b/e2e-tests/restart.spec.ts @@ -1,4 +1,4 @@ -import { test } from "./helpers/test_helper"; +import { test, Timeout } from "./helpers/test_helper"; import { expect } from "@playwright/test"; test("restart app", async ({ po }) => { @@ -8,7 +8,7 @@ test("restart app", async ({ po }) => { await po.clickRestart(); await expect(po.locateLoadingAppPreview()).toBeVisible(); await expect(po.locateLoadingAppPreview()).not.toBeVisible({ - timeout: 15_000, + timeout: Timeout.LONG, }); await po.snapshotPreview(); diff --git a/e2e-tests/snapshots/import.spec.ts_server-dump.txt b/e2e-tests/snapshots/import.spec.ts_server-dump.txt index c257cf8..61aaaee 100644 --- a/e2e-tests/snapshots/import.spec.ts_server-dump.txt +++ b/e2e-tests/snapshots/import.spec.ts_server-dump.txt @@ -325,7 +325,9 @@ This structured thinking ensures you: 4. Maintain a consistent approach to problem-solving +[[beginning of AI_RULES.md]] There's already AI rules... +[[end of AI_RULES.md]] # REMEMBER @@ -377,7 +379,14 @@ You need to first add Supabase to your app and then we can add auth. === role: user -message: This is my codebase. +message: This is my codebase. +[[beginning of AI_RULES.md]] +There's already AI rules... +[[end of AI_RULES.md]] + + + + @@ -394,31 +403,6 @@ message: This is my codebase. - -There's already AI rules... - - - - -import { defineConfig } from "vite"; -import react from "@vitejs/plugin-react-swc"; -import path from "path"; - -export default defineConfig(() => ({ - server: { - host: "::", - port: 8080, - }, - plugins: [react()], - resolve: { - alias: { - "@": path.resolve(__dirname, "./src"), - }, - }, -})); - - - const App = () =>
Minimal imported app
; @@ -439,6 +423,26 @@ createRoot(document.getElementById("root")!).render(); + +import { defineConfig } from "vite"; +import react from "@vitejs/plugin-react-swc"; +import path from "path"; + +export default defineConfig(() => ({ + server: { + host: "::", + port: 8080, + }, + plugins: [react()], + resolve: { + alias: { + "@": path.resolve(__dirname, "./src"), + }, + }, +})); + + + === diff --git a/e2e-tests/undo.spec.ts b/e2e-tests/undo.spec.ts index 537f094..5483fce 100644 --- a/e2e-tests/undo.spec.ts +++ b/e2e-tests/undo.spec.ts @@ -1,4 +1,4 @@ -import { test } from "./helpers/test_helper"; +import { test, Timeout } from "./helpers/test_helper"; import { expect } from "@playwright/test"; test("undo", async ({ po }) => { @@ -11,7 +11,7 @@ test("undo", async ({ po }) => { iframe.contentFrame().getByText("Testing:write-index(2)!"), ).toBeVisible({ // This can be pretty slow because it's waiting for the app to build. - timeout: 15_000, + timeout: Timeout.LONG, }); await po.clickUndo(); @@ -20,7 +20,7 @@ test("undo", async ({ po }) => { iframe.contentFrame().getByText("Testing:write-index!"), ).toBeVisible({ // Also, could be slow. - timeout: 15_000, + timeout: Timeout.LONG, }); await po.clickUndo(); @@ -29,6 +29,6 @@ test("undo", async ({ po }) => { iframe.contentFrame().getByText("Welcome to Your Blank App"), ).toBeVisible({ // Also, could be slow. - timeout: 15_000, + timeout: Timeout.LONG, }); }); diff --git a/package-lock.json b/package-lock.json index ecc606f..87454e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "dyad", - "version": "0.7.0", + "version": "0.8.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "dyad", - "version": "0.7.0", + "version": "0.8.0", "license": "MIT", "dependencies": { "@ai-sdk/anthropic": "^1.2.8", @@ -96,6 +96,7 @@ "@typescript-eslint/eslint-plugin": "^5.62.0", "@typescript-eslint/parser": "^5.62.0", "@vitest/ui": "^3.1.1", + "cross-env": "^7.0.3", "drizzle-kit": "^0.30.6", "electron": "35.1.4", "eslint": "^8.57.1", @@ -7873,6 +7874,25 @@ "dev": true, "license": "MIT" }, + "node_modules/cross-env": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz", + "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==", + "dev": true, + "license": "MIT", + "dependencies": { + "cross-spawn": "^7.0.1" + }, + "bin": { + "cross-env": "src/bin/cross-env.js", + "cross-env-shell": "src/bin/cross-env-shell.js" + }, + "engines": { + "node": ">=10.14", + "npm": ">=6", + "yarn": ">=1" + } + }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", diff --git a/package.json b/package.json index 8180637..8dc321e 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "test:ui": "vitest --ui", "extract-codebase": "ts-node scripts/extract-codebase.ts", "prepare": "husky install", - "pre:e2e": "E2E_TEST_BUILD=true npm run package", + "pre:e2e": "cross-env E2E_TEST_BUILD=true npm run package", "e2e": "playwright test" }, "keywords": [], @@ -64,6 +64,7 @@ "@typescript-eslint/eslint-plugin": "^5.62.0", "@typescript-eslint/parser": "^5.62.0", "@vitest/ui": "^3.1.1", + "cross-env": "^7.0.3", "drizzle-kit": "^0.30.6", "electron": "35.1.4", "eslint": "^8.57.1", diff --git a/src/ipc/handlers/import_handlers.ts b/src/ipc/handlers/import_handlers.ts index 8307cc6..9882c8a 100644 --- a/src/ipc/handlers/import_handlers.ts +++ b/src/ipc/handlers/import_handlers.ts @@ -11,6 +11,7 @@ import { eq } from "drizzle-orm"; import git from "isomorphic-git"; import { getGitAuthor } from "../utils/git_author"; import { ImportAppParams, ImportAppResult } from "../ipc_types"; +import { copyDirectoryRecursive } from "../utils/file_utils"; const logger = log.scope("import-handlers"); const handle = createLoggedHandler(logger); @@ -88,11 +89,10 @@ export function registerImportHandlers() { throw error; } } - // Copy the app folder to the Dyad apps directory, excluding node_modules - await fs.cp(sourcePath, destPath, { - recursive: true, - filter: (source) => !source.includes("node_modules"), - }); + // Copy the app folder to the Dyad apps directory. + // Why not use fs.cp? Because we want stable ordering for + // tests. + await copyDirectoryRecursive(sourcePath, destPath); const isGitRepo = await fs .access(path.join(destPath, ".git")) diff --git a/src/ipc/utils/file_utils.ts b/src/ipc/utils/file_utils.ts index 72d4e8e..8065087 100644 --- a/src/ipc/utils/file_utils.ts +++ b/src/ipc/utils/file_utils.ts @@ -39,13 +39,19 @@ export async function copyDirectoryRecursive( ) { await fsPromises.mkdir(destination, { recursive: true }); const entries = await fsPromises.readdir(source, { withFileTypes: true }); + // Why do we sort? This ensures stable ordering of files across platforms + // which is helpful for tests (and has no practical downsides). + entries.sort(); for (const entry of entries) { const srcPath = path.join(source, entry.name); const destPath = path.join(destination, entry.name); if (entry.isDirectory()) { - await copyDirectoryRecursive(srcPath, destPath); + // Exclude node_modules directories + if (entry.name !== "node_modules") { + await copyDirectoryRecursive(srcPath, destPath); + } } else { await fsPromises.copyFile(srcPath, destPath); } diff --git a/src/utils/codebase.ts b/src/utils/codebase.ts index 84d4644..7aaf423 100644 --- a/src/utils/codebase.ts +++ b/src/utils/codebase.ts @@ -226,7 +226,9 @@ async function collectFiles(dir: string, baseDir: string): Promise { // Skip large configuration files or generated code (just include the path) function isOmittedFile(relativePath: string): boolean { return ( - relativePath.includes(path.join("src", "components", "ui")) || + // Why are we not using path.join here? + // Because we have already normalized the path to use /. + relativePath.includes("src/components/ui") || relativePath.includes("eslint.config") || relativePath.includes("tsconfig.json") || relativePath.includes("package-lock.json") || @@ -243,7 +245,11 @@ const OMITTED_FILE_CONTENT = "// Contents omitted for brevity"; */ async function formatFile(filePath: string, baseDir: string): Promise { try { - const relativePath = path.relative(baseDir, filePath); + const relativePath = path + .relative(baseDir, filePath) + // Why? Normalize Windows-style paths which causes lots of weird issues (e.g. Git commit) + .split(path.sep) + .join("/"); if (isOmittedFile(relativePath)) { return ` @@ -310,7 +316,11 @@ export async function extractCodebase(appPath: string): Promise<{ const formattedContent = await formatFile(file, appPath); // Get raw content for the files array - const relativePath = path.relative(appPath, file); + const relativePath = path + .relative(appPath, file) + // Why? Normalize Windows-style paths which causes lots of weird issues (e.g. Git commit) + .split(path.sep) + .join("/"); const fileContent = isOmittedFile(relativePath) ? OMITTED_FILE_CONTENT : await readFileWithCache(file);