From c1697b94b44deae58481c426e3f821992e9055e8 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 22:14:19 -0500 Subject: [PATCH 1/5] update local testing workflow to allow for parallelism, improved dx --- package.json | 2 +- web-local/playwright.config.ts | 6 +- .../dashboard-flow-test-setup.ts | 3 +- .../dimension-and-measure-selection.spec.ts | 5 +- .../leaderboard-and-dim-table-sort.spec.ts | 8 +-- .../dashboard-flows/number-formatting.spec.ts | 8 +-- .../time-controls-from-config.spec.ts | 5 +- web-local/test/ui/dashboards.spec.ts | 12 +--- web-local/test/ui/models.spec.ts | 11 +-- web-local/test/ui/sources.spec.ts | 12 +--- web-local/test/ui/utils/getOpenPort.ts | 14 ++++ web-local/test/ui/utils/test.ts | 70 +++++++++++++++++++ 12 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 web-local/test/ui/utils/getOpenPort.ts create mode 100644 web-local/test/ui/utils/test.ts diff --git a/package.json b/package.json index 9afd1d85183..71be5966956 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "dev-web": "npm run dev -w web-local", "dev-runtime": "go run cli/main.go start dev-project --no-ui", "clean": "rm -rf dev-project", - "test": "npm run test -w web-common && npm run test -w web-local && npm run test -w web-auth" + "test": "npm run test -w web-common & npm run test -w web-auth & make cli && npm run test -w web-local" }, "overrides": { "@rgossiaux/svelte-headlessui": { diff --git a/web-local/playwright.config.ts b/web-local/playwright.config.ts index 4abcb13f3e1..aeb56aa963c 100644 --- a/web-local/playwright.config.ts +++ b/web-local/playwright.config.ts @@ -5,18 +5,16 @@ import { defineConfig, devices } from "@playwright/test"; export default defineConfig({ testDir: "./test/ui", /* Don't run tests in files in parallel */ - fullyParallel: false, + fullyParallel: !process.env.CI, /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, retries: 0, /* Opt out of parallel testing for now */ - workers: 1, + workers: process.env.CI ? 1 : 8, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: "html", /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { - /* Base URL to use in actions like `await page.goto('/')`. */ - baseURL: "http://localhost:8083", /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ trace: "on-first-retry", video: "retain-on-failure", diff --git a/web-local/test/ui/dashboard-flows/dashboard-flow-test-setup.ts b/web-local/test/ui/dashboard-flows/dashboard-flow-test-setup.ts index 20f88b078d9..f9093aedf4c 100644 --- a/web-local/test/ui/dashboard-flows/dashboard-flow-test-setup.ts +++ b/web-local/test/ui/dashboard-flows/dashboard-flow-test-setup.ts @@ -1,11 +1,10 @@ -import { test } from "@playwright/test"; import { createDashboardFromModel } from "web-local/test/ui/utils/dashboardHelpers"; import { createAdBidsModel } from "web-local/test/ui/utils/dataSpecifcHelpers"; +import { test } from "../utils/test"; export function useDashboardFlowTestSetup() { test.beforeEach(async ({ page }) => { test.setTimeout(60000); - await page.goto("/"); // disable animations await page.addStyleTag({ content: ` diff --git a/web-local/test/ui/dashboard-flows/dimension-and-measure-selection.spec.ts b/web-local/test/ui/dashboard-flows/dimension-and-measure-selection.spec.ts index 1f7ee4a5237..c420762cfa5 100644 --- a/web-local/test/ui/dashboard-flows/dimension-and-measure-selection.spec.ts +++ b/web-local/test/ui/dashboard-flows/dimension-and-measure-selection.spec.ts @@ -1,9 +1,8 @@ import { useDashboardFlowTestSetup } from "web-local/test/ui/dashboard-flows/dashboard-flow-test-setup"; -import { test, expect } from "@playwright/test"; -import { startRuntimeForEachTest } from "../utils/startRuntimeForEachTest"; +import { expect } from "@playwright/test"; +import { test } from "../utils/test"; test.describe("dimension and measure selectors", () => { - startRuntimeForEachTest(); // dashboard test setup useDashboardFlowTestSetup(); diff --git a/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts b/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts index 2771a41cef6..925aab64854 100644 --- a/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts +++ b/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts @@ -1,7 +1,7 @@ import { createDashboardFromModel } from "../utils/dashboardHelpers"; import { createAdBidsModel } from "../utils/dataSpecifcHelpers"; -import { test, expect, Locator } from "@playwright/test"; -import { startRuntimeForEachTest } from "../utils/startRuntimeForEachTest"; +import { expect, Locator } from "@playwright/test"; +import { test } from "../utils/test"; async function assertAAboveB(locA: Locator, locB: Locator) { const topA = await locA.boundingBox().then((box) => box?.y); @@ -15,11 +15,11 @@ async function assertAAboveB(locA: Locator, locB: Locator) { } test.describe("leaderboard and dimension table sorting", () => { - startRuntimeForEachTest(); + // startRuntimeForEachTest(); test("leaderboard and dimension table sorting", async ({ page }) => { test.setTimeout(30000); - await page.goto("/"); + // disable animations await page.addStyleTag({ content: ` diff --git a/web-local/test/ui/dashboard-flows/number-formatting.spec.ts b/web-local/test/ui/dashboard-flows/number-formatting.spec.ts index f5a05a8cd8e..903eb946307 100644 --- a/web-local/test/ui/dashboard-flows/number-formatting.spec.ts +++ b/web-local/test/ui/dashboard-flows/number-formatting.spec.ts @@ -3,16 +3,14 @@ import { waitForDashboard, } from "../utils/dashboardHelpers"; import { createAdBidsModel } from "../utils/dataSpecifcHelpers"; -import { test, expect } from "@playwright/test"; -import { startRuntimeForEachTest } from "../utils/startRuntimeForEachTest"; +import { expect } from "@playwright/test"; import { updateCodeEditor } from "../utils/commonHelpers"; +import { test } from "../utils/test"; test.describe("smoke tests for number formatting", () => { - startRuntimeForEachTest(); - test("smoke tests for number formatting", async ({ page }) => { test.setTimeout(60000); - await page.goto("/"); + // disable animations await page.addStyleTag({ content: ` diff --git a/web-local/test/ui/dashboard-flows/time-controls-from-config.spec.ts b/web-local/test/ui/dashboard-flows/time-controls-from-config.spec.ts index 9584a5bff1a..de517cf63d6 100644 --- a/web-local/test/ui/dashboard-flows/time-controls-from-config.spec.ts +++ b/web-local/test/ui/dashboard-flows/time-controls-from-config.spec.ts @@ -1,14 +1,13 @@ -import { expect, test } from "@playwright/test"; +import { expect } from "@playwright/test"; import { useDashboardFlowTestSetup } from "web-local/test/ui/dashboard-flows/dashboard-flow-test-setup"; import { updateCodeEditor } from "web-local/test/ui/utils/commonHelpers"; import { interactWithTimeRangeMenu, waitForDashboard, } from "web-local/test/ui/utils/dashboardHelpers"; -import { startRuntimeForEachTest } from "web-local/test/ui/utils/startRuntimeForEachTest"; +import { test } from "../utils/test"; test.describe("time controls settings from dashboard config", () => { - startRuntimeForEachTest(); // dashboard test setup useDashboardFlowTestSetup(); diff --git a/web-local/test/ui/dashboards.spec.ts b/web-local/test/ui/dashboards.spec.ts index 3d10daf997b..ea5a4dd08e9 100644 --- a/web-local/test/ui/dashboards.spec.ts +++ b/web-local/test/ui/dashboards.spec.ts @@ -1,4 +1,4 @@ -import { expect, Page, test } from "@playwright/test"; +import { expect, Page } from "@playwright/test"; import { updateCodeEditor } from "./utils/commonHelpers"; import { assertLeaderboards, @@ -18,15 +18,11 @@ import { } from "./utils/dataSpecifcHelpers"; import { TestEntityType, wrapRetryAssertion } from "./utils/helpers"; import { createOrReplaceSource } from "./utils/sourceHelpers"; -import { startRuntimeForEachTest } from "./utils/startRuntimeForEachTest"; import { waitForEntity } from "./utils/waitHelpers"; +import { test } from "./utils/test"; test.describe("dashboard", () => { - startRuntimeForEachTest(); - test("Autogenerate dashboard from source", async ({ page }) => { - await page.goto("/"); - await createOrReplaceSource(page, "AdBids.csv", "AdBids"); await createDashboardFromSource(page, "AdBids"); await waitForEntity( @@ -39,8 +35,6 @@ test.describe("dashboard", () => { }); test("Autogenerate dashboard from model", async ({ page }) => { - await page.goto("/"); - await createAdBidsModel(page); await Promise.all([ waitForEntity( @@ -99,7 +93,7 @@ test.describe("dashboard", () => { // }); test.setTimeout(60000); - await page.goto("/"); + // disable animations await page.addStyleTag({ content: ` diff --git a/web-local/test/ui/models.spec.ts b/web-local/test/ui/models.spec.ts index 8bae1b521ec..945f902ab09 100644 --- a/web-local/test/ui/models.spec.ts +++ b/web-local/test/ui/models.spec.ts @@ -1,4 +1,3 @@ -import { test } from "@playwright/test"; import { deleteEntity, gotoEntity, @@ -16,15 +15,11 @@ import { modelHasError, } from "./utils/modelHelpers"; import { createOrReplaceSource } from "./utils/sourceHelpers"; -import { startRuntimeForEachTest } from "./utils/startRuntimeForEachTest"; import { entityNotPresent, waitForEntity } from "./utils/waitHelpers"; +import { test } from "./utils/test"; test.describe("models", () => { - startRuntimeForEachTest(); - test("Create and edit model", async ({ page }) => { - await page.goto("/"); - await createOrReplaceSource(page, "AdBids.csv", "AdBids"); await createOrReplaceSource(page, "AdImpressions.tsv", "AdImpressions"); @@ -50,8 +45,6 @@ test.describe("models", () => { }); test("Rename and delete model", async ({ page }) => { - await page.goto("/"); - // make sure AdBids_rename_delete is present await createModel(page, "AdBids_rename_delete"); @@ -76,8 +69,6 @@ test.describe("models", () => { }); test("Create model from source", async ({ page }) => { - await page.goto("/"); - await createOrReplaceSource(page, "AdBids.csv", "AdBids"); await Promise.all([ diff --git a/web-local/test/ui/sources.spec.ts b/web-local/test/ui/sources.spec.ts index e2f38d2a65b..718fac34483 100644 --- a/web-local/test/ui/sources.spec.ts +++ b/web-local/test/ui/sources.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect } from "@playwright/test"; import { deleteEntity, renameEntityUsingMenu, @@ -14,15 +14,11 @@ import { createOrReplaceSource, uploadFile, } from "./utils/sourceHelpers"; -import { startRuntimeForEachTest } from "./utils/startRuntimeForEachTest"; import { entityNotPresent, waitForEntity } from "./utils/waitHelpers"; +import { test } from "./utils/test"; test.describe("sources", () => { - startRuntimeForEachTest(); - test("Import sources", async ({ page }) => { - await page.goto("/"); - await Promise.all([ waitForAdBids(page, "AdBids"), uploadFile(page, "AdBids.csv"), @@ -46,8 +42,6 @@ test.describe("sources", () => { }); test("Rename and delete sources", async ({ page }) => { - await page.goto("/"); - await createOrReplaceSource(page, "AdBids.csv", "AdBids"); // rename @@ -62,8 +56,6 @@ test.describe("sources", () => { }); test("Edit source", async ({ page }) => { - await page.goto("/"); - // Upload data & create two sources await createOrReplaceSource(page, "AdImpressions.tsv", "AdImpressions"); await createOrReplaceSource(page, "AdBids.csv", "AdBids"); diff --git a/web-local/test/ui/utils/getOpenPort.ts b/web-local/test/ui/utils/getOpenPort.ts new file mode 100644 index 00000000000..3bdc7ec391e --- /dev/null +++ b/web-local/test/ui/utils/getOpenPort.ts @@ -0,0 +1,14 @@ +import { createServer } from "net"; + +export async function getOpenPort(): Promise { + return new Promise((res) => { + const srv = createServer(); + srv.listen(0, () => { + const address = srv?.address(); + if (!address || typeof address === "string") { + throw new Error("Invalid address"); + } + srv.close(() => res(address.port)); + }); + }); +} diff --git a/web-local/test/ui/utils/test.ts b/web-local/test/ui/utils/test.ts new file mode 100644 index 00000000000..18c863d57c8 --- /dev/null +++ b/web-local/test/ui/utils/test.ts @@ -0,0 +1,70 @@ +import { test as base } from "@playwright/test"; +import { rmSync, writeFileSync, existsSync, mkdirSync } from "fs"; +import { spawn } from "node:child_process"; +import treeKill from "tree-kill"; +import { getOpenPort } from "./getOpenPort"; +import { asyncWaitUntil } from "@rilldata/web-common/lib/waitUtils"; +import axios from "axios"; + +const BASE_PROJECT_DIRECTORY = "temp/test-project"; + +export const test = base.extend({ + page: async ({ page }, use) => { + const TEST_PORT = await getOpenPort(); + const TEST_PORT_GRPC = await getOpenPort(); + const TEST_PROJECT_DIRECTORY = `${BASE_PROJECT_DIRECTORY}-${TEST_PORT}`; + + rmSync(TEST_PROJECT_DIRECTORY, { + force: true, + recursive: true, + }); + + if (!existsSync(TEST_PROJECT_DIRECTORY)) { + mkdirSync(TEST_PROJECT_DIRECTORY, { recursive: true }); + } + + // Add `rill.yaml` file to the project repo + writeFileSync( + `${TEST_PROJECT_DIRECTORY}/rill.yaml`, + 'compiler: rill-beta\ntitle: "Test Project"' + ); + + const cmd = `start --no-open --port ${TEST_PORT} --port-grpc ${TEST_PORT_GRPC} --db ${TEST_PROJECT_DIRECTORY}/stage.db?rill_pool_size=4 ${TEST_PROJECT_DIRECTORY}`; + + const childProcess = spawn("../rill", cmd.split(" "), { + stdio: "inherit", + shell: true, + }); + + childProcess.on("error", console.log); + + // Ping runtime until it's ready + await asyncWaitUntil(async () => { + try { + const response = await axios.get( + `http://localhost:${TEST_PORT}/v1/ping` + ); + return response.status === 200; + } catch (err) { + return false; + } + }); + + await page.goto(`http://localhost:${TEST_PORT}`); + + await use(page); + + const processExit = new Promise((resolve) => { + childProcess.on("exit", resolve); + }); + + if (childProcess.pid) treeKill(childProcess.pid); + + await processExit; + + rmSync(TEST_PROJECT_DIRECTORY, { + force: true, + recursive: true, + }); + }, +}); From f56c748ef03ecf80bb8041ac927f69f390f17dd3 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 14 Dec 2023 22:38:07 -0500 Subject: [PATCH 2/5] update comment --- web-local/playwright.config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web-local/playwright.config.ts b/web-local/playwright.config.ts index aeb56aa963c..4e3cc18616b 100644 --- a/web-local/playwright.config.ts +++ b/web-local/playwright.config.ts @@ -4,12 +4,12 @@ import { defineConfig, devices } from "@playwright/test"; */ export default defineConfig({ testDir: "./test/ui", - /* Don't run tests in files in parallel */ + /* Don't run tests in files in parallel in CI*/ fullyParallel: !process.env.CI, /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, retries: 0, - /* Opt out of parallel testing for now */ + /* Opt out of parallel testing in CI */ workers: process.env.CI ? 1 : 8, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: "html", From 16e3523bdfe728af57f65b0dfb2722b03b54c1b6 Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Mon, 18 Dec 2023 14:16:08 -0500 Subject: [PATCH 3/5] remove extraneous comment --- .../ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts b/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts index 925aab64854..fdff8b55a6c 100644 --- a/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts +++ b/web-local/test/ui/dashboard-flows/leaderboard-and-dim-table-sort.spec.ts @@ -15,8 +15,6 @@ async function assertAAboveB(locA: Locator, locB: Locator) { } test.describe("leaderboard and dimension table sorting", () => { - // startRuntimeForEachTest(); - test("leaderboard and dimension table sorting", async ({ page }) => { test.setTimeout(30000); From 56dfa5e466be5d18850961fc642696492678d30c Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Mon, 18 Dec 2023 14:50:24 -0500 Subject: [PATCH 4/5] move file/directory removal to before process exit --- web-local/test/ui/utils/test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web-local/test/ui/utils/test.ts b/web-local/test/ui/utils/test.ts index 18c863d57c8..3237ec577b6 100644 --- a/web-local/test/ui/utils/test.ts +++ b/web-local/test/ui/utils/test.ts @@ -54,6 +54,11 @@ export const test = base.extend({ await use(page); + rmSync(TEST_PROJECT_DIRECTORY, { + force: true, + recursive: true, + }); + const processExit = new Promise((resolve) => { childProcess.on("exit", resolve); }); @@ -61,10 +66,5 @@ export const test = base.extend({ if (childProcess.pid) treeKill(childProcess.pid); await processExit; - - rmSync(TEST_PROJECT_DIRECTORY, { - force: true, - recursive: true, - }); }, }); From 9a3ac5a1d2d430249e2eadd510df3beaa8b5cbec Mon Sep 17 00:00:00 2001 From: Brian Holmes Date: Thu, 15 Feb 2024 14:40:50 -0500 Subject: [PATCH 5/5] prettier --- web-local/tests/utils/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web-local/tests/utils/test.ts b/web-local/tests/utils/test.ts index 3237ec577b6..eeb77374379 100644 --- a/web-local/tests/utils/test.ts +++ b/web-local/tests/utils/test.ts @@ -26,7 +26,7 @@ export const test = base.extend({ // Add `rill.yaml` file to the project repo writeFileSync( `${TEST_PROJECT_DIRECTORY}/rill.yaml`, - 'compiler: rill-beta\ntitle: "Test Project"' + 'compiler: rill-beta\ntitle: "Test Project"', ); const cmd = `start --no-open --port ${TEST_PORT} --port-grpc ${TEST_PORT_GRPC} --db ${TEST_PROJECT_DIRECTORY}/stage.db?rill_pool_size=4 ${TEST_PROJECT_DIRECTORY}`; @@ -42,7 +42,7 @@ export const test = base.extend({ await asyncWaitUntil(async () => { try { const response = await axios.get( - `http://localhost:${TEST_PORT}/v1/ping` + `http://localhost:${TEST_PORT}/v1/ping`, ); return response.status === 200; } catch (err) {