test(e2e): add comprehensive service CRUD tests and UI utility functions #3258
Conversation
- Remove test.setTimeout(30000) as default timeout is already 30s - Test completes in ~9 seconds, well under the default limit
|
@Baoyuantop i had to delete that PR because of some of git issues but I recovered the commits and raised the new PR the changes are same as they were in the previous PR the CI issue has also been resolved. I think it should pass the e2e test as it does locally. |
- Wrapped deletion calls in retry logic to handle transient 500 errors - Added 404 error suppression to handle potential race conditions/idempotency
- Increase uiHasToastMsg default timeout to 30s (from 5s) for all toast messages - Increase services.crud-required-fields timeout to 30s for backend responses - Increase stream_routes.show-disabled-error timeout to 30s after Docker restart - Add comments explaining CI-specific timeout requirements These generous timeouts ensure tests pass reliably in slower CI environments while still failing fast enough locally to catch real issues. All 76 E2E tests pass successfully with these changes.
|
Fixed CI timing issues by increasing timeouts to 30s across all toast message assertions and critical backend operations. All 76 E2E tests now pass successfully. The changes ensure reliable test execution in slower CI environments while maintaining the principle that API calls are only used for setup/teardown all CRUD operations use UI interactions as required. |
| import { deleteAllServices, postServiceReq } from '@/apis/services'; | ||
| import { deleteAllStreamRoutes } from '@/apis/stream_routes'; | ||
|
|
||
| test.describe.configure({ mode: 'serial' }); |
There was a problem hiding this comment.
Why is it necessary to add this?
There was a problem hiding this comment.
This configuration is necessary to ensure test stability and isolation. Since the global Playwright config enables fullyParallel: true, adding mode: 'serial' is critical to prevent race conditions in this file, where tests share a single Service resource created in the beforeAll hook. Without it, concurrent test execution could lead to conflicts when reading or modifying that shared state.
There was a problem hiding this comment.
Why could CI be run through before, but not now? Please confirm the reason.
There was a problem hiding this comment.
The CI likely passed before due to a "lucky" execution order where the environment was clean. The failures now are caused by strict Foreign Key constraints in the backend.
If a previous test (or a parallel run) leaves "zombie" Routes or Stream Routes attached to a Service, the old deleteAllServices would fail to clean up, causing the beforeAll hook to crash. The updated deleteAllServices fixes this by explicitly removing dependencies first.
The mode: 'serial' is added as a safeguard. Since fullyParallel: true is enabled globally, this ensures that if more tests are added to this file in the future, they won't race against each other for the shared testServiceId.
| import { deleteAllServices, postServiceReq } from '@/apis/services'; | ||
| import { deleteAllStreamRoutes } from '@/apis/stream_routes'; | ||
|
|
||
| test.describe.configure({ mode: 'serial' }); |
There was a problem hiding this comment.
Why could CI be run through before, but not now? Please confirm the reason.
3 participants
Why submit this pull request?
What changes will this PR take into?
This PR adds comprehensive E2E tests for the Services resource, covering all CRUD operations and list page functionality.
Test Coverage (5 tests, all passing ✅):
List page tests (
services.list.spec.ts) - 3 testsCRUD with required fields (
services.crud-required-fields.spec.ts) - 1 testCRUD with all fields (
services.crud-all-fields.spec.ts) - 1 testFiles Added:
e2e/utils/ui/services.ts- UI helper functions for form filling and verification with proper selector disambiguatione2e/tests/services.list.spec.ts- List page and pagination testse2e/tests/services.crud-required-fields.spec.ts- Minimal fields CRUD testse2e/tests/services.crud-all-fields.spec.ts- All fields CRUD testsImplementation Details:
.first()to avoid conflicts between service and upstream fields (name, description, labels)deleteAllServiceshelperRelated issues
resolve #3085
Checklist: