-
Notifications
You must be signed in to change notification settings - Fork 16
Add e2e tests for login, survey and dashboard #681
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
base: staging
Are you sure you want to change the base?
Changes from 1 commit
6861c6a
bbfc98d
3c7eb78
4ae903e
1dce813
4b44e73
9771064
5852aaa
fa216ee
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 |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { test, expect } from "@playwright/test"; | ||
| import { login } from "./utils"; | ||
|
|
||
| test.describe("Dashboard", () => { | ||
| test("Check dashboard and Navbar", async ({ page }) => { | ||
| await login(page); | ||
|
|
||
| const overviewLink = await page.locator('a:has-text("Overview")').isVisible(); | ||
|
|
||
| if (!overviewLink) { | ||
| return; | ||
| } | ||
| //Visible the navbar | ||
| await expect(page.locator('a:has-text("Pipelines")')).toBeVisible(); | ||
| await expect(page.locator('a:has-text("Models")')).toBeVisible(); | ||
| await expect(page.locator('a:has-text("Artifacts")')).toBeVisible(); | ||
| await expect(page.locator('a:has-text("Stacks")')).toBeVisible(); | ||
|
|
||
| //Change the URL by clicking each nav item | ||
| await page.click('a:has-text("Pipelines")'); | ||
| await expect(page).toHaveURL(/\/pipelines\?tab=pipelines/); | ||
|
|
||
| await page.click('a:has-text("Models")'); | ||
| await expect(page).toHaveURL(/\/models/); | ||
|
|
||
| await page.click('a:has-text("Artifacts")'); | ||
| await expect(page).toHaveURL(/\/artifacts/); | ||
|
|
||
| await page.click('a:has-text("Stacks")'); | ||
| await expect(page).toHaveURL(/\/stacks/); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { test } from "@playwright/test"; | ||
| import { login } from "./utils"; | ||
|
|
||
| test.describe("Login", () => { | ||
| test("Login with default username", async ({ page }) => { | ||
| await login(page); | ||
| }); | ||
|
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. LGTM: Test case implementation is concise. Consider adding assertions. The test case structure is correct, using an async function and the However, the current implementation lacks assertions to verify the success of the login process. Consider adding assertions to ensure the login was successful, such as: test("Login with default username", async ({ page }) => {
await login(page);
// Add assertions here, for example:
await expect(page).toHaveURL('/dashboard'); // Assuming successful login redirects to dashboard
await expect(page.locator('text=Welcome, User')).toBeVisible(); // Assuming there's a welcome message
});This will make the test more robust by verifying the expected outcome of the login process. |
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { test } from "@playwright/test"; | ||
| import { login } from "./utils"; | ||
|
|
||
| test.describe("Survey", () => { | ||
| test("Fill survey for first time", async ({ page }) => { | ||
| await login(page); | ||
|
|
||
| const isVisible = await page.locator("text=Add your account details").isVisible(); | ||
|
|
||
| if (!isVisible) { | ||
| return; | ||
| } | ||
|
|
||
| //survey form - Step 1 | ||
| await page.fill('input[name="fullName"]', "test"); | ||
| await page.fill('input[name="email"]', "[email protected]"); | ||
| await page | ||
| .getByLabel("I want to receive news and recommendations about how to use ZenML") | ||
| .check(); | ||
| await page.click('button span:has-text("Continue")'); | ||
| await page.waitForSelector("text=What will be your primary use for ZenML?"); | ||
|
|
||
| //survey form - Step 2 | ||
| await page.click('div label:has-text("Personal")'); | ||
| await page.click('button span:has-text("Continue")'); | ||
| await page.waitForSelector("text=What best describes your current situation with ZenML?"); | ||
|
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. 🛠️ Refactor suggestion Enhance test robustness and verifiability for survey steps 1 and 2. While the test covers the necessary interactions, consider the following improvements:
Here's an example of how you might improve step 1: // Helper function
async function clickContinue(page) {
await page.click('[data-testid="continue-button"]');
}
// In the test
// Step 1
await page.fill('[data-testid="fullName-input"]', "test");
await page.fill('[data-testid="email-input"]', "[email protected]");
await page.check('[data-testid="news-checkbox"]');
await clickContinue(page);
await expect(page.locator('[data-testid="step2-title"]')).toBeVisible(); |
||
|
|
||
| //survey form - Step 3 | ||
| await page.check('label:has-text("I\'m new to MLOps and exploring")'); | ||
| await page.click('button span:has-text("Continue")'); | ||
| await page.waitForSelector("text=What is your current infrastructure?"); | ||
|
|
||
| //survey form - Step 4 | ||
| await page.check('label:has-text("GCP") button'); | ||
| await page.check('label:has-text("Azure")'); | ||
| await page.check('label:has-text("Openshift")'); | ||
| await page.check('label:has-text("AWS")'); | ||
| await page.check('label:has-text("Native Kubernetes")'); | ||
| await page.click('button span:has-text("Continue")'); | ||
| await page.waitForSelector("text=Join The ZenML Slack Community"); | ||
|
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. 🛠️ Refactor suggestion Improve robustness and add assertions for survey steps 3 and 4. The test covers the necessary interactions, but could be improved:
Here's an example of how you might improve step 4: // Helper function
async function checkMultipleOptions(page, options) {
for (const option of options) {
await page.check(`[data-testid="${option}-checkbox"]`);
}
}
// In the test
// Step 4
const infrastructureOptions = ['GCP', 'Azure', 'Openshift', 'AWS', 'Native-Kubernetes'];
await checkMultipleOptions(page, infrastructureOptions);
await clickContinue(page);
await expect(page.locator('[data-testid="step5-title"]')).toBeVisible(); |
||
|
|
||
| //survey form - Step 5 | ||
| await page.click('button span:has-text("Skip")'); | ||
| }); | ||
| }); | ||
|
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. 🛠️ Refactor suggestion Add final assertions to verify survey completion. The test concludes by clicking the "Skip" button, but it doesn't verify the final state. Consider the following improvements:
Here's a suggested improvement: // Final step
await page.click('[data-testid="skip-button"]');
// Verify survey completion
await expect(page.locator('[data-testid="survey-completion-message"]')).toBeVisible();
// Verify redirection to dashboard or relevant post-survey page
await expect(page.locator('[data-testid="dashboard-title"]')).toBeVisible(); |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { Page, expect } from "@playwright/test"; | ||
|
|
||
| // reusable login function | ||
| export async function login(page: Page) { | ||
| await page.goto("http://127.0.0.1:8237/"); | ||
| await expect(page.locator('h1:has-text("Log in to your account")')).toBeVisible(); | ||
| await page.fill('input[name="username"]', "default"); | ||
| await page.fill('input[name="password"]', ""); | ||
| await page.click('button span:has-text("Login")'); | ||
| } |
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.
Consider failing the test if "Overview" link is not visible.
The current implementation silently passes the test if the "Overview" link is not visible. This might hide potential issues with the dashboard rendering.
Consider modifying the code to fail the test explicitly:
if (!overviewLink) { - return; + throw new Error("Overview link is not visible. Dashboard might not have loaded correctly."); }This change will make the test fail with a clear error message if the dashboard doesn't load as expected.
📝 Committable suggestion