-
Notifications
You must be signed in to change notification settings - Fork 888
chore: enable terraform provisioners in e2e by default #13134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c810759
5294740
ae89373
e12e721
f4dbfb5
2a09af3
736df67
f104148
e82b2c3
de3f81d
8fdaf2c
f4d7c0d
50501e8
c25f6c4
604227e
f5e09cd
3862311
3173a5e
fca5946
d9a8dc7
e34546f
ab2c29f
c521532
2b77082
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,10 @@ 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 = !process.env.CODER_E2E_DISABLE_TERRAFORM; | ||
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to include this option if the Terraform stuff ever gets a bit messy. Right now for example docker containers are leaky. |
||
|
||
// Fake experiments to verify that site presents them as enabled. | ||
export const e2eFakeExperiment1 = "e2e-fake-experiment-1"; | ||
export const e2eFakeExperiment2 = "e2e-fake-experiment-2"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
enterpriseLicense, | ||
prometheusPort, | ||
requireEnterpriseTests, | ||
requireTerraformTests, | ||
} from "./constants"; | ||
import { expectUrl } from "./expectUrl"; | ||
import { | ||
|
@@ -43,6 +44,11 @@ export function requiresEnterpriseLicense() { | |
test.skip(!enterpriseLicense); | ||
} | ||
|
||
// requireTerraformProvisioner by default is enabled. | ||
export function requireTerraformProvisioner() { | ||
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 ( | ||
|
@@ -149,25 +155,46 @@ 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", | ||
} | ||
Comment on lines
+158
to
+162
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add ways to add more to the templates later. For now, just using the starter template is easier. |
||
|
||
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<string> => { | ||
// Required to have templates submit their provisioner type as echo! | ||
await page.addInitScript({ | ||
content: "window.playwright = true", | ||
}); | ||
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("/templates/new", { waitUntil: "domcontentloaded" }); | ||
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(); | ||
|
@@ -868,6 +895,7 @@ export async function openTerminalWindow( | |
page: Page, | ||
context: BrowserContext, | ||
workspaceName: string, | ||
agentName: string = "dev", | ||
): Promise<Page> { | ||
// Wait for the web terminal to open in a new tab | ||
const pagePromise = context.waitForEvent("page"); | ||
|
@@ -879,7 +907,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}`); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
import { test, expect } from "@playwright/test"; | ||
import { | ||
StarterTemplates, | ||
createTemplate, | ||
createWorkspace, | ||
echoResponsesWithParameters, | ||
openTerminalWindow, | ||
requireTerraformProvisioner, | ||
verifyParameters, | ||
} from "../helpers"; | ||
import { beforeCoderTest } from "../hooks"; | ||
|
@@ -147,3 +150,42 @@ 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("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.", | ||
); | ||
Comment on lines
+155
to
+158
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to resolve this in another PR. Terraform provisioners are now working, just need to do some cleanup before I allow this test into the mainstage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tracked here: #13163 |
||
requireTerraformProvisioner(); | ||
const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER); | ||
|
||
const workspaceName = 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", | ||
}, | ||
); | ||
|
||
// 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", | ||
}); | ||
|
||
const terminal = await openTerminalWindow( | ||
page, | ||
context, | ||
workspaceName, | ||
"main", | ||
); | ||
await terminal.waitForSelector( | ||
`//textarea[contains(@class,"xterm-helper-textarea")]`, | ||
{ | ||
state: "visible", | ||
}, | ||
); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,11 @@ import { 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, | ||
ProvisionerType, | ||
Template, | ||
TemplateExample, | ||
TemplateVersionVariable, | ||
|
@@ -50,6 +52,7 @@ export interface CreateTemplateData { | |
parameter_values_by_name?: Record<string, string>; | ||
user_variable_values?: VariableValue[]; | ||
allow_everyone_group_access: boolean; | ||
provisioner_type: ProvisionerType; | ||
} | ||
|
||
const validationSchema = Yup.object({ | ||
|
@@ -81,23 +84,31 @@ const defaultInitialValues: CreateTemplateData = { | |
allow_user_autostart: false, | ||
allow_user_autostop: false, | ||
allow_everyone_group_access: true, | ||
provisioner_type: "terraform", | ||
}; | ||
|
||
type GetInitialValuesParams = { | ||
fromExample?: TemplateExample; | ||
fromCopy?: Template; | ||
variables?: TemplateVersionVariable[]; | ||
allowAdvancedScheduling: boolean; | ||
searchParams: URLSearchParams; | ||
}; | ||
|
||
const getInitialValues = ({ | ||
fromExample, | ||
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"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd kind of rather leave this as a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is correct. The type of the |
||
|
||
if (!allowAdvancedScheduling) { | ||
initialValues = { | ||
...initialValues, | ||
|
@@ -164,6 +175,7 @@ export type CreateTemplateFormProps = ( | |
}; | ||
|
||
export const CreateTemplateForm: FC<CreateTemplateFormProps> = (props) => { | ||
const [searchParams] = useSearchParams(); | ||
const { | ||
onCancel, | ||
onSubmit, | ||
|
@@ -176,13 +188,15 @@ export const CreateTemplateForm: FC<CreateTemplateFormProps> = (props) => { | |
allowAdvancedScheduling, | ||
variablesSectionRef, | ||
} = props; | ||
|
||
const form = useFormik<CreateTemplateData>({ | ||
initialValues: getInitialValues({ | ||
allowAdvancedScheduling, | ||
fromExample: | ||
"starterTemplate" in props ? props.starterTemplate : undefined, | ||
fromCopy: "copiedTemplate" in props ? props.copiedTemplate : undefined, | ||
variables, | ||
searchParams, | ||
}), | ||
validationSchema, | ||
onSubmit, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required to run a "live" debugger in VSCode with the chrome window open the whole time. Otherwise the window keeps opening and closing for each snapshot step.