Skip to content

Conversation

dididy
Copy link
Contributor

@dididy dididy commented Oct 12, 2025

What is this PR for?

Addition and improvement of Home/Dashboard-related E2E tests for New UI


PAGES.WORKSPACE.HOME

→ src/app/pages/workspace/home/home.component

PAGES.WORKSPACE.MAIN

→ src/app/pages/workspace/workspace.component

What type of PR is it?

Improvement

Todos

What is the Jira issue?

ZEPPELIN-6359

How should this be tested?

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

Copy link
Contributor

Choose a reason for hiding this comment

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

verifyExeternalLinks() methods are duplicated and some unused methods coule be removed.

Comment on lines +85 to +96
if (docCount > 0) {
await expect(this.homePage.externalLinks.documentation).toBeVisible();
}
if (mailCount > 0) {
await expect(this.homePage.externalLinks.mailingList).toBeVisible();
}
if (issuesCount > 0) {
await expect(this.homePage.externalLinks.issuesTracking).toBeVisible();
}
if (githubCount > 0) {
await expect(this.homePage.externalLinks.github).toBeVisible();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Although this came in a previous PR, is there a reason we guard these visibility assertions with the *Count > 0 checks? If these links are expected to be present, would it make sense to assert visibility unconditionally?

Comment on lines 47 to 52
async verifyHomePageIntegrity(): Promise<void> {
await this.verifyHomePageElements();
await this.verifyNotebookFunctionalities();
await this.verifyTutorialNotebooks();
await this.verifyExternalLinks();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a personal take: since this helper is only used to check that the user lands on the home page in anonymous-login-redirect.spec, how about dropping it and simply calling verifyHomePageElements() to confirm the redirect?
We already assert the detailed home-page UI in home-page-elements.spec, so here we could verify only a couple of “home page” indicators to keep this file leaner. That might also let us remove a few related methods as a small cleanup. Happy to defer if there’s other planned usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants