Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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