feat(function): Add support for micro-sandbox configuration#159
feat(function): Add support for micro-sandbox configuration#159liuzewen99 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds ChangesmicroSandboxConfig feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/ut/resources/fc/impl/client_test.ts (1)
271-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negative-path test for
updateFunction, mirroringcreateFunction.
createFunctiontests cover both "withmicroSandboxConfig" and "withoutmicroSandboxConfig" cases, butupdateFunctiononly tests the positive path. Add a symmetric test assertingbodyMap.microSandboxConfigisundefinedwhen omitted, for consistent coverage of the request-forwarding contract.♻️ Suggested addition
it('should forward microSandboxConfig to the update request body', async () => { ... }); + + it('should not set microSandboxConfig on update when it is not provided', async () => { + const updateFunctionWithOptions = jest.fn().mockResolvedValue({} as any); + Object.defineProperty(client, 'fc20230330Client', { + value: { updateFunctionWithOptions }, + writable: true, + }); + + await client.updateFunction({ + functionName: 'test-function', + runtime: 'nodejs18', + } as IFunction); + + const bodyMap = updateFunctionWithOptions.mock.calls[0][1].body.toMap(); + expect(bodyMap.microSandboxConfig).toBeUndefined(); + }); });🤖 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__/ut/resources/fc/impl/client_test.ts` around lines 271 - 313, Add a negative-path test for updateFunction to match the createFunction coverage: the current test only verifies microSandboxConfig is forwarded when present, but there is no assertion for the omitted case. Add a sibling test in the updateFunction describe block that calls FC_Client.updateFunction without microSandboxConfig, inspects the mocked updateFunctionWithOptions request body, and asserts bodyMap.microSandboxConfig is undefined to keep the request-forwarding contract consistent.
🤖 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.
Nitpick comments:
In `@__tests__/ut/resources/fc/impl/client_test.ts`:
- Around line 271-313: Add a negative-path test for updateFunction to match the
createFunction coverage: the current test only verifies microSandboxConfig is
forwarded when present, but there is no assertion for the omitted case. Add a
sibling test in the updateFunction describe block that calls
FC_Client.updateFunction without microSandboxConfig, inspects the mocked
updateFunctionWithOptions request body, and asserts bodyMap.microSandboxConfig
is undefined to keep the request-forwarding contract consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de752600-a5df-45e3-8e60-51121bff0e67
📒 Files selected for processing (2)
__tests__/ut/local/local_test.ts__tests__/ut/resources/fc/impl/client_test.ts
15897f2 to
8ed590f
Compare
Summary by CodeRabbit