Unverified 提交 c337156d authored 作者: wwwillchen-bot's avatar wwwillchen-bot 提交者: GitHub

fix: use per-worker port for fake LLM server in parallel tests (#2557)

## Summary - Pass `fakeLlmPort` through `preLaunchHook` so each parallel worker uses its own fake LLM server instance - Dynamically generate webServer configs in `playwright.config.ts` based on parallelism setting - Allow fake-llm-server to accept port via command line argument (`--port=XXXX`) This ensures parallel Playwright tests don't conflict when accessing the fake LLM server. ## Test plan - [x] Lint checks pass - [x] All 784 unit tests pass - [ ] Run E2E tests in parallel to verify no port conflicts 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2557" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Fixes parallel E2E conflicts by running a dedicated fake LLM server per Playwright worker and passing its port through preLaunchHook, env vars, and app/test URLs so all requests hit the worker’s server. - **Bug Fixes** - Compute per-worker fakeLlmPort (FAKE_LLM_BASE_PORT + parallelIndex), expose via preLaunchHook, set FAKE_LLM_PORT, and pass into PageObject, Settings, GitHubConnector; Azure tests use it for TEST_AZURE_BASE_URL. - Use worker-scoped URLs for Azure, GitHub test APIs, Ollama, LM Studio, Engine, Gateway, and OpenAI provider; Electron GitHub/Vercel handlers read FAKE_LLM_PORT instead of hardcoded 3500. - Generate one webServer per worker with build+start to avoid startup races; health checks target each worker; reuse servers locally; extract FAKE_LLM_BASE_PORT to e2e-tests/helpers/test-ports.ts. - fake-llm-server accepts --port or PORT with NaN validation; docs updated with parallel port isolation and rebase workflow notes. <sup>Written for commit 407923e90ddc9cf4d9ec98ccc56849154d2ad932. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: 's avatarWill Chen <willchen90@gmail.com> Co-authored-by: 's avatarClaude Opus 4.5 <noreply@anthropic.com> Co-authored-by: 's avatarclaude[bot] <41898282+claude[bot]@users.noreply.github.com>
上级 a0a50774
......@@ -3,8 +3,8 @@ import { testWithConfigSkipIfWindows } from "./helpers/test_helper";
// Set environment variables before the test runs to enable Azure testing
const testAzure = testWithConfigSkipIfWindows({
preLaunchHook: async () => {
process.env.TEST_AZURE_BASE_URL = "http://localhost:3500/azure";
preLaunchHook: async ({ fakeLlmPort }) => {
process.env.TEST_AZURE_BASE_URL = `http://localhost:${fakeLlmPort}/azure`;
process.env.AZURE_API_KEY = "fake-azure-key-for-testing";
process.env.AZURE_RESOURCE_NAME = "fake-resource-for-testing";
},
......
......@@ -12,9 +12,16 @@ import { execSync } from "child_process";
import { showDebugLogs } from "./constants";
import { PageObject } from "./page-objects";
import { FAKE_LLM_BASE_PORT } from "./test-ports";
export interface ElectronConfig {
preLaunchHook?: ({ userDataDir }: { userDataDir: string }) => Promise<void>;
preLaunchHook?: ({
userDataDir,
fakeLlmPort,
}: {
userDataDir: string;
fakeLlmPort: number;
}) => Promise<void>;
showSetupScreen?: boolean;
}
......@@ -41,6 +48,7 @@ export const test = base.extend<{
const po = new PageObject(electronApp, page, {
userDataDir: (electronApp as any).$dyadUserDataDir,
fakeLlmPort: (electronApp as any).$fakeLlmPort,
});
await use(po);
},
......@@ -67,16 +75,21 @@ export const test = base.extend<{
{ auto: true },
],
electronApp: [
async ({ electronConfig }, use) => {
async ({ electronConfig }, use, testInfo) => {
// find the latest build in the out directory
const latestBuild = eph.findLatestBuild();
// parse the directory and find paths and other info
const appInfo = eph.parseElectronApp(latestBuild);
process.env.OLLAMA_HOST = "http://localhost:3500/ollama";
process.env.LM_STUDIO_BASE_URL_FOR_TESTING =
"http://localhost:3500/lmstudio";
process.env.DYAD_ENGINE_URL = "http://localhost:3500/engine/v1";
process.env.DYAD_GATEWAY_URL = "http://localhost:3500/gateway/v1";
// Calculate worker-specific port for fake LLM server
// Each parallel worker gets its own server to avoid test interference
const fakeLlmPort = FAKE_LLM_BASE_PORT + testInfo.parallelIndex;
process.env.FAKE_LLM_PORT = String(fakeLlmPort);
process.env.OLLAMA_HOST = `http://localhost:${fakeLlmPort}/ollama`;
process.env.LM_STUDIO_BASE_URL_FOR_TESTING = `http://localhost:${fakeLlmPort}/lmstudio`;
process.env.DYAD_ENGINE_URL = `http://localhost:${fakeLlmPort}/engine/v1`;
process.env.DYAD_GATEWAY_URL = `http://localhost:${fakeLlmPort}/gateway/v1`;
process.env.E2E_TEST_BUILD = "true";
if (!electronConfig.showSetupScreen) {
// This is just a hack to avoid the AI setup screen.
......@@ -85,7 +98,7 @@ export const test = base.extend<{
const baseTmpDir = os.tmpdir();
const userDataDir = path.join(baseTmpDir, `dyad-e2e-tests-${Date.now()}`);
if (electronConfig.preLaunchHook) {
await electronConfig.preLaunchHook({ userDataDir });
await electronConfig.preLaunchHook({ userDataDir, fakeLlmPort });
}
const electronApp = await electron.launch({
args: [
......@@ -101,6 +114,7 @@ export const test = base.extend<{
// },
});
(electronApp as any).$dyadUserDataDir = userDataDir;
(electronApp as any).$fakeLlmPort = fakeLlmPort;
console.log("electronApp launched!");
if (showDebugLogs) {
......
......@@ -37,6 +37,7 @@ import { ProModesDialog } from "./dialogs/ProModesDialog";
export class PageObject {
public userDataDir: string;
public fakeLlmPort: number;
// Component page objects (exposed for direct access)
public githubConnector: GitHubConnector;
......@@ -55,12 +56,13 @@ export class PageObject {
constructor(
public electronApp: ElectronApplication,
public page: Page,
{ userDataDir }: { userDataDir: string },
{ userDataDir, fakeLlmPort }: { userDataDir: string; fakeLlmPort: number },
) {
this.userDataDir = userDataDir;
this.fakeLlmPort = fakeLlmPort;
// Initialize component page objects
this.githubConnector = new GitHubConnector(this.page);
this.githubConnector = new GitHubConnector(this.page, fakeLlmPort);
this.chatActions = new ChatActions(this.page);
this.previewPanel = new PreviewPanel(this.page);
this.codeEditor = new CodeEditor(this.page);
......@@ -69,7 +71,7 @@ export class PageObject {
this.agentConsent = new AgentConsent(this.page);
this.navigation = new Navigation(this.page);
this.modelPicker = new ModelPicker(this.page);
this.settings = new Settings(this.page, userDataDir);
this.settings = new Settings(this.page, userDataDir, fakeLlmPort);
this.appManagement = new AppManagement(this.page, electronApp, userDataDir);
this.promptLibrary = new PromptLibrary(this.page);
}
......
......@@ -6,7 +6,10 @@
import { Page, expect } from "@playwright/test";
export class GitHubConnector {
constructor(public page: Page) {}
constructor(
public page: Page,
public fakeLlmPort: number,
) {}
async connect() {
await this.page.getByRole("button", { name: "Connect to GitHub" }).click();
......@@ -89,15 +92,15 @@ export class GitHubConnector {
async clearPushEvents() {
const response = await this.page.request.post(
"http://localhost:3500/github/api/test/clear-push-events",
`http://localhost:${this.fakeLlmPort}/github/api/test/clear-push-events`,
);
return await response.json();
}
async getPushEvents(repo?: string) {
const url = repo
? `http://localhost:3500/github/api/test/push-events?repo=${repo}`
: "http://localhost:3500/github/api/test/push-events";
? `http://localhost:${this.fakeLlmPort}/github/api/test/push-events?repo=${repo}`
: `http://localhost:${this.fakeLlmPort}/github/api/test/push-events`;
const response = await this.page.request.get(url);
return await response.json();
}
......
......@@ -11,6 +11,7 @@ export class Settings {
constructor(
public page: Page,
private userDataDir: string,
private fakeLlmPort: number,
) {}
async toggleAutoApprove() {
......@@ -130,7 +131,7 @@ export class Settings {
await this.page.getByText("API Base URLThe base URL for").click();
await this.page
.getByRole("textbox", { name: "API Base URL" })
.fill("http://localhost:3500/v1");
.fill(`http://localhost:${this.fakeLlmPort}/v1`);
await this.page.getByRole("button", { name: "Add Provider" }).click();
}
......
// Base port for fake LLM servers - each worker gets its own port
// Worker 0 -> 3500, Worker 1 -> 3501, etc.
// Shared between playwright.config.ts and test helpers to avoid
// importing the Playwright config from test code.
export const FAKE_LLM_BASE_PORT = 3500;
import { PlaywrightTestConfig } from "@playwright/test";
import os from "os";
import { FAKE_LLM_BASE_PORT } from "./e2e-tests/helpers/test-ports";
export { FAKE_LLM_BASE_PORT };
const timestamp = new Date().toISOString().replace(/[:.]/g, "-");
const parallelism = parseInt(process.env.PLAYWRIGHT_PARALLELISM || "1", 10);
// Generate webServer configurations for each parallel worker
// Each worker needs its own fake LLM server to avoid test interference
function generateWebServerConfigs(): PlaywrightTestConfig["webServer"] {
const configs: NonNullable<PlaywrightTestConfig["webServer"]> = [];
for (let i = 0; i < parallelism; i++) {
const port = FAKE_LLM_BASE_PORT + i;
configs.push({
// All servers run build to avoid race conditions since Playwright
// starts all webServer entries concurrently
command: `cd testing/fake-llm-server && npm run build && npm start -- --port=${port}`,
url: `http://localhost:${port}/health`,
// In CI, always start a fresh server; locally, reuse if one is already running
reuseExistingServer: !process.env.CI,
});
}
return configs;
}
const config: PlaywrightTestConfig = {
testDir: "./e2e-tests",
// Enable parallel test execution - E2E test builds skip the singleton lock
// Read parallelism from env var, default to 1 if not set
workers: parseInt(process.env.PLAYWRIGHT_PARALLELISM || "1", 10),
workers: parallelism,
retries: parseInt(
process.env.PLAYWRIGHT_RETRIES ?? (process.env.CI ? "2" : "0"),
10,
......@@ -48,12 +72,7 @@ const config: PlaywrightTestConfig = {
// video: "retain-on-failure",
},
webServer: [
{
command: `cd testing/fake-llm-server && npm run build && npm start`,
url: "http://localhost:3500/health",
},
],
webServer: generateWebServerConfigs(),
};
export default config;
......@@ -91,6 +91,12 @@ If the output under test contains non-deterministic or platform-specific content
The Pro mode build settings (Web Access, Turbo Edits, Smart Context) are inside a collapsed `<Accordion>` in `ProModeSelector`. E2E test helpers must expand the accordion before interacting with elements inside it. The `ProModesDialog` class in `e2e-tests/helpers/page-objects/dialogs/ProModesDialog.ts` has an `expandBuildModeSettings()` method that handles this — call it before clicking any build mode setting buttons.
## Parallel test port isolation
Each parallel Playwright worker gets its own fake LLM server on port `FAKE_LLM_BASE_PORT + parallelIndex`. The base port constant lives in `e2e-tests/helpers/test-ports.ts` (not in `playwright.config.ts`) to avoid importing the Playwright config from test code.
When adding new test server URLs, update **both** the test fixtures (`e2e-tests/helpers/fixtures.ts`) and the Electron app source that consumes them. The app reads `process.env.FAKE_LLM_PORT` to build its `TEST_SERVER_BASE` URL — if you hardcode a port in app source, parallel workers will all hit the same server.
## E2E test fixtures with .dyad directories
When adding E2E test fixtures that need a `.dyad` directory for testing:
......
......@@ -43,3 +43,14 @@ gh api repos/dyad-sh/dyad/issues/{PR_NUMBER}/labels -f "labels[]=label-name"
- When resolving import conflicts (e.g., `<<<<<<< HEAD` with different imports), keep **both** imports if both are valid and needed by the component
- When resolving conflicts in i18n-related commits, watch for duplicate constant definitions that conflict with imports from `@/lib/schemas` (e.g., `DEFAULT_ZOOM_LEVEL`)
- If both sides of a conflict have valid imports/hooks, keep both and remove any duplicate constant redefinitions
## Rebasing with uncommitted changes
If you need to rebase but have uncommitted changes (e.g., package-lock.json from startup npm install):
1. Stash changes: `git stash push -m "Stash changes before rebase"`
2. Rebase: `git rebase upstream/main` (resolve conflicts if needed)
3. Pop stash: `git stash pop`
4. Discard spurious changes like package-lock.json (if package.json unchanged): `git restore package-lock.json`
This prevents rebase conflicts from uncommitted changes while preserving any work in progress.
......@@ -57,7 +57,7 @@ const GITHUB_CLIENT_ID = process.env.GITHUB_CLIENT_ID || "Ov23liWV2HdC0RBLecWx";
// Use test server URLs when in test mode
const TEST_SERVER_BASE = "http://localhost:3500";
const TEST_SERVER_BASE = `http://localhost:${process.env.FAKE_LLM_PORT || "3500"}`;
const GITHUB_DEVICE_CODE_URL = IS_TEST_BUILD
? `${TEST_SERVER_BASE}/github/login/device/code`
......
......@@ -27,7 +27,7 @@ import {
const logger = log.scope("vercel_handlers");
// Use test server URLs when in test mode
const TEST_SERVER_BASE = "http://localhost:3500";
const TEST_SERVER_BASE = `http://localhost:${process.env.FAKE_LLM_PORT || "3500"}`;
const VERCEL_API_BASE = IS_TEST_BUILD
? `${TEST_SERVER_BASE}/vercel/api`
......
......@@ -5,7 +5,7 @@ import { IS_TEST_BUILD } from "./test_utils";
const logger = log.scope("vercel_utils");
// Use test server URLs when in test mode
const TEST_SERVER_BASE = "http://localhost:3500";
const TEST_SERVER_BASE = `http://localhost:${process.env.FAKE_LLM_PORT || "3500"}`;
const VERCEL_API_BASE = IS_TEST_BUILD
? `${TEST_SERVER_BASE}/vercel/api`
......
......@@ -24,7 +24,15 @@ app.use(cors());
app.use(express.json({ limit: "50mb" }));
app.use(express.urlencoded({ extended: true, limit: "50mb" }));
const PORT = 3500;
// Allow port to be specified via command line argument or environment variable
// Usage: node dist/index.js --port=3501 OR PORT=3501 node dist/index.js
const portArg = process.argv.find((arg) => arg.startsWith("--port="));
const PORT = portArg
? parseInt(portArg.split("=")[1], 10)
: parseInt(process.env.PORT || "3500", 10);
if (isNaN(PORT)) {
throw new Error(`Invalid port: ${portArg || process.env.PORT}`);
}
// Helper function to create OpenAI-like streaming response chunks
export function createStreamChunk(
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论