Add E2E tests for Resources/Namespaces in the dashboard #247

SunsetB612 · 2025-08-16T03:10:29Z

Add E2E tests for Resources/Namespaces in the dashboard

gemini-code-assist

Summary of Changes

Hello @SunsetB612, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the end-to-end testing coverage for the dashboard's Namespaces section. It introduces a suite of Playwright tests that validate the core functionalities of namespace management, including creation, listing, and deletion. Additionally, it includes a test to ensure proper handling of network errors when fetching namespace data. This work also involves a minor refactoring of the E2E test directory structure to better organize the test files.

Highlights

E2E Test File Restructuring: The existing login.spec.ts file was refactored and moved into a new login subdirectory, indicating an organization of E2E tests.
Comprehensive Namespace E2E Tests: New Playwright E2E tests have been introduced to cover the full lifecycle of namespace management within the dashboard. This includes tests for creating, listing, and deleting namespaces.
Network Error Handling Test for Namespaces: A dedicated E2E test was added to specifically verify the dashboard's resilience and error handling when network requests for namespaces fail, ensuring a robust user experience.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds a comprehensive set of E2E tests for namespace management in the dashboard, covering creation, deletion, listing, and network error scenarios. The tests are well-structured and cover important user flows.

However, there is significant code duplication across the new test files, particularly in the setup (beforeEach block and related constants). I recommend refactoring this common logic into a shared helper function to improve maintainability.

Additionally, I've pointed out several areas for improvement in the individual test files, such as removing hardcoded IP addresses, avoiding static waits (waitForTimeout), removing leftover debugging code, and strengthening assertions. Addressing these points will make the tests more robust and reliable.

gemini-code-assist · 2025-08-16T03:11:51Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+const baseURL = process.env.BASE_URL || 'http://192.168.47.131:5173';
+const basePath = '/multicloud-resource-manage';
+const token = process.env.KARMADA_TOKEN || '';


These constants, especially baseURL with a hardcoded IP address, are duplicated across all new test files. This makes the tests less portable and harder to maintain. As mentioned in the general feedback, this setup logic should be centralized in a shared helper. The playwright.config.ts already defines a baseURL based on localhost, which is a better practice than using a hardcoded IP.

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-network-error.spec.ts

+    const rowCount = await tableRows.count();
+    const errorCount = await errorMsg.count();
+
+    console.log(`Table rows: ${rowCount}, Error messages: ${errorCount}`);
+
+    // 最终断言：要么表格为空，要么显示网络错误
+    expect(rowCount === 0 || errorCount > 0).toBeTruthy();


This section contains leftover debugging code and a weak assertion. The test should explicitly check that an error message is visible and that the table is empty when the API call fails. This makes the test more precise and robust.

Suggested change

const rowCount = await tableRows.count();

const errorCount = await errorMsg.count();

console.log(`Table rows: ${rowCount}, Error messages: ${errorCount}`);

// 最终断言：要么表格为空，要么显示网络错误

expect(rowCount === 0 || errorCount > 0).toBeTruthy();

// Assert that an error message is shown and the table is empty

await expect(errorMsg).toBeVisible();

await expect(tableRows).toHaveCount(0);

is the relationship of two asserts should be or or and

it seems that the advice from bot more useful

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+    const searchBox = page.getByPlaceholder('Search by Name');
+    await searchBox.fill(namespaceName);
+    await searchBox.press('Enter');
+    await page.waitForTimeout(1000);


Using page.waitForTimeout() is generally discouraged as it can lead to flaky tests (if the wait is too short) or slow down the test suite (if it's too long). It's better to wait for a specific element state or network condition. In this case, the following waitForSelector should be sufficient. Please remove this static wait.

+1， waitForTimeout seems useful when debug the e2e test, but not suitable for e2e test in production

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+    await page.waitForSelector(`tr:has-text("${namespaceName}")`, { timeout: 60000 });
+    await expect(page.locator('table')).toContainText(namespaceName);
+
+    await page.screenshot({ path: 'debug-namespace-create.png', fullPage: true });


This screenshot seems to be for debugging purposes. While useful during development, it should be removed from the final test code to avoid cluttering the repository with artifacts. Playwright's tracing feature, enabled in your config (trace: 'retry-with-trace'), is a better way to debug failed test runs.

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-delete.spec.ts

+    const searchBox = page.getByPlaceholder('Search by Name');
+    await searchBox.fill(namespaceName);
+    await searchBox.press('Enter');
+    await page.waitForTimeout(1000);


Avoid using page.waitForTimeout(). It introduces flakiness and slows down tests. Prefer waiting for specific conditions or elements. The subsequent waitForSelector should be sufficient.

same problem

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-delete.spec.ts

+    await page.reload({ waitUntil: 'networkidle' });
+
+    // 使用搜索框确认删除
+    await searchBox.fill('');


The searchBox.fill(namespaceName) call on the next line will automatically clear the input before filling it. Therefore, this searchBox.fill('') call is redundant and can be removed.

await searchBox.fill(''); await searchBox.fill(namespaceName);

seems has the same effect as

await searchBox.fill(namespaceName);

you can test it locally and recheck it

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-delete.spec.ts

+    // 清空搜索框
+    await searchBox.clear();
+
+    await page.screenshot({ path: 'debug-namespace-delete.png', fullPage: true });


This screenshot appears to be for debugging. Please remove it from the final test code. The tracing feature in Playwright is the recommended way to debug test failures.

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-list.spec.ts

+    // 验证表格中包含默认 namespace
+    await expect(table).toContainText('default');
+
+    await page.screenshot({ path: 'debug-namespace-list.png', fullPage: true });


This screenshot seems to be for debugging purposes. It should be removed from the test to keep the codebase clean.

+1. What about add debug condition, and when debug condition is true, then take snapshot, otherwise ignore screenshotting, any ideas?

gemini-code-assist · 2025-08-16T03:11:52Z

ui/apps/dashboard/e2e/namespace/namespace-network-error.spec.ts

+    expect(rowCount === 0 || errorCount > 0).toBeTruthy();
+
+    // 截图调试
+    await page.screenshot({ path: 'debug-namespace-network-failure.png', fullPage: true });


This screenshot appears to be for debugging. Please remove it from the final test code.

warjiang · 2025-08-16T07:27:49Z

/assign

warjiang · 2025-08-20T14:39:34Z

.github/workflows/ci.yml

          path: karmada-dashboard/ui/apps/dashboard/playwright-report/
-          retention-days: 30
-
+          retention-days: 30


plz fix all kind of similar problem, just add extra new line at the end of the file ~

warjiang · 2025-08-20T14:39:57Z

.github/workflows/ci.yml

        # the same as the End of Life of the Kubernetes release: https://kubernetes.io/releases/
        # Please remember to update the CI Schedule Workflow when we add a new version.
        k8s: [ v1.31.0, v1.32.0, v1.33.0 ]
+        # k8s: [ v1.31.0 ]


clean the debug code

warjiang · 2025-08-20T14:54:14Z

.github/workflows/ci.yml

          export CLUSTER_VERSION=kindest/node:${{ matrix.k8s }}
          hack/local-up-karmada.sh
+          echo "KUBECONFIG=/home/runner/.kube/karmada.config" >> $GITHUB_ENV
+      - name: Debug cluster status


could optimize the debug code or adopt the debug code with debug var

warjiang · 2025-08-20T14:56:04Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+    const searchBox = page.getByPlaceholder('Search by Name');
+    await searchBox.fill(namespaceName);
+    await searchBox.press('Enter');
+    await page.waitForTimeout(1000);


+1， waitForTimeout seems useful when debug the e2e test, but not suitable for e2e test in production

warjiang · 2025-08-20T14:56:30Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+    await page.waitForSelector(`tr:has-text("${namespaceName}")`, { timeout: 60000 });
+    await expect(page.locator('table')).toContainText(namespaceName);
+
+    await page.screenshot({ path: 'debug-namespace-create.png', fullPage: true });


warjiang · 2025-08-20T14:57:53Z

ui/apps/dashboard/e2e/namespace/namespace-delete.spec.ts

+    await page.reload({ waitUntil: 'networkidle' });
+
+    // 使用搜索框确认删除
+    await searchBox.fill('');


await searchBox.fill(''); await searchBox.fill(namespaceName);

seems has the same effect as

await searchBox.fill(namespaceName);

warjiang · 2025-08-20T14:58:08Z

ui/apps/dashboard/e2e/namespace/namespace-delete.spec.ts

+    await page.reload({ waitUntil: 'networkidle' });
+
+    // 使用搜索框确认删除
+    await searchBox.fill('');


you can test it locally and recheck it

warjiang · 2025-08-20T14:59:28Z

ui/apps/dashboard/e2e/namespace/namespace-list.spec.ts

+    // 验证表格中包含默认 namespace
+    await expect(table).toContainText('default');
+
+    await page.screenshot({ path: 'debug-namespace-list.png', fullPage: true });


+1. What about add debug condition, and when debug condition is true, then take snapshot, otherwise ignore screenshotting, any ideas?

warjiang · 2025-08-20T15:00:45Z

ui/apps/dashboard/e2e/namespace/namespace-network-error.spec.ts

+    const rowCount = await tableRows.count();
+    const errorCount = await errorMsg.count();
+
+    console.log(`Table rows: ${rowCount}, Error messages: ${errorCount}`);
+
+    // 最终断言：要么表格为空，要么显示网络错误
+    expect(rowCount === 0 || errorCount > 0).toBeTruthy();


is the relationship of two asserts should be or or and

warjiang · 2025-08-20T15:01:09Z

ui/apps/dashboard/e2e/namespace/namespace-network-error.spec.ts

+    const rowCount = await tableRows.count();
+    const errorCount = await errorMsg.count();
+
+    console.log(`Table rows: ${rowCount}, Error messages: ${errorCount}`);
+
+    // 最终断言：要么表格为空，要么显示网络错误
+    expect(rowCount === 0 || errorCount > 0).toBeTruthy();


it seems that the advice from bot more useful

warjiang · 2025-08-24T16:23:01Z

.github/workflows/ci.yml

          hack/local-up-karmada.sh
+          echo "KUBECONFIG=/home/runner/.kube/karmada.config" >> $GITHUB_ENV
+      - name: Debug cluster status
+        if: ${{ env.DEBUG == 'true' }}


warjiang · 2025-08-24T16:23:58Z

.github/workflows/ci.yml

+          # 使用 kubectl create token 命令生成临时 token
+          #TOKEN=$(kubectl --context=karmada-apiserver create token e2e-test -n kube-system --duration=3600s)
+          # kubectl --context=karmada-apiserver apply -f karmada-dashboard/artifacts/dashboard
+          kubectl --context=karmada-host apply -k karmada-dashboard/artifacts/overlays/nodeport-mode


maybe we can reuse the debug var or just drop lines

warjiang · 2025-08-24T16:24:26Z

.github/workflows/ci.yml

+          make run-api &
+          echo $! > .api.pid
+          sleep 5
+      # ===== 添加API服务器健康检查 =====


plz translate the line~

warjiang · 2025-08-24T16:25:33Z

ui/apps/dashboard/e2e/namespace/namespace-create.spec.ts

+    // Debug
+    if(process.env.DEBUG === 'true'){
+        await page.screenshot({ path: 'debug-namespace-create.png', fullPage: true });
+    }


warjiang · 2025-08-24T16:26:21Z

ui/apps/dashboard/e2e/namespace/namespace-network-error.spec.ts

+    if(process.env.DEBUG === 'true'){
+        await page.screenshot({ path: 'debug-namespace-network-failure.png', fullPage: true });
+    }
+});


