feat: adding network policies oo concepts#680
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI finished reviewing your PR. |
|
Smoke tests fail as this is not yet in prod |
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 86.33% | 100% | ❌ |
| Lines | 79.31% | - | ℹ️ |
| Branches | 45.91% | - | ℹ️ |
| Statements | 78.08% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 88.19% | 100% | ❌ |
| Lines | 80.65% | - | ℹ️ |
| Branches | 45.91% | - | ℹ️ |
| Statements | 79.37% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 88.19% | 100% | ❌ |
| Lines | 80.65% | - | ℹ️ |
| Branches | 45.91% | - | ℹ️ |
| Statements | 79.37% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 89.44% | 100% | ❌ |
| Lines | 82.37% | - | ℹ️ |
| Branches | 54.08% | - | ℹ️ |
| Statements | 81.03% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 93.78% | 100% | ❌ |
| Lines | 83.9% | - | ℹ️ |
| Branches | 52.83% | - | ℹ️ |
| Statements | 82.5% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
✅ Object Smoke Tests & Coverage ReportTest Results✅ All smoke tests passed Coverage Results
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests) ✅ All tests passed and all object methods are covered! View detailed coverage reportCoverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%. |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
| test( | ||
| 'create network policy', | ||
| async () => { | ||
| policy = await sdk.networkPolicy.create({ | ||
| name: uniqueName('sdk-network-policy'), | ||
| allow_all: false, | ||
| allowed_hostnames: ['github.com', '*.npmjs.org'], | ||
| allow_devbox_to_devbox: false, | ||
| description: 'Test network policy', | ||
| }); | ||
| expect(policy).toBeDefined(); | ||
| expect(policy.id).toBeTruthy(); | ||
| policyId = policy.id; |
There was a problem hiding this comment.
Suggestion: The lifecycle tests rely on the "create network policy" test to initialize shared state (policy and policyId), so running any of the later tests in isolation (e.g., via a focused test.only) will cause them to dereference an undefined policy, making the suite order-dependent and flaky; moving the creation into a beforeAll and letting the test only assert on that state makes each test robust. [logic error]
Severity Level: Critical 🚨
- ❌ CI flaky tests failing when tests run in isolation.
- ⚠️ Developer productivity slowed by ordering-dependent tests.
- ⚠️ Smoke test suite unstable across parallel runners.| test( | |
| 'create network policy', | |
| async () => { | |
| policy = await sdk.networkPolicy.create({ | |
| name: uniqueName('sdk-network-policy'), | |
| allow_all: false, | |
| allowed_hostnames: ['github.com', '*.npmjs.org'], | |
| allow_devbox_to_devbox: false, | |
| description: 'Test network policy', | |
| }); | |
| expect(policy).toBeDefined(); | |
| expect(policy.id).toBeTruthy(); | |
| policyId = policy.id; | |
| beforeAll(async () => { | |
| policy = await sdk.networkPolicy.create({ | |
| name: uniqueName('sdk-network-policy'), | |
| allow_all: false, | |
| allowed_hostnames: ['github.com', '*.npmjs.org'], | |
| allow_devbox_to_devbox: false, | |
| description: 'Test network policy', | |
| }); | |
| policyId = policy.id; | |
| }); | |
| test( | |
| 'create network policy', | |
| async () => { | |
| expect(policy).toBeDefined(); | |
| expect(policy!.id).toBeTruthy(); |
Steps of Reproduction ✅
1. Run the test file focused on a later test (e.g., use `test.only` on `get network policy
info`) in `tests/smoketests/object-oriented/network-policy.test.ts:40` (the "get network
policy info" test). The file expects `policy` to be defined by the "create network policy"
test at `:23-38`. Since create wasn't executed, `policy` is undefined and the test
dereferences it at `:42` (`const info = await policy!.getInfo();`), causing an immediate
failure.
2. Reproduce locally by editing `tests/smoketests/object-oriented/network-policy.test.ts`
to add `.only` to any test after the create test (for example line 40) and run `yarn test
tests/smoketests/object-oriented/network-policy.test.ts`. Observe the failure originates
from the missing creation step referenced above.
3. Confirm in CI by configuring the runner to run a single test file or using
`--testNamePattern` to execute one of the later tests; the suite will fail because shared
state initialization is inside a different test instead of test lifecycle hooks
(`beforeAll`).
4. The proposed improvement moves creation into `beforeAll` (test file top-level lifecycle
at the same file) so each test in the suite can assume `policy` is present even when tests
are executed in isolation. This directly removes the order dependency demonstrated in
steps 1-3.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** tests/smoketests/object-oriented/network-policy.test.ts
**Line:** 23:35
**Comment:**
*Logic Error: The lifecycle tests rely on the "create network policy" test to initialize shared state (`policy` and `policyId`), so running any of the later tests in isolation (e.g., via a focused `test.only`) will cause them to dereference an undefined `policy`, making the suite order-dependent and flaky; moving the creation into a `beforeAll` and letting the test only assert on that state makes each test robust.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
this is a flaw on many of the tests
|
CodeAnt AI Incremental review completed. |
| let attempts = 0; | ||
|
|
||
| while (attempts < maxAttempts!) { | ||
| while (maxAttempts === undefined || attempts < maxAttempts) { |
There was a problem hiding this comment.
when would maxAttempts be undefined? If it is, can we set a default on it
| try { | ||
| await policy.getInfo(); | ||
| // Policy still exists, delete it | ||
| await policy.delete(); | ||
| } catch { | ||
| // Policy already deleted, ignore | ||
| } |
There was a problem hiding this comment.
can we abstract this into a helper function? like cleanUpPolicy or something
| }); | ||
| }); | ||
|
|
||
| describe('devbox creation edge cases', () => { |
There was a problem hiding this comment.
need a devbox test with network policy id
| dockerfile: 'FROM ubuntu:20.04\nRUN apt-get update && apt-get install -y curl', | ||
| system_setup_commands: ['echo "Blueprint setup complete"'], | ||
| }); | ||
| blueprint = await sdk.blueprint.create( |
There was a problem hiding this comment.
need bpt tests with network_policy_id for build and also for launch_parameters
| test( | ||
| 'create network policy', | ||
| async () => { | ||
| policy = await sdk.networkPolicy.create({ | ||
| name: uniqueName('sdk-network-policy'), | ||
| allow_all: false, | ||
| allowed_hostnames: ['github.com', '*.npmjs.org'], | ||
| allow_devbox_to_devbox: false, | ||
| description: 'Test network policy', | ||
| }); | ||
| expect(policy).toBeDefined(); | ||
| expect(policy.id).toBeTruthy(); | ||
| policyId = policy.id; |
There was a problem hiding this comment.
this is a flaw on many of the tests
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 98.75% | 100% | ❌ |
| Lines | 86.97% | - | ℹ️ |
| Branches | 65.4% | - | ℹ️ |
| Statements | 85.45% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
✅ Object Smoke Tests & Coverage ReportTest Results✅ All smoke tests passed Coverage Results
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests) ✅ All tests passed and all object methods are covered! View detailed coverage reportCoverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%. |
✅ Object Smoke Tests & Coverage ReportTest Results✅ All smoke tests passed Coverage Results
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests) ✅ All tests passed and all object methods are covered! View detailed coverage reportCoverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%. |
|
| Metric | Coverage | Required | Status |
|---|---|---|---|
| Functions | 99.37% | 100% | ❌ |
| Lines | 87.54% | - | ℹ️ |
| Branches | 65.4% | - | ℹ️ |
| Statements | 86% | - | ℹ️ |
Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)
View detailed coverage report
Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.
CodeAnt-AI Description
Add NetworkPolicy object API and improve SDK smoke tests and polling behavior
What Changed
Impact
✅ Manage network policies from the SDK✅ Reliable blueprint builds with longer polling for slow builds✅ Fewer manual steps to mount storage objects and configure devbox/network policies💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.