feat: adds missing endpoints to e2e api tests - #5831
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR expands Management API E2E coverage for observability, complexity analysis, Skills, and Webhooks. It adds collection-script tests and updates authentication setup for deployments without admin credentials. ChangesManagement API E2E coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
98b6650 to
9989d15
Compare
9989d15 to
7eab1bd
Compare
1ad11a8 to
6fd9721
Compare
6fd9721 to
81bd10e
Compare
7eab1bd to
054f389
Compare
75346ae to
84f3a03
Compare
87b5a88 to
845b444
Compare
845b444 to
b311f3b
Compare
46236c0 to
041817a
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/e2e/api/runners/run-newman-api-tests.sh (1)
528-533: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote the comparisons and name the skip exit code.
Shellcheck reports SC2086 on both tests.
AUTH_SETUP_EXITcomes from$?, so word splitting is not reachable, but quoting clears the linter. The value3also duplicatesEXIT_SKIPintests/e2e/api/runners/set-auth-config.mjs. A named constant documents the cross-file contract.♻️ Proposed change
+# Must match EXIT_SKIP in runners/set-auth-config.mjs. +AUTH_SETUP_EXIT_SKIP=3 # Exit 3 means the auth pass cannot run here (fresh database, no bootstrap # token). That is a coverage gap, not a test failure -- do not fail a run # whose unauthenticated pass was green. - if [ $AUTH_SETUP_EXIT -eq 3 ]; then + if [ "$AUTH_SETUP_EXIT" -eq "$AUTH_SETUP_EXIT_SKIP" ]; then echo -e "${YELLOW}Skipping authenticated API management pass (see message above).${NC}" | tee -a "$LOG_FILE" - elif [ $AUTH_SETUP_EXIT -ne 0 ]; then + elif [ "$AUTH_SETUP_EXIT" -ne 0 ]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/api/runners/run-newman-api-tests.sh` around lines 528 - 533, Update the AUTH_SETUP_EXIT comparisons in the authenticated API management pass to quote the variable, clearing SC2086, and replace the literal skip value 3 with a named EXIT_SKIP constant matching set-auth-config.mjs. Preserve the existing skip behavior for the named exit code and failure handling for other nonzero results.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/collections/bifrost-api-management.postman_collection.json`:
- Around line 5135-5158: Update the “Cleanup Orphan Skill Files” request to
avoid deleting orphaned files belonging to other runs: either scope cleanup to
the unique skill name created by this test run, or verify and enforce that the
runner uses a dedicated server before retaining the global force=true cleanup.
Preserve isolation for parallel suites in line with the existing
Date.now()-based skill naming.
- Around line 4696-4711: Guard response parsing and nested identifier access in
all three Postman test scripts:
tests/e2e/api/collections/bifrost-api-management.postman_collection.json#L4696-L4711
should set skill_id only when body.skill.id exists; `#L4621-L4638` should catch
parse or missing-field failures and default skill_upload_storage_key and
skill_upload_blob_id to empty strings; and `#L5263-L5277` should set webhook_id
only when body.endpoint.id exists. Keep failures from throwing unhandled script
errors and preserve the existing successful-response assertions.
- Around line 4110-4121: Update the prerequest script for the complexity
configuration request to validate that complexity_config_json is present and
parses successfully before accessing it; if absent or invalid, call
pm.execution.skipRequest() and stop processing. Also verify current.keywords
exists before reading simple_keywords, while preserving the existing e2eprobe
injection for valid configurations.
In `@tests/e2e/api/README.md`:
- Around line 119-120: Update the resource-naming logic described in the README
to append a per-worker, per-run, or otherwise collision-resistant unique suffix
in addition to Date.now(). Ensure every parallel Newman process generates
distinct collection resource names and preserves the existing replay behavior.
- Around line 109-117: Update the status guidance in the README section
describing lifecycle requests and “(Coverage Probe)” requests to match the
collection script: coverage probes may accept 2xx or 4xx/5xx responses rather
than any status, and explicitly document the permitted 404/403 exceptions for
before-create, MCP, and plugin requests without the suffix.
---
Nitpick comments:
In `@tests/e2e/api/runners/run-newman-api-tests.sh`:
- Around line 528-533: Update the AUTH_SETUP_EXIT comparisons in the
authenticated API management pass to quote the variable, clearing SC2086, and
replace the literal skip value 3 with a named EXIT_SKIP constant matching
set-auth-config.mjs. Preserve the existing skip behavior for the named exit code
and failure handling for other nonzero results.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a63ddd9-8040-46a5-a448-48da3322d735
📒 Files selected for processing (4)
tests/e2e/api/README.mdtests/e2e/api/collections/bifrost-api-management.postman_collection.jsontests/e2e/api/runners/run-newman-api-tests.shtests/e2e/api/runners/set-auth-config.mjs
bf93a03 to
8e4e594
Compare
8e4e594 to
e66d4e0
Compare
e66d4e0 to
7eeea55
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/collections/collection-scripts.test.mjs`:
- Around line 1-3: Move collection-scripts.test.mjs out of tests/e2e into the
appropriate non-E2E test location, preserving its standalone Node.js execution
and test behavior; do not add the E2E fixture unless converting it to run
through the required E2E framework.
- Around line 1-3: Move collection-scripts.test.mjs out of tests/e2e/ into a
standalone validation location, unless you also wire it into the managed E2E
pipeline with the required base.fixture import and E2E conventions. Ensure the
chosen location and invocation clearly match how CI executes the validation.
- Around line 136-148: Update the complexityPrerequest test fixture to use an
ordered JSON string directly instead of JSON.stringify, and compare
ctx.pm.request.body.raw against the expected ordered JSON string. Remove the
JSON.parse-based field assertions while preserving the existing skipped-state
assertion and expected e2eprobe injection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 362ce412-af06-48e3-844a-3459a833d608
📒 Files selected for processing (3)
tests/e2e/api/README.mdtests/e2e/api/collections/bifrost-api-management.postman_collection.jsontests/e2e/api/collections/collection-scripts.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/api/README.md
- tests/e2e/api/collections/bifrost-api-management.postman_collection.json
7eeea55 to
9dc428f
Compare
The merge-base changed after approval.
9dc428f to
ac5f30f
Compare
Merge activity
|
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines