tbonelee commented on code in PR #5102:
URL: https://github.com/apache/zeppelin/pull/5102#discussion_r2433199972
##########
zeppelin-web-angular/e2e/models/home-page.util.ts:
##########
@@ -82,10 +82,18 @@ export class HomePageUtil {
const issuesCount = await
this.homePage.externalLinks.issuesTracking.count();
const githubCount = await this.homePage.externalLinks.github.count();
- 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();
+ 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();
+ }
Review Comment:
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?
##########
zeppelin-web-angular/e2e/models/home-page.util.ts:
##########
Review Comment:
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.
##########
zeppelin-web-angular/e2e/models/home-page.util.ts:
##########
Review Comment:
`verifyExeternalLinks()` methods are duplicated and some unused methods
coule be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
