tbonelee commented on code in PR #5102: URL: https://github.com/apache/zeppelin/pull/5102#discussion_r2458808409
########## zeppelin-web-angular/e2e/tests/home/home-page-enhanced-functionality.spec.ts: ########## @@ -0,0 +1,86 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect, test } from '@playwright/test'; +import { HomePageUtil } from '../../models/home-page.util'; +import { addPageAnnotationBeforeEach, performLoginIfRequired, waitForZeppelinReady, PAGES } from '../../utils'; + +addPageAnnotationBeforeEach(PAGES.WORKSPACE.HOME); + +test.describe('Home Page Enhanced Functionality', () => { + let homeUtil: HomePageUtil; + + test.beforeEach(async ({ page }) => { + homeUtil = new HomePageUtil(page); + await page.goto('/'); + await waitForZeppelinReady(page); + await performLoginIfRequired(page); + }); + + test.describe('Given documentation links are displayed', () => { + test('When documentation link is checked Then should have correct version in URL', async ({ page }) => { + await homeUtil.verifyDocumentationVersionLink(); + }); + + test('When external links are checked Then should all open in new tab', async ({ page }) => { + await homeUtil.verifyAllExternalLinksTargetBlank(); + }); + }); + + test.describe('Given responsive grid layout', () => { + test('When page loads Then should display responsive grid with correct attributes', async ({ page }) => { + await homeUtil.verifyGridResponsiveness(); + }); + + test('When grid is displayed Then should have proper column structure', async ({ page }) => { + await homeUtil.verifyResponsiveGrid(); + }); + }); Review Comment: The method names `verifyResponsiveGrid()` and `verifyGridResponsivenewss()` are a bit misleading since they don't actually test responsive behavior. We could change the test name/description and util's method name, or change the how those util methods work. ########## zeppelin-web-angular/e2e/tests/home/home-page-note-operations.spec.ts: ########## @@ -0,0 +1,250 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect, test } from '@playwright/test'; +import { HomePage } from '../../models/home-page'; +import { addPageAnnotationBeforeEach, performLoginIfRequired, waitForZeppelinReady, PAGES } from '../../utils'; + +addPageAnnotationBeforeEach(PAGES.WORKSPACE.HOME); + +test.describe('Home Page Note Operations', () => { + let homePage: HomePage; + + test.beforeEach(async ({ page }) => { + homePage = new HomePage(page); + await page.goto('/'); + await waitForZeppelinReady(page); + await performLoginIfRequired(page); + await page.waitForSelector('zeppelin-node-list', { timeout: 15000 }); + }); + + test.describe('Given note operations are available', () => { + test('When note list loads Then should show note action buttons on hover', async ({ page }) => { + const notesExist = await page.locator('.node .file').count(); + + if (notesExist > 0) { + const firstNote = page.locator('.node .file').first(); + await firstNote.hover(); + + await expect(page.locator('.file .operation a[nztooltiptitle*="Rename note"]').first()).toBeVisible(); + await expect(page.locator('.file .operation a[nztooltiptitle*="Clear output"]').first()).toBeVisible(); + await expect(page.locator('.file .operation a[nztooltiptitle*="Move note to Trash"]').first()).toBeVisible(); + } else { + console.log('No notes available for testing operations'); + } + }); + + test('When hovering over note actions Then should show tooltip descriptions', async ({ page }) => { + const noteExists = await page + .locator('.node .file') + .first() + .isVisible() + .catch(() => false); + + if (noteExists) { + const firstNote = page.locator('.node .file').first(); + await firstNote.hover(); + + const renameIcon = page.locator('.file .operation a[nztooltiptitle*="Rename note"]').first(); + const clearIcon = page.locator('.file .operation a[nztooltiptitle*="Clear output"]').first(); + const deleteIcon = page.locator('.file .operation a[nztooltiptitle*="Move note to Trash"]').first(); + + await expect(renameIcon).toBeVisible(); + await expect(clearIcon).toBeVisible(); + await expect(deleteIcon).toBeVisible(); + } Review Comment: I'm not sure whether it is possible or not, it could be better if we also simulate the mouse hover on those icons and test tooltip visibilities. ########## zeppelin-web-angular/e2e/models/home-page.util.ts: ########## Review Comment: We could still remove `getPageMetadata()`, `navigateToLoginAndWaitForRedirect()` and related imports. ########## zeppelin-web-angular/e2e/tests/home/home-page-enhanced-functionality.spec.ts: ########## @@ -0,0 +1,86 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect, test } from '@playwright/test'; +import { HomePageUtil } from '../../models/home-page.util'; +import { addPageAnnotationBeforeEach, performLoginIfRequired, waitForZeppelinReady, PAGES } from '../../utils'; + +addPageAnnotationBeforeEach(PAGES.WORKSPACE.HOME); + +test.describe('Home Page Enhanced Functionality', () => { + let homeUtil: HomePageUtil; + + test.beforeEach(async ({ page }) => { + homeUtil = new HomePageUtil(page); + await page.goto('/'); + await waitForZeppelinReady(page); + await performLoginIfRequired(page); + }); + + test.describe('Given documentation links are displayed', () => { + test('When documentation link is checked Then should have correct version in URL', async ({ page }) => { + await homeUtil.verifyDocumentationVersionLink(); + }); + + test('When external links are checked Then should all open in new tab', async ({ page }) => { + await homeUtil.verifyAllExternalLinksTargetBlank(); + }); + }); + + test.describe('Given responsive grid layout', () => { + test('When page loads Then should display responsive grid with correct attributes', async ({ page }) => { + await homeUtil.verifyGridResponsiveness(); + }); + + test('When grid is displayed Then should have proper column structure', async ({ page }) => { + await homeUtil.verifyResponsiveGrid(); + }); + }); + + test.describe('Given welcome section display', () => { + test('When page loads Then should show welcome content with proper text', async ({ page }) => { + await homeUtil.verifyWelcomeSection(); + }); + + test('When welcome section is displayed Then should contain interactive elements', async ({ page }) => { + await homeUtil.verifyNotebookSection(); + }); + }); + + test.describe('Given community section content', () => { + test('When community section loads Then should display help and community headings', async ({ page }) => { + await homeUtil.verifyHelpSection(); + await homeUtil.verifyCommunitySection(); + }); + + test('When external links are displayed Then should show correct targets', async ({ page }) => { + const linkTargets = await homeUtil.testExternalLinkTargets(); + + expect(linkTargets.documentationHref).toContain('zeppelin.apache.org/docs'); + expect(linkTargets.mailingListHref).toContain('community.html'); + expect(linkTargets.issuesTrackingHref).toContain('issues.apache.org'); + expect(linkTargets.githubHref).toContain('github.com/apache/zeppelin'); + }); + }); + + test.describe('Given message service integration', () => { + test('When notes info message is received Then should update loading state', async ({ page }) => { + await page.evaluate(() => { + const mockData = { notes: [] }; + window.dispatchEvent(new CustomEvent('notes-info', { detail: mockData })); + }); + + await page.waitForTimeout(1000); + await homeUtil.verifyNotebookSection(); + }); + }); Review Comment: Which scenario is this test covering? ########## zeppelin-web-angular/e2e/tests/home/home-page-layout.spec.ts: ########## @@ -0,0 +1,144 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect, test } from '@playwright/test'; +import { HomePage } from '../../models/home-page'; +import { HomePageUtil } from '../../models/home-page.util'; +import { addPageAnnotationBeforeEach, performLoginIfRequired, waitForZeppelinReady, PAGES } from '../../utils'; + +test.describe('Home Page - Layout and Grid', () => { + addPageAnnotationBeforeEach(PAGES.WORKSPACE.HOME); + + test.beforeEach(async ({ page }) => { + await page.goto('/#/'); + await waitForZeppelinReady(page); + await performLoginIfRequired(page); + }); + + test.describe('Responsive Grid Layout', () => { + test('should display responsive grid structure', async ({ page }) => { + const homePageUtil = new HomePageUtil(page); + + await test.step('Given I am on the home page', async () => { + const homePage = new HomePage(page); + await homePage.navigateToHome(); + }); + + await test.step('When the page loads', async () => { + await waitForZeppelinReady(page); + }); + + await test.step('Then I should see the responsive grid layout', async () => { + await homePageUtil.verifyResponsiveGrid(); + }); + }); + + test('should have proper column distribution', async ({ page }) => { + const homePage = new HomePage(page); + + await test.step('Given I am on the home page', async () => { + await homePage.navigateToHome(); + }); + + await test.step('When I examine the grid columns', async () => { + await expect(homePage.moreInfoGrid).toBeVisible(); + }); + + await test.step('Then I should see the notebook column with proper sizing', async () => { + await expect(homePage.notebookColumn).toBeVisible(); + // Check that the column contains notebook content + const notebookHeading = homePage.notebookColumn.locator('h3'); + await expect(notebookHeading).toBeVisible(); + const headingText = await notebookHeading.textContent(); + expect(headingText).toContain('Notebook'); + }); + + await test.step('And I should see the help/community column with proper sizing', async () => { + await expect(homePage.helpCommunityColumn).toBeVisible(); + // Check that the column contains help and community content + const helpHeading = homePage.helpCommunityColumn.locator('h3').first(); + await expect(helpHeading).toBeVisible(); + const helpText = await helpHeading.textContent(); + expect(helpText).toContain('Help'); + }); + }); + + test('should maintain layout structure across different viewport sizes', async ({ page }) => { + const homePage = new HomePage(page); + + await test.step('Given I am on the home page', async () => { + await homePage.navigateToHome(); + }); + + await test.step('When I resize to tablet view', async () => { + await page.setViewportSize({ width: 768, height: 1024 }); + await page.waitForTimeout(500); + }); + + await test.step('Then the grid should still be visible and functional', async () => { + await expect(homePage.moreInfoGrid).toBeVisible(); + await expect(homePage.notebookColumn).toBeVisible(); + await expect(homePage.helpCommunityColumn).toBeVisible(); + }); + + await test.step('When I resize to mobile view', async () => { + await page.setViewportSize({ width: 375, height: 667 }); + await page.waitForTimeout(500); + }); + + await test.step('Then the grid should adapt to mobile layout', async () => { + await expect(homePage.moreInfoGrid).toBeVisible(); + // Check that both columns are still visible and stacked vertically Review Comment: Is this testing if the columns are stacked vertically? If not, we could remove misleading words `and stacked vertically` in the comment or removed the whole comment. -- 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]
