Skip to content

chore: skip global.setup if first user already exists #12930

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 11, 2024
Merged

chore: skip global.setup if first user already exists #12930

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6021d5
chore: skip global.setup if first user already exists
Emyrk Apr 10, 2024
1b721ed
fixup api route url
Emyrk Apr 10, 2024
7c382ab
add ui mode
Emyrk Apr 10, 2024
b9bc1a1
Fix package.json
Emyrk Apr 10, 2024
fb29ca4
remove boolean condition from setupAPICalls
Emyrk Apr 11, 2024
8f2fbc1
make fmt
Emyrk Apr 11, 2024
0d9e335
Update site/e2e/global.setup.ts
Emyrk Apr 11, 2024
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: 8 additions & 0 deletions site/e2e/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,11 @@ Enterprise tests require a license key to run.
```shell
export CODER_E2E_ENTERPRISE_LICENSE=<license key>
```

# Debugging tests

To debug a test, it is more helpful to run it in `ui` mode.

```
pnpm playwright:test-ui
```
8 changes: 5 additions & 3 deletions site/e2e/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import { findSessionToken, randomName } from "./helpers";

let currentOrgId: string;

export const setupApiCalls = async (page: Page) => {
const token = await findSessionToken(page);
API.setSessionToken(token);
export const setupApiCalls = async (page: Page, unauthenticated?: boolean) => {
Copy link
Member

Choose a reason for hiding this comment

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

having a boolean as an optional argument is almost always a bad idea imo, because from the call site setupApiCalls(page, true) is incredibly vague. it should either be on an options object, or ideally imo, it would just gracefully do nothing if there is no token, and not take any additional params.

try {
  const token = await findSessionToken(page);
  API.setSessionToken(token);
} catch {}

if (!unauthenticated) {
const token = await findSessionToken(page);
API.setSessionToken(token);
}
API.setHost(`http://127.0.0.1:${coderPort}`);
};

Expand Down
9 changes: 9 additions & 0 deletions site/e2e/global.setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
import { expect, test } from "@playwright/test";
import { hasFirstUser } from "api/api";
import { Language } from "pages/CreateUserPage/CreateUserForm";
import { setupApiCalls } from "./api";
import * as constants from "./constants";
import { storageState } from "./playwright.config";

test("setup deployment", async ({ page }) => {
await page.goto("/", { waitUntil: "domcontentloaded" });
await setupApiCalls(page, true);
const exists = await hasFirstUser();
if (exists) {
// First user already exists, abort early. All tests execute this as a dependency,
// if you run multiple tests in the UI, this will fail unless we check this.
return;
}

// Setup first user
await page.getByLabel(Language.usernameLabel).fill(constants.username);
Expand Down
2 changes: 2 additions & 0 deletions site/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
"lint:types": "tsc -p .",
"playwright:install": "playwright install --with-deps chromium",
"playwright:test": "playwright test --config=e2e/playwright.config.ts",
"//": "If we are in a coder workspace, use the `ui-port` to allow opening remotely. If native, use the default app. It's generally a bit smoother to use.",
Copy link
Member

Choose a reason for hiding this comment

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

pls no, I hate these hacky "json comments" 😭

"playwright:test-ui": "playwright test --config=e2e/playwright.config.ts --ui $([[ \"$CODER\" == \"true\" ]] && echo --ui-port=7500 --ui-host=0.0.0.0)",
Copy link
Member Author

Choose a reason for hiding this comment

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

@aslilac This works on local or in coder.

Copy link
Member

Choose a reason for hiding this comment

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

why not just set these flags all the time? they're harmless options and seem like they should always work.

"gen:provisioner": "protoc --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_out=./e2e/ --ts_proto_opt=outputJsonMethods=false,outputEncodeMethods=encode-no-creation,outputClientImpl=false,nestJs=false,outputPartialMethods=false,fileSuffix=Generated,suffix=hey -I ../provisionersdk/proto ../provisionersdk/proto/provisioner.proto && pnpm exec prettier --ignore-path '/dev/null' --cache --write './e2e/provisionerGenerated.ts'",
"storybook": "STORYBOOK=true storybook dev -p 6006",
"storybook:build": "storybook build",
Expand Down