From c8107597a5d7bebfc053d80d6e744d32417c0f35 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 1 May 2024 16:12:42 -0500 Subject: [PATCH 01/24] make gen --- docs/cli/server.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli/server.md b/docs/cli/server.md index a7c32c2d78420..0f010d73d4fb2 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -711,7 +711,7 @@ Enables capturing of logs as events in traces. This is useful for debugging, but | YAML | provisioning.daemons | | Default | 3 | -Number of provisioner daemons to create on start. If builds are stuck in queued state for a long time, consider increasing this. +Number of terraform provisioner daemons to create on start. If builds are stuck in queued state for a long time, consider increasing this. ### --provisioner-daemon-poll-interval From 5294740a62e1beca1a0b1a35514b8d87dda4f9dc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 1 May 2024 16:26:41 -0500 Subject: [PATCH 02/24] add icon to health provisioner page for prov type --- .../src/pages/HealthPage/ProvisionerDaemonsPage.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx index 63ee9280987c5..a676786b00558 100644 --- a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx +++ b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx @@ -4,6 +4,7 @@ import CloseIcon from "@mui/icons-material/Close"; import Person from "@mui/icons-material/Person"; import Sell from "@mui/icons-material/Sell"; import SwapHoriz from "@mui/icons-material/SwapHoriz"; +import ViewInArIcon from "@mui/icons-material/ViewInAr"; import IconButton from "@mui/material/IconButton"; import Tooltip from "@mui/material/Tooltip"; import type { FC } from "react"; @@ -115,6 +116,18 @@ export const ProvisionerDaemonsPage: FC = () => { gap: 12, }} > + { + // Add pills for the supported provisioenr types. + daemon.provisioners.map((provType) => { + return ( + + }> + {provType} + + + ); + }) + } }> {daemon.api_version} From ae89373050a2efebbdc4cddc49e98a808b7d427d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 1 May 2024 16:32:19 -0500 Subject: [PATCH 03/24] update golden files --- cli/testdata/coder_server_--help.golden | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 6d8f866c11c0b..69ee62e07c472 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -460,8 +460,8 @@ updating, and deleting workspace resources. server. --provisioner-daemons int, $CODER_PROVISIONER_DAEMONS (default: 3) - Number of provisioner daemons to create on start. If builds are stuck - in queued state for a long time, consider increasing this. + Number of terraform provisioner daemons to create on start. If builds + are stuck in queued state for a long time, consider increasing this. TELEMETRY OPTIONS: Telemetry is critical to our ability to improve Coder. We strip all From e12e721236235f4c5d5d023a0a7c5687ee43d2e1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 1 May 2024 17:02:19 -0500 Subject: [PATCH 04/24] update golden files --- enterprise/cli/testdata/coder_server_--help.golden | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 4d4576d6d57cc..4a873ce9c8779 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -461,8 +461,8 @@ updating, and deleting workspace resources. server. --provisioner-daemons int, $CODER_PROVISIONER_DAEMONS (default: 3) - Number of provisioner daemons to create on start. If builds are stuck - in queued state for a long time, consider increasing this. + Number of terraform provisioner daemons to create on start. If builds + are stuck in queued state for a long time, consider increasing this. TELEMETRY OPTIONS: Telemetry is critical to our ability to improve Coder. We strip all From f4dbfb5913d1d2fd07a905bbdb97e7987fd14c3a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 14:00:57 -0500 Subject: [PATCH 05/24] make gen --- docs/cli/server.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli/server.md b/docs/cli/server.md index 0f010d73d4fb2..a7c32c2d78420 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -711,7 +711,7 @@ Enables capturing of logs as events in traces. This is useful for debugging, but | YAML | provisioning.daemons | | Default | 3 | -Number of terraform provisioner daemons to create on start. If builds are stuck in queued state for a long time, consider increasing this. +Number of provisioner daemons to create on start. If builds are stuck in queued state for a long time, consider increasing this. ### --provisioner-daemon-poll-interval From 2a09af35d31e39ffce523f7c2c21db532c8414eb Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 14:33:03 -0500 Subject: [PATCH 06/24] update golden files --- cli/testdata/coder_server_--help.golden | 4 ++-- enterprise/cli/testdata/coder_server_--help.golden | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 69ee62e07c472..6d8f866c11c0b 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -460,8 +460,8 @@ updating, and deleting workspace resources. server. --provisioner-daemons int, $CODER_PROVISIONER_DAEMONS (default: 3) - Number of terraform provisioner daemons to create on start. If builds - are stuck in queued state for a long time, consider increasing this. + Number of provisioner daemons to create on start. If builds are stuck + in queued state for a long time, consider increasing this. TELEMETRY OPTIONS: Telemetry is critical to our ability to improve Coder. We strip all diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 4a873ce9c8779..4d4576d6d57cc 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -461,8 +461,8 @@ updating, and deleting workspace resources. server. --provisioner-daemons int, $CODER_PROVISIONER_DAEMONS (default: 3) - Number of terraform provisioner daemons to create on start. If builds - are stuck in queued state for a long time, consider increasing this. + Number of provisioner daemons to create on start. If builds are stuck + in queued state for a long time, consider increasing this. TELEMETRY OPTIONS: Telemetry is critical to our ability to improve Coder. We strip all From 736df67786a4a3fd05c8e530368e6076b9c083fd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 14:39:50 -0500 Subject: [PATCH 07/24] remove ui pills for provisioners on health page --- site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx index a676786b00558..ebd0c5ff630b5 100644 --- a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx +++ b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx @@ -116,18 +116,6 @@ export const ProvisionerDaemonsPage: FC = () => { gap: 12, }} > - { - // Add pills for the supported provisioenr types. - daemon.provisioners.map((provType) => { - return ( - - }> - {provType} - - - ); - }) - } }> {daemon.api_version} From f104148e96ecf646b48b0318e080156fb43fae2e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 14:42:24 -0500 Subject: [PATCH 08/24] remove unused import --- site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx index ebd0c5ff630b5..63ee9280987c5 100644 --- a/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx +++ b/site/src/pages/HealthPage/ProvisionerDaemonsPage.tsx @@ -4,7 +4,6 @@ import CloseIcon from "@mui/icons-material/Close"; import Person from "@mui/icons-material/Person"; import Sell from "@mui/icons-material/Sell"; import SwapHoriz from "@mui/icons-material/SwapHoriz"; -import ViewInArIcon from "@mui/icons-material/ViewInAr"; import IconButton from "@mui/material/IconButton"; import Tooltip from "@mui/material/Tooltip"; import type { FC } from "react"; From e82b2c34dc8e79b499a21267aebaaeb6c5831c0f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 16:54:09 -0500 Subject: [PATCH 09/24] chore: terraform provisioners enabled in e2e by default Opt out supported for "lite" tests. Environment checking happens to verify terraform + docker exist before running. --- site/e2e/constants.ts | 6 ++++++ site/e2e/helpers.ts | 12 +++++++++++- site/e2e/playwright.config.ts | 25 +++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/site/e2e/constants.ts b/site/e2e/constants.ts index 3e1283e5491c4..69c1d6f163267 100644 --- a/site/e2e/constants.ts +++ b/site/e2e/constants.ts @@ -39,6 +39,12 @@ export const requireEnterpriseTests = Boolean( ); export const enterpriseLicense = process.env.CODER_E2E_ENTERPRISE_LICENSE ?? ""; +// Disabling terraform tests is optional for environments without Docker + Terraform. +// By default, we opt into these tests. +export const requireTerraformTests = !Boolean( + process.env.CODER_E2E_DISABLE_TERRAFORM, +); + // Fake experiments to verify that site presents them as enabled. export const e2eFakeExperiment1 = "e2e-fake-experiment-1"; export const e2eFakeExperiment2 = "e2e-fake-experiment-2"; diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 1c5349fbf5e5b..1c11eac6f031a 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -18,7 +18,7 @@ import { coderPort, enterpriseLicense, prometheusPort, - requireEnterpriseTests, + requireEnterpriseTests, requireTerraformTests, } from "./constants"; import { expectUrl } from "./expectUrl"; import { @@ -43,6 +43,16 @@ export function requiresEnterpriseLicense() { test.skip(!enterpriseLicense); } +// requiresTerraform by default is enabled. +export function requiresTerraform() { + if (requireTerraformTests) { + return; + } + + test.skip(!requireTerraformTests); +} + + // createWorkspace creates a workspace for a template. // It does not wait for it to be running, but it does navigate to the page. export const createWorkspace = async ( diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 0e10c1ff34b0a..e06e9aa5684fe 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -6,14 +6,34 @@ import { coderdPProfPort, e2eFakeExperiment1, e2eFakeExperiment2, - gitAuth, + gitAuth, requireTerraformTests, } from "./constants"; +import {execSync} from "child_process"; +import {requiresTerraform} from "./helpers"; export const wsEndpoint = process.env.CODER_E2E_WS_ENDPOINT; // This is where auth cookies are stored! export const storageState = path.join(__dirname, ".auth.json"); +if(requireTerraformTests) { + try { + // If running terraform tests, verify the requirements exist in the + // environment. + // + // These execs will throw an error if the status code is non-zero. + // So if both these work, then we can launch terraform provisioners. + const terraformExec = execSync('terraform --version') + const dockerExec = execSync('dockser --version') + } catch(e) { + throw new Error("Terraform provisioners require docker & terraform. " + + "At least one of these is not present in the runtime environment. To check yourself:\n" + + "\tterraform --version\n"+ + "\tdocker --version") + } +} + + const localURL = (port: number, path: string): string => { return `http://localhost:${port}${path}`; }; @@ -60,7 +80,8 @@ export default defineConfig({ "--dangerous-disable-rate-limits", "--provisioner-daemons 10", // TODO: Enable some terraform provisioners - "--provisioner-types=echo", + `--provisioner-types=echo${requireTerraformTests ? ",terraform": ""}`, + `--provisioner-daemons=10`, "--web-terminal-renderer=dom", "--pprof-enable", ] From de3f81d9a937a96df5cd65dc1e631a97b36d75b7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 17:21:01 -0500 Subject: [PATCH 10/24] fixup config --- site/e2e/playwright.config.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index e06e9aa5684fe..ea5ccebec85e2 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -24,12 +24,12 @@ if(requireTerraformTests) { // These execs will throw an error if the status code is non-zero. // So if both these work, then we can launch terraform provisioners. const terraformExec = execSync('terraform --version') - const dockerExec = execSync('dockser --version') + const dockerExec = execSync('docker --version') } catch(e) { throw new Error("Terraform provisioners require docker & terraform. " + "At least one of these is not present in the runtime environment. To check yourself:\n" + - "\tterraform --version\n"+ - "\tdocker --version") + "\t$ terraform --version\n"+ + "\t$ docker --version") } } From 8fdaf2c44c70e7b6808c36ed1a48228dcaa66569 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 18:32:48 -0500 Subject: [PATCH 11/24] begin work on terraform e2e test --- .vscode/settings.json | 4 ++- site/e2e/constants.ts | 4 +-- site/e2e/helpers.ts | 41 ++++++++++++++++++++------ site/e2e/tests/createWorkspace.spec.ts | 17 +++++++++++ 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 1f47d7ecf99d4..c95554245cab5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -222,5 +222,7 @@ "go.testFlags": ["-short", "-coverpkg=./..."], // We often use a version of TypeScript that's ahead of the version shipped // with VS Code. - "typescript.tsdk": "./site/node_modules/typescript/lib" + "typescript.tsdk": "./site/node_modules/typescript/lib", + // Playwright tests in VSCode will open a browser to live "view" the test. + "playwright.reuseBrowser": true } diff --git a/site/e2e/constants.ts b/site/e2e/constants.ts index 69c1d6f163267..850df331a6adb 100644 --- a/site/e2e/constants.ts +++ b/site/e2e/constants.ts @@ -41,9 +41,7 @@ export const enterpriseLicense = process.env.CODER_E2E_ENTERPRISE_LICENSE ?? ""; // Disabling terraform tests is optional for environments without Docker + Terraform. // By default, we opt into these tests. -export const requireTerraformTests = !Boolean( - process.env.CODER_E2E_DISABLE_TERRAFORM, -); +export const requireTerraformTests = !process.env.CODER_E2E_DISABLE_TERRAFORM; // Fake experiments to verify that site presents them as enabled. export const e2eFakeExperiment1 = "e2e-fake-experiment-1"; diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 1c11eac6f031a..385e36666aa1a 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -18,7 +18,8 @@ import { coderPort, enterpriseLicense, prometheusPort, - requireEnterpriseTests, requireTerraformTests, + requireEnterpriseTests, + requireTerraformTests, } from "./constants"; import { expectUrl } from "./expectUrl"; import { @@ -52,7 +53,6 @@ export function requiresTerraform() { test.skip(!requireTerraformTests); } - // createWorkspace creates a workspace for a template. // It does not wait for it to be running, but it does navigate to the page. export const createWorkspace = async ( @@ -159,25 +159,48 @@ export const verifyParameters = async ( } }; +// StarterTemplates are ids of starter templates that can be used in place of +// the responses payload. These starter templates will require real provisioners. +export enum StarterTemplates { + STARTER_DOCKER = "docker", +} + +function isStarterTemplate( + input: EchoProvisionerResponses | StarterTemplates | undefined, +): input is StarterTemplates { + if (!input) { + return false; + } + return typeof input === "string"; +} + // createTemplate navigates to the /templates/new page and uploads a template // with the resources provided in the responses argument. export const createTemplate = async ( page: Page, - responses?: EchoProvisionerResponses, + responses?: EchoProvisionerResponses | StarterTemplates, ): Promise => { // Required to have templates submit their provisioner type as echo! await page.addInitScript({ content: "window.playwright = true", }); - await page.goto("/templates/new", { waitUntil: "domcontentloaded" }); + let path = "/templates/new"; + if (isStarterTemplate(responses)) { + path += `?exampleId=${responses}`; + } + + await page.goto(path, { waitUntil: "domcontentloaded" }); await expectUrl(page).toHavePathName("/templates/new"); - await page.getByTestId("file-upload").setInputFiles({ - buffer: await createTemplateVersionTar(responses), - mimeType: "application/x-tar", - name: "template.tar", - }); + if (!isStarterTemplate(responses)) { + await page.getByTestId("file-upload").setInputFiles({ + buffer: await createTemplateVersionTar(responses), + mimeType: "application/x-tar", + name: "template.tar", + }); + } + const name = randomName(); await page.getByLabel("Name *").fill(name); await page.getByTestId("form-submit").click(); diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index 1fa770c5d3614..e0c3f127f409e 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -1,8 +1,10 @@ import { test, expect } from "@playwright/test"; import { + StarterTemplates, createTemplate, createWorkspace, echoResponsesWithParameters, + requiresTerraform, verifyParameters, } from "../helpers"; import { beforeCoderTest } from "../hooks"; @@ -147,3 +149,18 @@ test("create workspace with disable_param search params", async ({ page }) => { await expect(page.getByLabel(/First parameter/i)).toBeDisabled(); await expect(page.getByLabel(/Second parameter/i)).toBeDisabled(); }); + +test("docker based workspace", async ({ page }) => { + requiresTerraform(); + const templateName = await createTemplate( + page, + StarterTemplates.STARTER_DOCKER, + ); + + await page.goto(`/templates/${templateName}/workspace`, { + waitUntil: "domcontentloaded", + }); + + await expect(page.getByLabel(/First parameter/i)).toBeDisabled(); + await expect(page.getByLabel(/Second parameter/i)).toBeDisabled(); +}); From f4d7c0d11e680909a2ac9057628607b445e3f8a3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 2 May 2024 18:42:57 -0500 Subject: [PATCH 12/24] update import order --- site/e2e/playwright.config.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index ea5ccebec85e2..7b16f486d7898 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -1,4 +1,5 @@ import { defineConfig } from "@playwright/test"; +import { execSync } from "child_process"; import * as path from "path"; import { coderMain, @@ -6,34 +7,34 @@ import { coderdPProfPort, e2eFakeExperiment1, e2eFakeExperiment2, - gitAuth, requireTerraformTests, + gitAuth, + requireTerraformTests, } from "./constants"; -import {execSync} from "child_process"; -import {requiresTerraform} from "./helpers"; export const wsEndpoint = process.env.CODER_E2E_WS_ENDPOINT; // This is where auth cookies are stored! export const storageState = path.join(__dirname, ".auth.json"); -if(requireTerraformTests) { +if (requireTerraformTests) { try { // If running terraform tests, verify the requirements exist in the // environment. // // These execs will throw an error if the status code is non-zero. // So if both these work, then we can launch terraform provisioners. - const terraformExec = execSync('terraform --version') - const dockerExec = execSync('docker --version') - } catch(e) { - throw new Error("Terraform provisioners require docker & terraform. " + - "At least one of these is not present in the runtime environment. To check yourself:\n" + - "\t$ terraform --version\n"+ - "\t$ docker --version") + execSync("terraform --version"); + execSync("docker --version"); + } catch (e) { + throw new Error( + "Terraform provisioners require docker & terraform. " + + "At least one of these is not present in the runtime environment. To check yourself:\n" + + "\t$ terraform --version\n" + + "\t$ docker --version", + ); } } - const localURL = (port: number, path: string): string => { return `http://localhost:${port}${path}`; }; @@ -80,7 +81,7 @@ export default defineConfig({ "--dangerous-disable-rate-limits", "--provisioner-daemons 10", // TODO: Enable some terraform provisioners - `--provisioner-types=echo${requireTerraformTests ? ",terraform": ""}`, + `--provisioner-types=echo${requireTerraformTests ? ",terraform" : ""}`, `--provisioner-daemons=10`, "--web-terminal-renderer=dom", "--pprof-enable", From 50501e88d1156f893dba0587ed7b84da8bf5a6a1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 12:10:39 -0500 Subject: [PATCH 13/24] create e2e terraform test with docker template --- site/e2e/helpers.ts | 5 ++- site/e2e/playwright.config.ts | 2 +- site/e2e/tests/createWorkspace.spec.ts | 41 +++++++++++++++++----- site/src/pages/CreateTemplatePage/utils.ts | 4 +-- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 385e36666aa1a..e7fc8b3901cb7 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -182,7 +182,10 @@ export const createTemplate = async ( ): Promise => { // Required to have templates submit their provisioner type as echo! await page.addInitScript({ - content: "window.playwright = true", + content: `window.playwrightProvisionerType = ${ + // Starter templates use the terraform type. + isStarterTemplate(responses) ? "terraform" : "echo" + }`, }); let path = "/templates/new"; diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 7b16f486d7898..a50b821697581 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -75,7 +75,7 @@ export default defineConfig({ `go run -tags embed ${coderMain} server`, "--global-config $(mktemp -d -t e2e-XXXXXXXXXX)", `--access-url=http://localhost:${coderPort}`, - `--http-address=localhost:${coderPort}`, + `--http-address=0.0.0.0:${coderPort}`, "--in-memory", "--telemetry=false", "--dangerous-disable-rate-limits", diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index e0c3f127f409e..15416058f4a05 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -150,17 +150,42 @@ test("create workspace with disable_param search params", async ({ page }) => { await expect(page.getByLabel(/Second parameter/i)).toBeDisabled(); }); -test("docker based workspace", async ({ page }) => { +test("create docker workspace", async ({ page }) => { + test.skip( + true, + "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", + ); requiresTerraform(); - const templateName = await createTemplate( - page, - StarterTemplates.STARTER_DOCKER, + const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER); + + const _ = await createWorkspace(page, template); + + // The workspace agents must be ready before we try to interact with the workspace. + await page.waitForSelector( + `//div[@role="status"][@data-testid="agent-status-ready"]`, + { + state: "visible", + }, ); - await page.goto(`/templates/${templateName}/workspace`, { - waitUntil: "domcontentloaded", + // Wait for the terminal button to be visible, and click it. + const terminalButton = + "//a[@data-testid='terminal'][normalize-space()='Terminal']"; + await page.waitForSelector(terminalButton, { + state: "visible", }); - await expect(page.getByLabel(/First parameter/i)).toBeDisabled(); - await expect(page.getByLabel(/Second parameter/i)).toBeDisabled(); + // We can't click the terminal button because that opens a new tab. + // So grab the href, and manually navigate. + const terminalPageURL = await page.getAttribute(terminalButton, "href"); + expect(terminalPageURL).not.toBeNull(); + await page.goto(terminalPageURL!, { + waitUntil: "domcontentloaded", + }); + await page.waitForSelector( + `//textarea[contains(@class,"xterm-helper-textarea")]`, + { + state: "visible", + }, + ); }); diff --git a/site/src/pages/CreateTemplatePage/utils.ts b/site/src/pages/CreateTemplatePage/utils.ts index cc7266d1de664..3e35b331b1515 100644 --- a/site/src/pages/CreateTemplatePage/utils.ts +++ b/site/src/pages/CreateTemplatePage/utils.ts @@ -8,8 +8,8 @@ import { calculateAutostopRequirementDaysValue } from "utils/schedule"; import type { CreateTemplateData } from "./CreateTemplateForm"; const provisioner: ProvisionerType = - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Playwright needs to use a different provisioner type! - typeof (window as any).playwright !== "undefined" ? "echo" : "terraform"; + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Playwright might need to use a different provisioner type! + (window as any).playwrightProvisionerType || "terraform"; export const newTemplate = (formData: CreateTemplateData) => { const { autostop_requirement_days_of_week, autostop_requirement_weeks } = From c25f6c415ec319643c18bf14afa2b1da8ff234e0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 12:11:38 -0500 Subject: [PATCH 14/24] Run docker e2e in CI once --- site/e2e/tests/createWorkspace.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index 15416058f4a05..26d103f7fc0fb 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -152,7 +152,7 @@ test("create workspace with disable_param search params", async ({ page }) => { test("create docker workspace", async ({ page }) => { test.skip( - true, + false, "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", ); requiresTerraform(); From 604227e46d8dc724cbc422cbdc107e5f638791c1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 12:18:28 -0500 Subject: [PATCH 15/24] disable leaky test --- site/e2e/tests/createWorkspace.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index 26d103f7fc0fb..15416058f4a05 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -152,7 +152,7 @@ test("create workspace with disable_param search params", async ({ page }) => { test("create docker workspace", async ({ page }) => { test.skip( - false, + true, "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", ); requiresTerraform(); From f5e09cd1309fa702d165b933e5dd741d2c8a3e4a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 17:26:10 -0500 Subject: [PATCH 16/24] fixup hidden input field --- site/e2e/helpers.ts | 17 +- .../CreateTemplatePage/CreateTemplateForm.tsx | 265 ++++++++++-------- .../DuplicateTemplateView.tsx | 1 + .../CreateTemplatePage/UploadTemplateView.tsx | 1 + site/src/pages/CreateTemplatePage/utils.ts | 15 +- 5 files changed, 167 insertions(+), 132 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index e7fc8b3901cb7..fa5bba132d8a9 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -180,14 +180,6 @@ export const createTemplate = async ( page: Page, responses?: EchoProvisionerResponses | StarterTemplates, ): Promise => { - // Required to have templates submit their provisioner type as echo! - await page.addInitScript({ - content: `window.playwrightProvisionerType = ${ - // Starter templates use the terraform type. - isStarterTemplate(responses) ? "terraform" : "echo" - }`, - }); - let path = "/templates/new"; if (isStarterTemplate(responses)) { path += `?exampleId=${responses}`; @@ -197,6 +189,15 @@ export const createTemplate = async ( await expectUrl(page).toHavePathName("/templates/new"); if (!isStarterTemplate(responses)) { + await page + .locator(`xpath=//input[@data-testid="provisioner-type-input"]`) + .evaluate((el: HTMLElement) => { + // This is a little jank, but the "setAttribute" updates the HTML, but not the formik values. + el.setAttribute("value", "echo"); + // This '.click()' activates the onClick handler that tells the input to update it's formik value. + el.click(); + }); + await page.getByTestId("file-upload").setInputFiles({ buffer: await createTemplateVersionTar(responses), mimeType: "application/x-tar", diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 0ba63ce7de2c1..a299ed0193bcb 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -1,11 +1,12 @@ import TextField from "@mui/material/TextField"; -import { useFormik } from "formik"; +import { Field, FormikProvider, useFormik } from "formik"; import camelCase from "lodash/camelCase"; import capitalize from "lodash/capitalize"; import type { FC } from "react"; import * as Yup from "yup"; import type { ProvisionerJobLog, + ProvisionerType, Template, TemplateExample, TemplateVersionVariable, @@ -50,6 +51,7 @@ export interface CreateTemplateData { parameter_values_by_name?: Record; user_variable_values?: VariableValue[]; allow_everyone_group_access: boolean; + provisioner_type: ProvisionerType; } const validationSchema = Yup.object({ @@ -81,6 +83,7 @@ const defaultInitialValues: CreateTemplateData = { allow_user_autostart: false, allow_user_autostop: false, allow_everyone_group_access: true, + provisioner_type: "terraform", }; type GetInitialValuesParams = { @@ -190,133 +193,163 @@ export const CreateTemplateForm: FC = (props) => { const getFieldHelpers = getFormHelpers(form, error); return ( - - {/* General info */} - - - {"starterTemplate" in props && ( - - )} - {"copiedTemplate" in props && ( - - )} - {"upload" in props && ( - { - await fillNameAndDisplayWithFilename(file.name, form); - props.upload.onUpload(file); - }} - /> - )} - - - - + + + {/* General info */} + + + {"starterTemplate" in props && ( + + )} + {"copiedTemplate" in props && ( + + )} + {"upload" in props && ( + { + await fillNameAndDisplayWithFilename(file.name, form); + props.upload.onUpload(file); + }} + /> + )} - {/* Display info */} - - - - + + + - {/* Variables */} - {variables && variables.length > 0 && ( + {/* Display info */} - {variables.map((variable, index) => ( - { - await form.setFieldValue("user_variable_values." + index, { - name: variable.name, - value, - }); - }} - /> - ))} + + + + + form.setFieldValue("icon", value)} + /> - )} -
- ({ - backgroundColor: "transparent", - border: 0, - fontWeight: 500, - fontSize: 14, - cursor: "pointer", - color: theme.palette.text.secondary, + {/* Variables */} + {variables && variables.length > 0 && ( + + + {variables.map((variable, index) => ( + { + await form.setFieldValue("user_variable_values." + index, { + name: variable.name, + value, + }); + }} + /> + ))} + + + )} + +
+ ({ + backgroundColor: "transparent", + border: 0, + fontWeight: 500, + fontSize: 14, + cursor: "pointer", + color: theme.palette.text.secondary, - "&:hover": { - textDecoration: "underline", - textUnderlineOffset: 4, - color: theme.palette.text.primary, - }, - })} - > - Show build logs - - ) - } - onCancel={onCancel} - isLoading={isSubmitting} - submitLabel={jobError ? "Retry" : "Create template"} - /> -
- + "&:hover": { + textDecoration: "underline", + textUnderlineOffset: 4, + color: theme.palette.text.primary, + }, + })} + > + Show build logs + + ) + } + onCancel={onCancel} + isLoading={isSubmitting} + submitLabel={jobError ? "Retry" : "Create template"} + /> +
+ + ); }; diff --git a/site/src/pages/CreateTemplatePage/DuplicateTemplateView.tsx b/site/src/pages/CreateTemplatePage/DuplicateTemplateView.tsx index fd87e3b586c22..91ac28acc9127 100644 --- a/site/src/pages/CreateTemplatePage/DuplicateTemplateView.tsx +++ b/site/src/pages/CreateTemplatePage/DuplicateTemplateView.tsx @@ -82,6 +82,7 @@ export const DuplicateTemplateView: FC = ({ version: firstVersionFromFile( templateVersionQuery.data!.job.file_id, formData.user_variable_values, + formData.provisioner_type, ), template: newTemplate(formData), }); diff --git a/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx b/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx index c6cc5fccac8e3..ac650baff112b 100644 --- a/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx +++ b/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx @@ -64,6 +64,7 @@ export const UploadTemplateView: FC = ({ version: firstVersionFromFile( uploadedFile!.hash, formData.user_variable_values, + formData.provisioner_type, ), template: newTemplate(formData), }); diff --git a/site/src/pages/CreateTemplatePage/utils.ts b/site/src/pages/CreateTemplatePage/utils.ts index 3e35b331b1515..a1536b8a4ce5c 100644 --- a/site/src/pages/CreateTemplatePage/utils.ts +++ b/site/src/pages/CreateTemplatePage/utils.ts @@ -1,4 +1,5 @@ import type { + CreateTemplateVersionRequest, Entitlements, ProvisionerType, TemplateExample, @@ -7,10 +8,6 @@ import type { import { calculateAutostopRequirementDaysValue } from "utils/schedule"; import type { CreateTemplateData } from "./CreateTemplateForm"; -const provisioner: ProvisionerType = - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Playwright might need to use a different provisioner type! - (window as any).playwrightProvisionerType || "terraform"; - export const newTemplate = (formData: CreateTemplateData) => { const { autostop_requirement_days_of_week, autostop_requirement_weeks } = formData; @@ -56,10 +53,11 @@ export const getFormPermissions = (entitlements: Entitlements) => { export const firstVersionFromFile = ( fileId: string, variables: VariableValue[] | undefined, -) => { + provisionerType: ProvisionerType, +): CreateTemplateVersionRequest => { return { storage_method: "file" as const, - provisioner: provisioner, + provisioner: provisionerType, user_variable_values: variables, file_id: fileId, tags: {}, @@ -69,10 +67,11 @@ export const firstVersionFromFile = ( export const firstVersionFromExample = ( example: TemplateExample, variables: VariableValue[] | undefined, -) => { +): CreateTemplateVersionRequest => { return { storage_method: "file" as const, - provisioner: provisioner, + // All starter templates are for the terraform provisioner type. + provisioner: "terraform", user_variable_values: variables, example_id: example.id, tags: {}, From 38623111c8e9ce8378573b1b94b807d71820b8dc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 17:41:43 -0500 Subject: [PATCH 17/24] remove comments --- site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index a299ed0193bcb..18271ac64c65b 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -222,15 +222,6 @@ export const CreateTemplateForm: FC = (props) => { For testing purposes, we expose this as a hidden form element that can be changed. For example, to "echo" */} - {/* */} - Date: Sun, 5 May 2024 10:51:31 -0500 Subject: [PATCH 18/24] switch to query params --- site/e2e/helpers.ts | 12 ++------ .../CreateTemplatePage/CreateTemplateForm.tsx | 30 +++++++------------ 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index fa5bba132d8a9..6481275a5adb6 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -183,21 +183,15 @@ export const createTemplate = async ( let path = "/templates/new"; if (isStarterTemplate(responses)) { path += `?exampleId=${responses}`; + } else { + // The form page will read this value and use it as the default type. + path += "?provisioner_type=echo"; } await page.goto(path, { waitUntil: "domcontentloaded" }); await expectUrl(page).toHavePathName("/templates/new"); if (!isStarterTemplate(responses)) { - await page - .locator(`xpath=//input[@data-testid="provisioner-type-input"]`) - .evaluate((el: HTMLElement) => { - // This is a little jank, but the "setAttribute" updates the HTML, but not the formik values. - el.setAttribute("value", "echo"); - // This '.click()' activates the onClick handler that tells the input to update it's formik value. - el.click(); - }); - await page.getByTestId("file-upload").setInputFiles({ buffer: await createTemplateVersionTar(responses), mimeType: "application/x-tar", diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 18271ac64c65b..bef17decbc4e4 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -33,6 +33,7 @@ import { } from "utils/schedule"; import { TemplateUpload, type TemplateUploadProps } from "./TemplateUpload"; import { VariableInput } from "./VariableInput"; +import { useSearchParams } from "react-router-dom"; const MAX_DESCRIPTION_CHAR_LIMIT = 128; @@ -91,6 +92,7 @@ type GetInitialValuesParams = { fromCopy?: Template; variables?: TemplateVersionVariable[]; allowAdvancedScheduling: boolean; + searchParams: URLSearchParams; }; const getInitialValues = ({ @@ -98,9 +100,15 @@ const getInitialValues = ({ fromCopy, allowAdvancedScheduling, variables, + searchParams, }: GetInitialValuesParams) => { let initialValues = defaultInitialValues; + // Will assume the query param has a valid ProvisionerType, as this query param is only used + // in testing. + defaultInitialValues.provisioner_type = + (searchParams.get("provisioner_type") as ProvisionerType) || "terraform"; + if (!allowAdvancedScheduling) { initialValues = { ...initialValues, @@ -167,6 +175,7 @@ export type CreateTemplateFormProps = ( }; export const CreateTemplateForm: FC = (props) => { + const [searchParams, _] = useSearchParams(); const { onCancel, onSubmit, @@ -179,6 +188,7 @@ export const CreateTemplateForm: FC = (props) => { allowAdvancedScheduling, variablesSectionRef, } = props; + const form = useFormik({ initialValues: getInitialValues({ allowAdvancedScheduling, @@ -186,6 +196,7 @@ export const CreateTemplateForm: FC = (props) => { "starterTemplate" in props ? props.starterTemplate : undefined, fromCopy: "copiedTemplate" in props ? props.copiedTemplate : undefined, variables, + searchParams, }), validationSchema, onSubmit, @@ -217,25 +228,6 @@ export const CreateTemplateForm: FC = (props) => { /> )} - {/* - This value is always "terraform" in production. - For testing purposes, we expose this as a hidden form element - that can be changed. For example, to "echo" - */} - { - await form.setFieldValue("provisioner_type", e.target.value); - }} - /> - Date: Sun, 5 May 2024 11:07:48 -0500 Subject: [PATCH 19/24] imports --- site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index bef17decbc4e4..0002679694802 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -1,8 +1,9 @@ import TextField from "@mui/material/TextField"; -import { Field, FormikProvider, useFormik } from "formik"; +import { FormikProvider, useFormik } from "formik"; import camelCase from "lodash/camelCase"; import capitalize from "lodash/capitalize"; import type { FC } from "react"; +import { useSearchParams } from "react-router-dom"; import * as Yup from "yup"; import type { ProvisionerJobLog, @@ -33,7 +34,6 @@ import { } from "utils/schedule"; import { TemplateUpload, type TemplateUploadProps } from "./TemplateUpload"; import { VariableInput } from "./VariableInput"; -import { useSearchParams } from "react-router-dom"; const MAX_DESCRIPTION_CHAR_LIMIT = 128; From d9a8dc7c3229c3ba0fa015acea592aff5d1705b6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Sun, 5 May 2024 11:11:08 -0500 Subject: [PATCH 20/24] remove diff --- .../CreateTemplatePage/CreateTemplateForm.tsx | 240 +++++++++--------- 1 file changed, 119 insertions(+), 121 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 0002679694802..3ab1d0730d2eb 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -1,5 +1,5 @@ import TextField from "@mui/material/TextField"; -import { FormikProvider, useFormik } from "formik"; +import { useFormik } from "formik"; import camelCase from "lodash/camelCase"; import capitalize from "lodash/capitalize"; import type { FC } from "react"; @@ -204,135 +204,133 @@ export const CreateTemplateForm: FC = (props) => { const getFieldHelpers = getFormHelpers(form, error); return ( - - - {/* General info */} - - - {"starterTemplate" in props && ( - - )} - {"copiedTemplate" in props && ( - - )} - {"upload" in props && ( - { - await fillNameAndDisplayWithFilename(file.name, form); - props.upload.onUpload(file); - }} - /> - )} - - + {/* General info */} + + + {"starterTemplate" in props && ( + + )} + {"copiedTemplate" in props && ( + + )} + {"upload" in props && ( + { + await fillNameAndDisplayWithFilename(file.name, form); + props.upload.onUpload(file); + }} /> - - + )} + + + + - {/* Display info */} + {/* Display info */} + + + + + + + form.setFieldValue("icon", value)} + /> + + + + {/* Variables */} + {variables && variables.length > 0 && ( - - - - - form.setFieldValue("icon", value)} - /> + {variables.map((variable, index) => ( + { + await form.setFieldValue("user_variable_values." + index, { + name: variable.name, + value, + }); + }} + /> + ))} + )} - {/* Variables */} - {variables && variables.length > 0 && ( - - - {variables.map((variable, index) => ( - { - await form.setFieldValue("user_variable_values." + index, { - name: variable.name, - value, - }); - }} - /> - ))} - - - )} - -
- ({ - backgroundColor: "transparent", - border: 0, - fontWeight: 500, - fontSize: 14, - cursor: "pointer", - color: theme.palette.text.secondary, +
+ ({ + backgroundColor: "transparent", + border: 0, + fontWeight: 500, + fontSize: 14, + cursor: "pointer", + color: theme.palette.text.secondary, - "&:hover": { - textDecoration: "underline", - textUnderlineOffset: 4, - color: theme.palette.text.primary, - }, - })} - > - Show build logs - - ) - } - onCancel={onCancel} - isLoading={isSubmitting} - submitLabel={jobError ? "Retry" : "Create template"} - /> -
- - + "&:hover": { + textDecoration: "underline", + textUnderlineOffset: 4, + color: theme.palette.text.primary, + }, + })} + > + Show build logs + + ) + } + onCancel={onCancel} + isLoading={isSubmitting} + submitLabel={jobError ? "Retry" : "Create template"} + /> +
+ ); }; From e34546fdacb1106178d49eee8aed15fff4ff11ff Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 7 May 2024 09:31:08 -0500 Subject: [PATCH 21/24] PR suggestions, using web terminal helper --- site/e2e/helpers.ts | 3 +- site/e2e/playwright.config.ts | 2 +- site/e2e/tests/createWorkspace.spec.ts | 28 +++++++++---------- .../CreateTemplatePage/CreateTemplateForm.tsx | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 6481275a5adb6..9d2f1df5551a4 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -899,6 +899,7 @@ export async function openTerminalWindow( page: Page, context: BrowserContext, workspaceName: string, + agentName: string = "dev", ): Promise { // Wait for the web terminal to open in a new tab const pagePromise = context.waitForEvent("page"); @@ -910,7 +911,7 @@ export async function openTerminalWindow( // isn't POSIX compatible, such as Fish. const commandQuery = `?command=${encodeURIComponent("/usr/bin/env bash")}`; await expectUrl(terminal).toHavePathName( - `/@admin/${workspaceName}.dev/terminal`, + `/@admin/${workspaceName}.${agentName}/terminal`, ); await terminal.goto(`/@admin/${workspaceName}.dev/terminal${commandQuery}`); diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index a50b821697581..d8093c085d3fa 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -25,7 +25,7 @@ if (requireTerraformTests) { // So if both these work, then we can launch terraform provisioners. execSync("terraform --version"); execSync("docker --version"); - } catch (e) { + } catch { throw new Error( "Terraform provisioners require docker & terraform. " + "At least one of these is not present in the runtime environment. To check yourself:\n" + diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index 15416058f4a05..d078665707b51 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -4,6 +4,7 @@ import { createTemplate, createWorkspace, echoResponsesWithParameters, + openTerminalWindow, requiresTerraform, verifyParameters, } from "../helpers"; @@ -150,15 +151,15 @@ test("create workspace with disable_param search params", async ({ page }) => { await expect(page.getByLabel(/Second parameter/i)).toBeDisabled(); }); -test("create docker workspace", async ({ page }) => { - test.skip( - true, - "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", - ); +test("create docker workspace", async ({ context, page }) => { + // test.skip( + // true, + // "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", + // ); requiresTerraform(); const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER); - const _ = await createWorkspace(page, template); + const workspaceName = await createWorkspace(page, template); // The workspace agents must be ready before we try to interact with the workspace. await page.waitForSelector( @@ -175,14 +176,13 @@ test("create docker workspace", async ({ page }) => { state: "visible", }); - // We can't click the terminal button because that opens a new tab. - // So grab the href, and manually navigate. - const terminalPageURL = await page.getAttribute(terminalButton, "href"); - expect(terminalPageURL).not.toBeNull(); - await page.goto(terminalPageURL!, { - waitUntil: "domcontentloaded", - }); - await page.waitForSelector( + const terminal = await openTerminalWindow( + page, + context, + workspaceName, + "main", + ); + await terminal.waitForSelector( `//textarea[contains(@class,"xterm-helper-textarea")]`, { state: "visible", diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 3ab1d0730d2eb..8370be000e9c1 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -175,7 +175,7 @@ export type CreateTemplateFormProps = ( }; export const CreateTemplateForm: FC = (props) => { - const [searchParams, _] = useSearchParams(); + const [searchParams] = useSearchParams(); const { onCancel, onSubmit, From ab2c29ff567331d86c28c6b82a7d13f04d05283d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 7 May 2024 09:38:52 -0500 Subject: [PATCH 22/24] PR feedback on executable errors --- site/e2e/helpers.ts | 4 --- site/e2e/playwright.config.ts | 48 ++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 9d2f1df5551a4..8ec43bcf43145 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -46,10 +46,6 @@ export function requiresEnterpriseLicense() { // requiresTerraform by default is enabled. export function requiresTerraform() { - if (requireTerraformTests) { - return; - } - test.skip(!requireTerraformTests); } diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index d8093c085d3fa..889976fe4615b 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -16,23 +16,37 @@ export const wsEndpoint = process.env.CODER_E2E_WS_ENDPOINT; // This is where auth cookies are stored! export const storageState = path.join(__dirname, ".auth.json"); -if (requireTerraformTests) { - try { - // If running terraform tests, verify the requirements exist in the - // environment. - // - // These execs will throw an error if the status code is non-zero. - // So if both these work, then we can launch terraform provisioners. - execSync("terraform --version"); - execSync("docker --version"); - } catch { - throw new Error( - "Terraform provisioners require docker & terraform. " + - "At least one of these is not present in the runtime environment. To check yourself:\n" + - "\t$ terraform --version\n" + - "\t$ docker --version", - ); - } +// If running terraform tests, verify the requirements exist in the +// environment. +// +// These execs will throw an error if the status code is non-zero. +// So if both these work, then we can launch terraform provisioners. +let hasTerraform = false; +let hasDocker = false; +try { + execSync("terraform --version"); + hasTerraform = true; +} catch { + /* empty */ +} + +try { + execSync("docker --version"); + hasDocker = true; +} catch { + /* empty */ +} + +if (!hasTerraform || !hasDocker) { + const msg = + "Terraform provisioners require docker & terraform binaries to function. \n" + + (hasTerraform + ? "" + : "\tThe `terraform` executable is not present in the runtime environment.\n") + + (hasDocker + ? "" + : "\tThe `docker` executable is not present in the runtime environment.\n"); + throw new Error(msg); } const localURL = (port: number, path: string): string => { From c521532dfe61f1f3b85e33f40b86dc841dbef63b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 8 May 2024 08:30:30 -0500 Subject: [PATCH 23/24] rename require --- site/e2e/helpers.ts | 4 ++-- site/e2e/tests/createWorkspace.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 8ec43bcf43145..3f58184b1c1ac 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -44,8 +44,8 @@ export function requiresEnterpriseLicense() { test.skip(!enterpriseLicense); } -// requiresTerraform by default is enabled. -export function requiresTerraform() { +// requireTerraformProvisioner by default is enabled. +export function requireTerraformProvisioner() { test.skip(!requireTerraformTests); } diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index d078665707b51..f0ad7cb6cc23a 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -5,7 +5,7 @@ import { createWorkspace, echoResponsesWithParameters, openTerminalWindow, - requiresTerraform, + requireTerraformProvisioner, verifyParameters, } from "../helpers"; import { beforeCoderTest } from "../hooks"; @@ -156,7 +156,7 @@ test("create docker workspace", async ({ context, page }) => { // true, // "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", // ); - requiresTerraform(); + requireTerraformProvisioner(); const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER); const workspaceName = await createWorkspace(page, template); From 2b77082e4ecbfa71390ee73a346c344c67c36c7a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 8 May 2024 13:05:38 -0500 Subject: [PATCH 24/24] skip docker test, it leaks containers --- site/e2e/tests/createWorkspace.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index f0ad7cb6cc23a..5f1713b60aaa7 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -152,10 +152,10 @@ test("create workspace with disable_param search params", async ({ page }) => { }); test("create docker workspace", async ({ context, page }) => { - // test.skip( - // true, - // "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", - // ); + test.skip( + true, + "creating docker containers is currently leaky. They are not cleaned up when the tests are over.", + ); requireTerraformProvisioner(); const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER);