Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions app/e2e/dashboard-portability.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
timeout: 5_000,
});
await expect(dialog.getByText("E2E NeoDash Import Test")).toBeVisible();
await expect(dialog.getByText("4 widgets")).toBeVisible();
await expect(dialog.getByText("6 widgets")).toBeVisible();

// No connection mapping should appear (NeoDash skips it)
await expect(dialog.getByText("Map each connection")).not.toBeVisible();
Expand All @@ -173,9 +173,9 @@
// Should redirect to the imported dashboard
await page.waitForURL(/\/[\w-]+$/, { timeout: 15_000 });

// Verify 4 widget cards rendered (NeoDash report titles are not preserved
// as widget titles — the converter maps settings but not report.title)
await expect(page.locator("[data-testid='widget-card']")).toHaveCount(4, {
// Verify 6 widget cards rendered — includes gantt and graph3d→graph
// Report titles are now preserved as widget settings.title
await expect(page.locator("[data-testid='widget-card']")).toHaveCount(6, {
timeout: 15_000,
});

Expand Down Expand Up @@ -240,7 +240,7 @@

// Should import without crashing — unknown type falls back to JSON Viewer
await page.waitForURL(/\/[\w-]+$/, { timeout: 15_000 });
await expect(page.getByText("JSON Viewer")).toBeVisible({

Check failure on line 243 in app/e2e/dashboard-portability.spec.ts

View workflow job for this annotation

GitHub Actions / E2E (shard 2/5)

[chromium] › e2e/dashboard-portability.spec.ts:189:7 › NeoDash legacy import › NeoDash import with unsupported chart type degrades to JSON viewer

1) [chromium] › e2e/dashboard-portability.spec.ts:189:7 › NeoDash legacy import › NeoDash import with unsupported chart type degrades to JSON viewer Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByText('JSON Viewer') Expected: visible Timeout: 15000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 15000ms - waiting for getByText('JSON Viewer') 241 | // Should import without crashing — unknown type falls back to JSON Viewer 242 | await page.waitForURL(/\/[\w-]+$/, { timeout: 15_000 }); > 243 | await expect(page.getByText("JSON Viewer")).toBeVisible({ | ^ 244 | timeout: 15_000, 245 | }); 246 | at /home/runner/work/neoboard/neoboard/app/e2e/dashboard-portability.spec.ts:243:51

Check failure on line 243 in app/e2e/dashboard-portability.spec.ts

View workflow job for this annotation

GitHub Actions / E2E (shard 2/5)

[chromium] › e2e/dashboard-portability.spec.ts:189:7 › NeoDash legacy import › NeoDash import with unsupported chart type degrades to JSON viewer

1) [chromium] › e2e/dashboard-portability.spec.ts:189:7 › NeoDash legacy import › NeoDash import with unsupported chart type degrades to JSON viewer Error: expect(locator).toBeVisible() failed Locator: getByText('JSON Viewer') Expected: visible Timeout: 15000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 15000ms - waiting for getByText('JSON Viewer') 241 | // Should import without crashing — unknown type falls back to JSON Viewer 242 | await page.waitForURL(/\/[\w-]+$/, { timeout: 15_000 }); > 243 | await expect(page.getByText("JSON Viewer")).toBeVisible({ | ^ 244 | timeout: 15_000, 245 | }); 246 | at /home/runner/work/neoboard/neoboard/app/e2e/dashboard-portability.spec.ts:243:51
timeout: 15_000,
});

Expand Down
24 changes: 24 additions & 0 deletions app/e2e/fixtures/imports/neodash-sample.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,30 @@
"height": 4,
"settings": {},
"parameters": {}
},
{
"id": "r5",
"title": "Movie Timeline",
"type": "gantt",
"query": "MATCH (m:Movie) RETURN m.title AS task, m.released AS start, m.released + 2 AS end LIMIT 5",
"x": 0,
"y": 8,
"width": 12,
"height": 4,
"settings": {},
"parameters": {}
},
{
"id": "r6",
"title": "3D Network",
"type": "graph3d",
"query": "MATCH (p:Person)-[r:ACTED_IN]->(m:Movie) RETURN p, r, m LIMIT 20",
"x": 0,
"y": 12,
"width": 12,
"height": 6,
"settings": {},
"parameters": {}
}
]
}
Expand Down
69 changes: 50 additions & 19 deletions app/e2e/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as path from "node:path";
import * as crypto from "node:crypto";
import * as net from "node:net";
import * as http from "node:http";
import { spawn } from "node:child_process";
import { spawn, execSync } from "node:child_process";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { migrate } from "drizzle-orm/postgres-js/migrator";
Expand Down Expand Up @@ -249,28 +249,59 @@ export default async function globalSetup() {
process.env.TEST_NEO4J_BOLT_URL = `bolt://localhost:${neo4jBoltPort}`;
process.env.TEST_PG_PORT = String(pgPort);

// ── Start the Next.js server on a dynamically allocated port ────────────
// Env vars are passed directly to the process — .env.local is never touched.
// ── Build & start the Next.js server on a dynamically allocated port ───
// Always use a production build + `next start` for consistent, fast E2E
// runs. `next dev` recompiles pages on demand which adds 10+ minutes of
// webpack overhead locally. A one-time `next build` (~2 min) then instant
// `next start` is what CI already does and is dramatically faster overall.
const appDir = path.resolve(__dirname, "..");
const serverCmd = process.env.CI ? "start" : "dev";
console.log(
`⏳ Starting Next.js ${serverCmd} server on port ${serverPort}...`,
);
const args = ["next", serverCmd, "--port", String(serverPort)];
// Use webpack explicitly — Turbopack (Next.js 16 default) doesn't correctly
// resolve CJS/ESM interop for the @neoboard/connection package at runtime.
if (serverCmd === "dev") args.push("--webpack");
const serverEnv = {
...process.env,
DATABASE_URL: databaseUrl,
ENCRYPTION_KEY: TEST_ENCRYPTION_KEY,
API_KEY_HMAC_SECRET: TEST_API_KEY_HMAC_SECRET,
NEXTAUTH_SECRET: TEST_NEXTAUTH_SECRET,
NEXTAUTH_URL: `http://localhost:${serverPort}`,
};

// Build once — skip if a previous build exists and E2E_SKIP_BUILD is set,
// OR if .next/BUILD_ID already exists (auto-detect cached build).
const buildIdPath = path.join(appDir, ".next", "BUILD_ID");
const hasCachedBuild = fs.existsSync(buildIdPath);

if (process.env.E2E_SKIP_BUILD && !hasCachedBuild) {
throw new Error(
"E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
"Run `npx next build` once or unset E2E_SKIP_BUILD.",
);
}

if (process.env.E2E_SKIP_BUILD) {
console.log(
"⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
);
} else {
if (hasCachedBuild) {
console.log(
"⏳ Rebuilding Next.js (production)... (set E2E_SKIP_BUILD=1 to reuse previous build)",
);
} else {
console.log("⏳ Building Next.js (production)...");
}
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
}
Comment on lines +272 to +297

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

E2E_SKIP_BUILD=0 is truthy and will still skip the build.

process.env.E2E_SKIP_BUILD is a string, so E2E_SKIP_BUILD=0 / false / "" (unset via export then overridden) all evaluate truthy (except ""). A contributor who flips this to 0 expecting "off" will unintentionally skip the rebuild and quietly run stale bundles in E2E. Consider parsing it explicitly.

♻️ Proposed tweak
-  if (process.env.E2E_SKIP_BUILD && !hasCachedBuild) {
+  const skipBuild = /^(1|true|yes)$/i.test(process.env.E2E_SKIP_BUILD ?? "");
+
+  if (skipBuild && !hasCachedBuild) {
     throw new Error(
       "E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
         "Run `npx next build` once or unset E2E_SKIP_BUILD.",
     );
   }
 
-  if (process.env.E2E_SKIP_BUILD) {
+  if (skipBuild) {
     console.log(
       "⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
     );
   } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (process.env.E2E_SKIP_BUILD && !hasCachedBuild) {
throw new Error(
"E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
"Run `npx next build` once or unset E2E_SKIP_BUILD.",
);
}
if (process.env.E2E_SKIP_BUILD) {
console.log(
"⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
);
} else {
if (hasCachedBuild) {
console.log(
"⏳ Rebuilding Next.js (production)... (set E2E_SKIP_BUILD=1 to reuse previous build)",
);
} else {
console.log("⏳ Building Next.js (production)...");
}
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
}
const skipBuild = /^(1|true|yes)$/i.test(process.env.E2E_SKIP_BUILD ?? "");
if (skipBuild && !hasCachedBuild) {
throw new Error(
"E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
"Run `npx next build` once or unset E2E_SKIP_BUILD.",
);
}
if (skipBuild) {
console.log(
"⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
);
} else {
if (hasCachedBuild) {
console.log(
"⏳ Rebuilding Next.js (production)... (set E2E_SKIP_BUILD=1 to reuse previous build)",
);
} else {
console.log("⏳ Building Next.js (production)...");
}
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/e2e/global-setup.ts` around lines 272 - 297, The code currently treats
process.env.E2E_SKIP_BUILD as a boolean which makes values like "0" or "false"
still count as truthy and skip the build; update the checks around
process.env.E2E_SKIP_BUILD (the block that decides whether to skip or run
execSync("npx next build", { cwd: appDir, stdio: "inherit", env: serverEnv }))
to explicitly parse/normalize the env var (e.g. treat "0", "false", "no" as
falsy and "1", "true", "yes" as truthy) before branching; use the normalized
boolean in both the initial hasCachedBuild guard and the later if/else that logs
skipping vs building so a user setting E2E_SKIP_BUILD=0 will not skip the
rebuild.


console.log(`⏳ Starting Next.js production server on port ${serverPort}...`);
const args = ["next", "start", "--port", String(serverPort)];
const server = spawn("npx", args, {
cwd: appDir,
stdio: "pipe",
env: {
...process.env,
DATABASE_URL: databaseUrl,
ENCRYPTION_KEY: TEST_ENCRYPTION_KEY,
API_KEY_HMAC_SECRET: TEST_API_KEY_HMAC_SECRET,
NEXTAUTH_SECRET: TEST_NEXTAUTH_SECRET,
NEXTAUTH_URL: `http://localhost:${serverPort}`,
},
env: serverEnv,
detached: true,
});
server.unref();
Expand Down
88 changes: 72 additions & 16 deletions app/e2e/new-charts.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { test, expect, ALICE, createTestDashboard, typeInEditor } from "./fixtures";
import {
test,
expect,
ALICE,
createTestDashboard,
typeInEditor,
} from "./fixtures";

// ---------------------------------------------------------------------------
// New chart types (v0.8) — creation flow tests
// New chart types — creation flow tests
// ---------------------------------------------------------------------------
// These tests verify the end-to-end creation flow for each new chart type:
// Gauge, Sankey, Sunburst, Radar, Treemap.
// Gauge, Sankey, Sunburst, Radar, Treemap, Gantt.
//
// We focus on the creation flow (dialog → query → add widget) rather than
// visual rendering details — chart rendering is verified by unit tests.
Expand Down Expand Up @@ -50,7 +56,9 @@ test.describe("New chart types — creation flow", () => {
);

// The Add Widget button should be enabled (no Run required for this flow)
await expect(dialog.getByRole("button", { name: "Add Widget" })).toBeEnabled({
await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
Expand Down Expand Up @@ -78,7 +86,9 @@ test.describe("New chart types — creation flow", () => {
"MATCH (p:Person)-[:ACTED_IN]->(m:Movie) WITH m, count(p) AS cast RETURN m.title AS name, cast AS value ORDER BY cast DESC LIMIT 10",
);

await expect(dialog.getByRole("button", { name: "Add Widget" })).toBeEnabled({
await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
Expand Down Expand Up @@ -106,7 +116,9 @@ test.describe("New chart types — creation flow", () => {
"MATCH (p:Person)-[r]->(m:Movie) WITH type(r) AS indicator, count(*) AS value RETURN indicator, value, 100 AS max",
);

await expect(dialog.getByRole("button", { name: "Add Widget" })).toBeEnabled({
await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
Expand Down Expand Up @@ -134,7 +146,9 @@ test.describe("New chart types — creation flow", () => {
"MATCH (p:Person)-[r:ACTED_IN]->(m:Movie) RETURN p.name AS source, m.title AS target, 1 AS value LIMIT 15",
);

await expect(dialog.getByRole("button", { name: "Add Widget" })).toBeEnabled({
await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
Expand Down Expand Up @@ -162,7 +176,39 @@ test.describe("New chart types — creation flow", () => {
"MATCH (p:Person)-[r]->(m:Movie) RETURN type(r) AS parent, m.title AS name, 1 AS value LIMIT 20",
);

await expect(dialog.getByRole("button", { name: "Add Widget" })).toBeEnabled({
await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
});

test("should create a Gantt widget", async ({ page }) => {
test.setTimeout(60_000);

await page.getByRole("button", { name: "Add Widget" }).first().click();
const dialog = page.getByRole("dialog", { name: "Add Widget" });

// Select Neo4j connection first
await dialog.getByRole("combobox").nth(0).click();
await page.getByRole("option").first().click();

// Select Gantt chart type
await dialog.getByRole("combobox").nth(1).click();
await page.getByRole("option", { name: "Gantt" }).click();

// Type query — tasks with start/end timestamps
await typeInEditor(
dialog,
page,
"MATCH (p:Person)-[:ACTED_IN]->(m:Movie) WITH m.title AS task, m.released AS start, m.released + 2 AS end RETURN task, start, end LIMIT 8",
);

await expect(
dialog.getByRole("button", { name: "Add Widget" }),
).toBeEnabled({
timeout: 10_000,
});
await dialog.getByRole("button", { name: "Add Widget" }).click();
Expand All @@ -182,11 +228,15 @@ test.describe("Widget Showcase seed dashboard", () => {
await page.waitForURL(/\/[\w-]+$/, { timeout: 10_000 });
});

test("should render the Widget Showcase dashboard with widget cards", async ({ page }) => {
test("should render the Widget Showcase dashboard with widget cards", async ({
page,
}) => {
test.setTimeout(60_000);

// At least one widget card should be visible on the page
await expect(page.locator("[data-testid='widget-card']").first()).toBeVisible({
await expect(
page.locator("[data-testid='widget-card']").first(),
).toBeVisible({
timeout: 15_000,
});
});
Expand All @@ -202,7 +252,9 @@ test.describe("Widget Showcase seed dashboard", () => {
test("should show the Rule-Based Styling page tab", async ({ page }) => {
test.setTimeout(30_000);

await expect(page.getByRole("tab", { name: "Rule-Based Styling" })).toBeVisible({
await expect(
page.getByRole("tab", { name: "Rule-Based Styling" }),
).toBeVisible({
timeout: 10_000,
});
});
Expand All @@ -214,18 +266,22 @@ test.describe("Widget Showcase seed dashboard", () => {
await page.getByRole("tab", { name: "Simple Charts" }).click();

// Multiple widget cards should be present (bar, line, pie, single-value, table, gauge, radar, sankey, treemap, sunburst)
await expect(page.locator("[data-testid='widget-card']").first()).toBeVisible({
await expect(
page.locator("[data-testid='widget-card']").first(),
).toBeVisible({
timeout: 15_000,
});

// At least 10 widgets should be on this page
const widgetCount = await page.locator("[data-testid='widget-card']").count();
const widgetCount = await page
.locator("[data-testid='widget-card']")
.count();
expect(widgetCount).toBeGreaterThanOrEqual(10);
});

test("should show Color Palettes page tab", async ({ page }) => {
await expect(
page.getByRole("tab", { name: "Color Palettes" }),
).toBeVisible({ timeout: 10_000 });
await expect(page.getByRole("tab", { name: "Color Palettes" })).toBeVisible(
{ timeout: 10_000 },
);
});
});
10 changes: 4 additions & 6 deletions app/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ export default defineConfig({
fullyParallel: true,
forbidOnly: !!process.env.CI,
retries: 1,
// CI: 2 workers for parallel execution against the production server.
// Locally: 4 workers — Playwright's auto-detect picks based on CPU cores
// but collapses to serial under Docker-testcontainer load, turning a
// ~11-minute run into a ~20-minute run. An explicit number keeps local
// timing deterministic regardless of host contention.
workers: process.env.CI ? 2 : 4,
// CI: 2 workers (constrained runner resources).
// Locally: 6 workers — balances parallelism with server/DB contention.
// Override with --workers=N on the CLI for experimentation.
workers: process.env.CI ? 2 : 6,
// CI: github (PR annotations) + list (real-time stream) + blob (for cross-shard merge).
// Local: interactive HTML report.
reporter: process.env.CI ? [["github"], ["list"], ["blob"]] : "html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ vi.mock("@neoboard/components", () => ({
),
}));

// Mock @radix-ui/react-select primitives used directly in transform-editor
vi.mock("@radix-ui/react-select", () => ({
Item: ({ children, value }: { children: React.ReactNode; value: string }) => (
<option value={value}>{children}</option>
),
ItemText: ({ children }: { children: React.ReactNode }) => (
<span>{children}</span>
),
ItemIndicator: ({ children }: { children: React.ReactNode }) => (
<>{children}</>
),
}));

// Mock ValueOrParamInput
vi.mock("../value-or-param-input", () => ({
ValueOrParamInput: (props: Record<string, unknown>) => (
Expand Down
2 changes: 2 additions & 0 deletions app/src/components/widget-editor/chart-type-selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
Sun,
Radar,
LayoutGrid,
GanttChart as GanttChartIcon,
} from "lucide-react";
import type { LucideIcon } from "lucide-react";
import { Label, Combobox } from "@neoboard/components";
Expand All @@ -43,6 +44,7 @@ export const chartTypeIcons: Record<ChartType, LucideIcon> = {
sunburst: Sun,
radar: Radar,
treemap: LayoutGrid,
gantt: GanttChartIcon,
};

/** Get label + Icon for a chart type. Label from registry, Icon from UI layer. */
Expand Down
Loading
Loading