test: stream_routes in services#3113
Merged
SkyeYoung merged 18 commits intoapache:masterfrom Jun 16, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds test coverage for stream routes under services and introduces related schema, UI, form, and API enhancements to support these tests.
- Stream route schema updates to make
confandloggeroptional - Integration of
produceRmUpstreamWhenHasinto add/edit form pipelines - Addition of
deleteAllStreamRouteshelper and comprehensive E2E tests for listing and CRUD
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/schema/apisix/stream_routes.ts | Made conf and logger optional in StreamRouteProtocol schema |
| src/routes/stream_routes/index.tsx | Updated list columns to display server_addr and server_port |
| src/routes/stream_routes/detail.$id.tsx | Applied produceRmUpstreamWhenHas in update mutation pipeline |
| src/routes/stream_routes/add.tsx | Applied produceRmUpstreamWhenHas in creation mutation pipeline |
| src/components/form/JsonInput.tsx | Extended JSON input with objValue support and omitted prop |
| src/components/form-slice/FormPartStreamRoute/index.tsx | Provided objValue to JSON inputs for protocol fields |
| src/apis/stream_routes.ts | Added deleteAllStreamRoutes helper |
| e2e/tests/services.stream_routes.list.spec.ts | E2E tests for service-scoped stream routes listing |
| e2e/tests/services.stream_routes.crud.spec.ts | E2E tests for service-scoped stream routes CRUD |
| e2e/pom/services.ts | POM updates for navigating and interacting with stream routes |
Comments suppressed due to low confidence (3)
src/routes/stream_routes/detail.$id.tsx:42
- [nitpick] The function name
produceRmUpstreamWhenHasis cryptic; consider renaming to something more descriptive likeremoveUpstreamIfServiceIdPresentfor clarity.
import { produceRmUpstreamWhenHas } from '@/utils/form-producer';
src/apis/stream_routes.ts:67
- There aren't any unit tests covering
deleteAllStreamRoutes; adding tests to confirm it correctly paginates and deletes all items would catch regressions early.
export const deleteAllStreamRoutes = async (req: AxiosInstance) => {
src/components/form/JsonInput.tsx:18
- [nitpick] Introducing a new dependency just for
omitmay increase bundle size and cognitive load; consider using built-in object destructuring or a shared utility (e.g.,lodash.omit) to maintain consistency.
import { omit } from 'rambdax';
juzhiyuan
reviewed
Jun 13, 2025
This was referenced Jun 13, 2025
4 tasks
…_routes-in-services
LiteSun
approved these changes
Jun 16, 2025
juzhiyuan
approved these changes
Jun 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please answer these questions before submitting a pull request, or your PR will get closed.
Why submit this pull request?
What changes will this PR take into?
Depend on apache/apisix#12291, #3111, #3112, #3117
Meanwhile, some essential code that needed fixing has been fixed.
Paths:
e2e/tests/services.stream_routes.list.spec.ts:
e2e/tests/services.stream_routes.crud.spec.ts:
stream_routes.crud-*.spec.ts, the CRUD tests included in this PR reduce the repetition of existing paths.