extra empty line should be added

warjiang · 2025-08-24T16:28:07Z

@SunsetB612 in short

plz translate chinese to english
fix the format of all submitted files, add extra empty line
rm unused code or keep it with debug var

after that, I think we can merge the PR and push forward ~

Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>

warjiang · 2025-08-25T00:36:44Z

/lgtm
/approve
thanks for your hard work ~

karmada-bot · 2025-08-25T00:36:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: warjiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [warjiang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot requested review from jhnine and samzong August 16, 2025 03:10

gemini-code-assist bot reviewed Aug 16, 2025

View reviewed changes

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 16, 2025

gemini-code-assist bot reviewed Aug 16, 2025

View reviewed changes

karmada-bot assigned warjiang Aug 16, 2025

SunsetB612 force-pushed the add-e2e-fabric3 branch 2 times, most recently from 26af17b to 13affa1 Compare August 20, 2025 06:37

warjiang reviewed Aug 20, 2025

View reviewed changes

SunsetB612 force-pushed the add-e2e-fabric3 branch from 13affa1 to 8d71a78 Compare August 20, 2025 18:49

karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 24, 2025

SunsetB612 force-pushed the add-e2e-fabric3 branch from aeff57c to b406008 Compare August 24, 2025 11:59

karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 24, 2025

warjiang reviewed Aug 24, 2025

View reviewed changes

Add E2E tests for Resources/Namespaces in the dashboard

1734a70

Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>

SunsetB612 force-pushed the add-e2e-fabric3 branch from b68374b to 1734a70 Compare August 24, 2025 18:42

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2025

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2025

karmada-bot merged commit 3564b0e into karmada-io:main Aug 25, 2025
10 checks passed

XiShanYongYe-Chang mentioned this pull request Sep 10, 2025

[Umbrella][Summer OSPP 2025] Introduce E2E for the Karmada Dashboard #261

Open

18 tasks

Add E2E tests for Resources/Namespaces in the dashboard #247

Add E2E tests for Resources/Namespaces in the dashboard #247

Conversation

SunsetB612 commented Aug 16, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

warjiang commented Aug 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!