Copilot commented on code in PR #3305:
URL: https://github.com/apache/apisix-dashboard/pull/3305#discussion_r2893187568
##########
e2e/tests/services.stream_routes.list.spec.ts:
##########
@@ -16,240 +16,231 @@
*/
import { servicesPom } from '@e2e/pom/services';
import { randomId } from '@e2e/utils/common';
+import { env } from '@e2e/utils/env';
import { e2eReq } from '@e2e/utils/req';
import { test } from '@e2e/utils/test';
import { uiGoto } from '@e2e/utils/ui';
-import { expect } from '@playwright/test';
+import { expect, type Page } from '@playwright/test';
-import { deleteAllServices, postServiceReq } from '@/apis/services';
-import {
- deleteAllStreamRoutes,
- postStreamRouteReq,
-} from '@/apis/stream_routes';
+import { postServiceReq } from '@/apis/services';
+import { getStreamRouteListReq, postStreamRouteReq } from
'@/apis/stream_routes';
test.describe.configure({ mode: 'serial' });
const serviceName = randomId('test-service');
const anotherServiceName = randomId('another-service');
+// Use indexed octets with random offset to ensure uniqueness (0-4 starting at
random value to avoid reserved ranges)
+const randomOffset = Math.floor(Math.random() * 100) + 100;
+let octetIndex = randomOffset;
+const getUniqueOctet = () => (octetIndex++ % 256);
const streamRoutes = [
{
- server_addr: '127.0.0.1',
+ server_addr: `127.0.0.${getUniqueOctet()}`,
server_port: 8080,
},
{
- server_addr: '127.0.0.2',
+ server_addr: `127.0.0.${getUniqueOctet()}`,
server_port: 8081,
},
{
- server_addr: '127.0.0.3',
+ server_addr: `127.0.0.${getUniqueOctet()}`,
server_port: 8082,
},
];
-// Stream route that uses upstream directly instead of service_id
const upstreamStreamRoute = {
- server_addr: '127.0.0.40',
+ server_addr: `127.0.0.${getUniqueOctet()}`,
server_port: 9090,
upstream: {
nodes: [{ host: 'example.com', port: 80, weight: 100 }],
},
};
-// Stream route that belongs to another service
const anotherServiceStreamRoute = {
- server_addr: '127.0.0.20',
+ server_addr: `127.0.0.${getUniqueOctet()}`,
server_port: 9091,
};
let testServiceId: string;
let anotherServiceId: string;
const createdStreamRoutes: string[] = [];
-test.beforeAll(async () => {
- await deleteAllStreamRoutes(e2eReq);
- await deleteAllServices(e2eReq);
+let upstreamStreamRouteId: string;
+let anotherServiceStreamRouteId: string;
- // Create a test service for testing service stream routes
+test.beforeAll(async () => {
const serviceResponse = await postServiceReq(e2eReq, {
name: serviceName,
desc: 'Test service for stream route listing',
+ labels: { test: serviceName },
});
testServiceId = serviceResponse.data.value.id;
- // Create another service
const anotherServiceResponse = await postServiceReq(e2eReq, {
name: anotherServiceName,
desc: 'Another test service for stream route isolation testing',
+ labels: { test: anotherServiceName },
});
anotherServiceId = anotherServiceResponse.data.value.id;
- // Create test stream routes under the service
for (const streamRoute of streamRoutes) {
const streamRouteResponse = await postStreamRouteReq(e2eReq, {
server_addr: streamRoute.server_addr,
server_port: streamRoute.server_port,
service_id: testServiceId,
+ labels: { test: serviceName },
});
+ if (!streamRouteResponse.data?.value) {
+ throw new Error(`Failed to create stream route:
${JSON.stringify(streamRouteResponse.data)}`);
+ }
createdStreamRoutes.push(streamRouteResponse.data.value.id);
}
- // Create a stream route that uses upstream directly instead of service_id
- await postStreamRouteReq(e2eReq, upstreamStreamRoute);
+ const upstreamStreamRouteResponse = await postStreamRouteReq(e2eReq, {
+ ...upstreamStreamRoute,
+ labels: { test: 'upstream-' + serviceName },
+ });
+ if (!upstreamStreamRouteResponse.data?.value) {
+ throw new Error(`Failed to create upstream stream route:
${JSON.stringify(upstreamStreamRouteResponse.data)}`);
+ }
+ upstreamStreamRouteId = upstreamStreamRouteResponse.data.value.id;
- // Create a stream route under another service
- await postStreamRouteReq(e2eReq, {
+ const anotherServiceStreamRouteResponse = await postStreamRouteReq(e2eReq, {
...anotherServiceStreamRoute,
service_id: anotherServiceId,
+ labels: { test: anotherServiceName },
});
+ if (!anotherServiceStreamRouteResponse.data?.value) {
+ throw new Error(`Failed to create another service stream route:
${JSON.stringify(anotherServiceStreamRouteResponse.data)}`);
+ }
+ anotherServiceStreamRouteId =
anotherServiceStreamRouteResponse.data.value.id;
+
+ // Wait for data propagation to complete by polling the backend
+ await expect(async () => {
+ const res = await getStreamRouteListReq(e2eReq, { page_size: 100 } as
Parameters<typeof getStreamRouteListReq>[1]);
+ const existingIds = res.list.map((r) => r.value.id);
+ const expectedIds = [...createdStreamRoutes, upstreamStreamRouteId,
anotherServiceStreamRouteId].filter(Boolean);
+ expect(expectedIds.every((id) => existingIds.includes(id))).toBeTruthy();
+ }).toPass({ timeout: 15000, intervals: [1000] });
});
test.afterAll(async () => {
- await deleteAllStreamRoutes(e2eReq);
- await deleteAllServices(e2eReq);
+ const allStreamRouteIds = [...createdStreamRoutes, upstreamStreamRouteId,
anotherServiceStreamRouteId].filter(Boolean);
+ for (const id of allStreamRouteIds) {
+ await e2eReq.delete(`/stream_routes/${id}`);
+ }
+
+ if (testServiceId) {
+ await e2eReq.delete(`/services/${testServiceId}`);
+ }
+ if (anotherServiceId) {
+ await e2eReq.delete(`/services/${anotherServiceId}`);
+ }
});
-test('should only show stream routes with current service_id', async ({
- page,
-}) => {
- await test.step('should only show stream routes with current service_id',
async () => {
- await servicesPom.toIndex(page);
- await servicesPom.isIndexPage(page);
+async function navigateToServiceDetail(page: Page, id: string) {
+ await uiGoto(page, '/services/detail/$id', { id });
+ await page.waitForLoadState('load');
+ await servicesPom.isDetailPage(page);
+}
- await page
- .getByRole('row', { name: serviceName })
- .getByRole('button', { name: 'View' })
- .click();
- await servicesPom.isDetailPage(page);
+test('should only show stream routes with current service_id', async ({ page
}) => {
+ await test.step('should only show stream routes with current service_id',
async () => {
+ await navigateToServiceDetail(page, testServiceId);
await servicesPom.getServiceStreamRoutesTab(page).click();
await servicesPom.isServiceStreamRoutesPage(page);
+ await page.waitForLoadState('load');
+
+ await expect(page.getByRole('cell', { name:
anotherServiceStreamRoute.server_addr })).toBeHidden();
+ await expect(page.getByRole('cell', { name:
upstreamStreamRoute.server_addr })).toBeHidden();
- // Stream routes from another service should not be visible
- await expect(
- page.getByRole('cell', { name: anotherServiceStreamRoute.server_addr })
- ).toBeHidden();
- // Upstream stream route (without service_id) should not be visible
- await expect(
- page.getByRole('cell', { name: upstreamStreamRoute.server_addr })
- ).toBeHidden();
- // Only stream routes belonging to current service should be visible
for (const streamRoute of streamRoutes) {
- await expect(
- page.getByRole('cell', { name: streamRoute.server_addr })
- ).toBeVisible();
+ await expect(page.getByRole('cell', { name: streamRoute.server_addr
})).toBeVisible({ timeout: 30000 });
}
});
await test.step('without service_id stream routes should still exist in the
stream routes list', async () => {
- await uiGoto(page, '/stream_routes');
- await expect(page).toHaveURL((url) =>
- url.pathname.endsWith('/stream_routes')
- );
- const title = page.getByRole('heading', { name: 'Stream Routes' });
- await expect(title).toBeVisible();
-
- // All stream routes should be visible in the global stream routes list
- await expect(
- page.getByRole('cell', { name: upstreamStreamRoute.server_addr })
- ).toBeVisible();
- await expect(
- page.getByRole('cell', { name: anotherServiceStreamRoute.server_addr })
- ).toBeVisible();
+ // Navigate using POM to reach the correct base path
+ await page.goto(`${env.E2E_TARGET_URL}stream_routes`);
Review Comment:
`page.goto(`${env.E2E_TARGET_URL}stream_routes`)` assumes `E2E_TARGET_URL`
always ends with a trailing slash. If someone overrides the env var without the
trailing `/`, this produces an invalid URL (e.g. `.../apisixstream_routes`).
Use `uiGoto(page, '/stream_routes')`, `new URL('stream_routes',
env.E2E_TARGET_URL)`, or otherwise ensure exactly one `/` between base and path.
```suggestion
await uiGoto(page, '/stream_routes');
```
--
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]
