feat: improve test coverage to 80% and fix CI/CD issues#24
Conversation
…h context support
…d mocking os.Open
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds context propagation and Close semantics across the Libvirt client, adapter, and mocks; threads context through adapter helpers and tests; standardizes handler error responses and expands request structs; adds extensive handler error-path tests; downgrades several GitHub Actions versions and updates CI workflow actions. Changes
Sequence Diagram(s)sequenceDiagram
participant API as HTTP Handler
participant Adapter as LibvirtAdapter
participant Client as LibvirtClient
participant Libvirt as libvirt daemon
API->>Adapter: HTTP request (ctx)
Adapter->>Client: method(ctx, ...)
Client->>Libvirt: libvirt call (cancellable)
alt libvirt success
Libvirt-->>Client: result
Client-->>Adapter: result
Adapter-->>API: HTTP 2xx response
else context canceled / error
Libvirt-->>Client: error / context.Canceled
Client-->>Adapter: propagate error
Adapter-->>API: map to HTTP error (4xx/5xx)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
This PR significantly improves test coverage across multiple handlers and repositories to meet the 80% project target. It also addresses security hotspots in the libvirt adapter by adding proper context propagation and cancellation handling, fixes CI/CD workflow version compatibility issues, and improves error handling consistency across HTTP handlers.
Changes:
- Added comprehensive test coverage for noop repositories and libvirt adapter with context cancellation tests
- Enhanced error handling in handlers with proper error messages and consistent patterns
- Fixed CI/CD workflow version issues by downgrading to compatible versions (v4/v6 instead of v6/v9)
- Added context propagation throughout libvirt client operations for proper cancellation support
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/repositories/noop/repos_extra_test.go | New comprehensive tests for noop repository implementations |
| internal/repositories/libvirt/real_client.go | Added context parameter to all operations for proper cancellation |
| internal/repositories/libvirt/mock_client_test.go | Updated mock to support context parameters |
| internal/repositories/libvirt/libvirt_client.go | Updated interface to accept context in all methods |
| internal/repositories/libvirt/lb_proxy.go | Extracted constants and made exec commands testable |
| internal/repositories/libvirt/adapter*.go | Added context propagation and new test files with extensive coverage |
| internal/handlers/*_handler_test.go | Added error path tests and improved coverage |
| internal/handlers/*_handler.go | Improved error messages with consistent patterns |
| .github/workflows/*.yml | Downgraded GitHub Actions versions to compatible releases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/cloud/cli_commands_test.go (1)
65-92: Handle JSON encode errors instead of discarding them.
_ = json.NewEncoder(w).Encode(resp)silently drops errors, which conflicts with the “no silent failures” guideline. Please check and handle the error (e.g.,http.Erroror setStatusInternalServerError) and apply the same pattern across the other mock handlers in this file.♻️ Example fix
- _ = json.NewEncoder(w).Encode(resp) + if err := json.NewEncoder(w).Encode(resp); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return true + }internal/handlers/rbac_handler.go (1)
31-35: Addomitemptyon optional JSON fields.
DescriptionandPermissionsare optional but currently serialize empty values; addomitemptyto align with the JSON-tag guidance.♻️ Suggested fix
type CreateRoleRequest struct { Name string `json:"name" binding:"required"` - Description string `json:"description"` - Permissions []domain.Permission `json:"permissions"` + Description string `json:"description,omitempty"` + Permissions []domain.Permission `json:"permissions,omitempty"` }As per coding guidelines, optional fields should include
omitemptyinjsontags.internal/handlers/stack_handler.go (1)
104-118: Swagger documentation doesn't match implementation.The Swagger comment specifies
@Success 204but the implementation returnshttp.StatusOK(200). Align the documentation with the implementation or vice versa.🔧 Option 1: Update Swagger to match implementation
-// `@Success` 204 +// `@Success` 200 {object} map[string]string🔧 Option 2: Update implementation to match Swagger
- httputil.Success(c, http.StatusOK, gin.H{"message": "stack deleted"}) + c.Status(http.StatusNoContent)
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 57-61: The workflow step named "Upload Coverage to Codecov" is
pinned to the old uses: codecov/codecov-action@v4; update that to the v5 series
(e.g., uses: codecov/codecov-action@v5 or the specific latest tag like `@v5.5.2`)
in the CI job so the action receives the latest fixes and security updates, and
verify any v5-required input name changes for fields like files or
fail_ci_if_error while making the update.
In @.github/workflows/load-tests.yml:
- Around line 79-83: Replace the deprecated grafana/k6-action@v0.3.0 usage with
the maintained setup and run actions: use grafana/setup-k6-action (v0.3.1) to
install k6 and then grafana/run-k6-action to execute the test; pass the existing
inputs (filename and flags) to the run action so tests/load/api-smoke.js and
flags --vus 10 --duration 30s continue to be used. Locate the block referencing
grafana/k6-action and swap it for a two-step sequence using
grafana/setup-k6-action and grafana/run-k6-action, ensuring versions are updated
to the latest stable release (v0.3.1) and that filename and flags are forwarded
to the run action.
In `@internal/handlers/gateway_handler_test.go`:
- Around line 153-220: The tests TestGatewayHandlerProxySuccess,
TestGatewayHandlerProxyWithoutSlash and TestGatewayHandlerProxyWithSlash
currently mock GetProxy to return a ReverseProxy with only a Director, causing
real network dials and StatusBadGateway; update the mocks to return a
ReverseProxy that targets the httptest.Server (use
httputil.NewSingleHostReverseProxy with the test server's URL parsed via
net/url) OR set a custom Transport on the returned *httputil.ReverseProxy that
routes requests to the test server (or a stub RoundTripper) so the proxy request
succeeds and the success path is exercised; modify the svc.On("GetProxy", ...)
returns accordingly for the functions in these tests.
- Around line 257-279: In TestGatewayHandlerProxyParamWithoutSlash, the returned
ReverseProxy has an empty Director causing nondeterministic dialing; create an
httptest.NewServer that returns 200 and configure the mock GetProxy (on
mockGatewayService) to return a ReverseProxy whose Director sets req.URL.Scheme,
req.URL.Host and req.URL.Path to route to that test server (mirror the setup in
TestGatewayHandlerProxyWithoutSlash/TestGatewayHandlerProxyWithSlash), and then
assert the deterministic expected status (adjust the assert to expect
http.StatusOK if the test server returns 200, or http.StatusBadGateway only if
you intentionally simulate a failed transport).
In `@internal/handlers/image_handler_test.go`:
- Line 261: The test uses an undefined variable pathInvalid in the DELETE
request (c.Request = httptest.NewRequest("DELETE", imagesAPI+pathInvalid, nil)),
causing build failure; define pathInvalid before use (e.g., create pathInvalid
:= "<invalid-path>" or reuse the existing invalid-path variable/name used
earlier around line 154) or replace the reference with the proper literal
invalid path used elsewhere in the test so imagesAPI+pathInvalid is valid;
ensure the variable name matches the one used in other test cases (e.g.,
pathInvalid) and is declared in the same scope as the c.Request call.
- Line 154: The test uses an undefined identifier pathInvalid in the
httptest.NewRequest call (alongside imagesAPI and uploadSuffix); define
pathInvalid as the intended test path constant (e.g., const pathInvalid =
"/invalid" or whatever the test expects) near the other constants, or replace
the identifier inline with the literal string value to match the test scenario
so the call in the test function (the line constructing the POST request)
compiles.
In `@internal/handlers/rbac_handler_test.go`:
- Around line 20-22: The test constant bindPath is incorrect: update the
bindPath constant used in rbac_handler_test.go from "/rbac/bind" to
"/rbac/bindings" so it matches the handler's actual route; locate the bindPath
variable declaration (near rolesPath and testRoleName) and change its value to
"/rbac/bindings" so tests exercise the real routing contract exposed by the
handlers.
In `@internal/handlers/stack_handler_test.go`:
- Around line 179-181: Remove the redundant duplicate assertion in the test:
there are two identical assert.Equal(t, http.StatusOK, w.Code) calls
back-to-back; keep a single assert.Equal(t, http.StatusOK, w.Code) and remove
the duplicate so the test ends with svc.AssertExpectations(t) after one status
check.
In `@internal/handlers/storage_handler_test.go`:
- Around line 188-226: Each error-path subtest (the t.Run blocks named
"UploadServiceError", "DownloadServiceError", "ListServiceError", and
"DeleteServiceError") is missing mock expectation verification; add a defer
svc.AssertExpectations(t) immediately after the svc, handler, r := setup(t) line
in each subtest so that expectations set on the mock service (methods Upload,
Download, ListObjects, DeleteObject) are asserted when the subtest finishes.
In `@internal/repositories/libvirt/adapter_test.go`:
- Around line 160-166: The mock for execCommandContext in
TestLBProxyAdapterOperations still calls the external "true" binary; replace it
with a hermetic test helper process that doesn't depend on PATH by returning an
*exec.Cmd that re-runs the test binary and triggers a TestHelperProcess helper.
Concretely, change the mock to return exec.CommandContext(ctx, os.Args[0],
"-test.run=TestHelperProcess", "--", name, strings.Join(args, " ")) with an env
var like "TEST_HELPER_PROCESS=1", and add a TestHelperProcess(t *testing.T)
function in the test file that checks the env var and exits 0 (no-op) or
simulates desired stdout/stderr/exit codes; keep using the execCommandContext
symbol and ensure TestHelperProcess is used only by tests.
In `@internal/repositories/libvirt/adapter_volume_test.go`:
- Around line 98-105: The test override of mkdirTemp currently ignores errors
from os.WriteFile; update the anonymous function assigned to mkdirTemp in
adapter_volume_test.go to capture the write error (e.g., writeErr :=
os.WriteFile(...)) and if writeErr != nil return the created temp dir and
writeErr (or wrap it) so callers see the failure instead of silently proceeding;
keep the existing defer restoring oldMkdir and ensure the function still returns
the original os.MkdirTemp error if that occurs.
- Around line 64-96: These snapshot tests (TestCreateVolumeSnapshotSuccess and
TestRestoreVolumeSnapshotSuccess) rely on /usr/bin/true via the execCommand stub
and should be skipped on non-Linux platforms; at the start of each test (or in a
shared test helper invoked by both tests) check runtime.GOOS and call
t.Skip("Snapshot tests require Linux with QEMU/KVM") when not "linux", so the
tests are gracefully skipped on unsupported platforms while leaving the existing
execCommand stub and mocks (execCommand, TestCreateVolumeSnapshotSuccess,
TestRestoreVolumeSnapshotSuccess) unchanged.
In `@internal/repositories/libvirt/real_client.go`:
- Around line 166-173: The conversion of needResults (uint32) to int32 in
RealLibvirtClient.NetworkGetDhcpLeases can overflow; before calling
r.conn.NetworkGetDhcpLeases validate needResults against math.MaxInt32 and
return a clear error if it exceeds MaxInt32 (or alternatively clamp to MaxInt32
if preferred), then safely cast to int32 for the call; update the function
signature only if you decide to change the API type instead of the bounds check.
🧹 Nitpick comments (27)
internal/handlers/snapshot_handler_test.go (2)
60-69: Consider more descriptive constant names.The constants improve maintainability by avoiding magic strings, but some names are cryptic (e.g.,
testSSnap,testSpath,testSpathID). More descriptive names would improve readability.💡 Suggested naming improvements
const ( - testSSnap = "test snapshot" - testSpath = "/snapshots" - testSpathID = "/snapshots/" - testSRestore = "/restore" - testVolRest = "restored-volume" - snapPathInvalid = "invalid" - snapHeaderContentType = "Content-Type" - snapAppJSON = "application/json" + testSnapshotDescription = "test snapshot" + testSnapshotsPath = "/snapshots" + testSnapshotsPathWithID = "/snapshots/" + testRestorePath = "/restore" + testRestoredVolumeName = "restored-volume" + testInvalidID = "invalid" + headerContentType = "Content-Type" + contentTypeJSON = "application/json" )
188-194: Remove unused parameter from setup function.The
_ *testing.Tparameter is unused. Either remove it or use it for test helper functionality.💡 Proposed fix
- setup := func(_ *testing.T) (*mockSnapshotService, *SnapshotHandler, *gin.Engine) { + setup := func() (*mockSnapshotService, *SnapshotHandler, *gin.Engine) { svc := new(mockSnapshotService) handler := NewSnapshotHandler(svc) r := gin.New() return svc, handler, r }Then update all calls from
setup(t)tosetup().internal/repositories/noop/repos_extra_test.go (1)
12-15: Consider renaminguIDtouserIDfor clarity.The variable
uIDcould be more descriptive. Since it's used as a user identifier in methods likeListByUserID, renaming it touserIDwould improve readability.Suggested change
func TestNoopRepositoriesExtra(t *testing.T) { ctx := context.Background() - uID := uuid.New() + userID := uuid.New() id := uuid.New()Then update all usages of
uIDtouserIDthroughout the file.internal/handlers/security_group_handler_test.go (1)
465-470: Addgin.SetMode(gin.TestMode)and uset.Helper()for consistency.The helper function doesn't set gin test mode, but other tests in this file (lines 84, 111, 131, etc.) explicitly call
gin.SetMode(gin.TestMode). This inconsistency means tests using this helper will run with verbose debug logging. Additionally, thetparameter is unused—either remove it or use it to callt.Helper()for cleaner test failure stack traces.♻️ Suggested improvement
-func setupSecurityGroupHandlerTest(_ *testing.T) (*mockSecurityGroupService, *SecurityGroupHandler, *gin.Engine) { +func setupSecurityGroupHandlerTest(t *testing.T) (*mockSecurityGroupService, *SecurityGroupHandler, *gin.Engine) { + t.Helper() + gin.SetMode(gin.TestMode) svc := new(mockSecurityGroupService) handler := NewSecurityGroupHandler(svc) r := gin.New() return svc, handler, r }internal/handlers/storage_handler_test.go (2)
158-163: Reuse existingsetupStorageHandlerTesthelper.This duplicates
setupStorageHandlerTest(lines 80-86) but omitsgin.SetMode(gin.TestMode), which suppresses verbose debug output during tests. Reuse the existing helper for consistency.♻️ Suggested refactor
func TestStorageHandlerErrorPaths(t *testing.T) { - setup := func(_ *testing.T) (*mockStorageService, *StorageHandler, *gin.Engine) { - svc := new(mockStorageService) - handler := NewStorageHandler(svc) - r := gin.New() - return svc, handler, r - } - t.Run("UploadMissingBucketOrKey", func(t *testing.T) { - _, handler, r := setup(t) + _, handler, r := setupStorageHandlerTest(t)Apply the same change to all subtests that call
setup(t).
169-178: Consider condensing verbose comments.The explanation is helpful but overly detailed for a test file. A single concise comment would suffice.
✏️ Suggested condensed comment
- // This URL matching might be tricky with params if one is empty. - // Actually if I send /storage/b1 without key, it won't match :key path segment usually. - // But handler logic says: bucket := c.Param("bucket"), key := c.Param("key"). - // If I use a route like /storage/:bucket/*key, maybe? - // The existing route is /storage/:bucket/:key. - // If I request /storage/b1/ then key is empty? - // Let's try explicit empty key if possible or verify param logic. - // Actually if Params are missing from context (e.g. if I don't use router but call handler directly), it fails. - // But using router, if I request /storage/b1/, it might match if trailing slash enabled? - // Let's rely on manually setting params for this specific test to ensure we hit the check. + // Bypass router and set params directly to test empty key validation.internal/handlers/image_handler_test.go (2)
344-346: Avoid silent failures with blank identifier assignments.Per coding guidelines, blank identifier assignments like
_, _ = part.Write(...)and_ = writer.Close()should be avoided. In tests, setup failures should cause early test termination to prevent false results.♻️ Proposed fix
- _, _ = part.Write([]byte("image content")) - _ = writer.Close() + _, err := part.Write([]byte("image content")) + assert.NoError(t, err) + err = writer.Close() + assert.NoError(t, err)As per coding guidelines, silent failures should be avoided in test files.
359-364: Consider marking as test helper and using consistently.The
*testing.Tparameter is unused. Addingt.Helper()improves stack traces when tests fail. Also consider using this helper more consistently across other test functions to reduce duplication.♻️ Proposed enhancement
-func setupImageHandlerTest(_ *testing.T) (*mockImageService, *ImageHandler, *gin.Engine) { +func setupImageHandlerTest(t *testing.T) (*mockImageService, *ImageHandler, *gin.Engine) { + t.Helper() svc := new(mockImageService) handler := NewImageHandler(svc) r := gin.New() return svc, handler, r }internal/handlers/secret_handler_test.go (1)
149-197: Prefer a table-driven structure for these subtests.This block is already using subtests, but the repeated setup and assertions are a great fit for a single table-driven loop to reduce duplication and keep adding cases cheap. As per coding guidelines.
♻️ Suggested refactor (table-driven)
func TestSecretHandlerDelete(t *testing.T) { - t.Run("SuccessByID", func(t *testing.T) { - svc, handler, r := setupSecretHandlerTest(t) - r.DELETE(secretsPath+"/:id", handler.Delete) - id := uuid.New() - svc.On("DeleteSecret", mock.Anything, id).Return(nil) - req, _ := http.NewRequest(http.MethodDelete, secretsPath+"/"+id.String(), nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - svc.AssertExpectations(t) - }) - - t.Run("SuccessByName", func(t *testing.T) { - svc, handler, r := setupSecretHandlerTest(t) - r.DELETE(secretsPath+"/:id", handler.Delete) - id := uuid.New() - svc.On("GetSecretByName", mock.Anything, testSecretName).Return(&domain.Secret{ID: id}, nil) - svc.On("DeleteSecret", mock.Anything, id).Return(nil) - req, _ := http.NewRequest(http.MethodDelete, secretsPath+"/"+testSecretName, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - svc.AssertExpectations(t) - }) - - t.Run("GetByNameError", func(t *testing.T) { - svc, handler, r := setupSecretHandlerTest(t) - r.DELETE(secretsPath+"/:id", handler.Delete) - svc.On("GetSecretByName", mock.Anything, testSecretName).Return(nil, errors.New(errors.NotFound, errNotFound)) - req, _ := http.NewRequest(http.MethodDelete, secretsPath+"/"+testSecretName, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusNotFound, w.Code) - svc.AssertExpectations(t) - }) - - t.Run("DeleteError", func(t *testing.T) { - svc, handler, r := setupSecretHandlerTest(t) - r.DELETE(secretsPath+"/:id", handler.Delete) - id := uuid.New() - svc.On("DeleteSecret", mock.Anything, id).Return(errors.New(errors.Internal, "internal error")) - req, _ := http.NewRequest(http.MethodDelete, secretsPath+"/"+id.String(), nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusInternalServerError, w.Code) - svc.AssertExpectations(t) - }) + cases := []struct { + name string + makeURL func(id uuid.UUID) string + setup func(svc *mockSecretService, id uuid.UUID) + want int + }{ + { + name: "SuccessByID", + makeURL: func(id uuid.UUID) string { return secretsPath + "/" + id.String() }, + setup: func(svc *mockSecretService, id uuid.UUID) { + svc.On("DeleteSecret", mock.Anything, id).Return(nil) + }, + want: http.StatusOK, + }, + { + name: "SuccessByName", + makeURL: func(_ uuid.UUID) string { return secretsPath + "/" + testSecretName }, + setup: func(svc *mockSecretService, id uuid.UUID) { + svc.On("GetSecretByName", mock.Anything, testSecretName).Return(&domain.Secret{ID: id}, nil) + svc.On("DeleteSecret", mock.Anything, id).Return(nil) + }, + want: http.StatusOK, + }, + { + name: "GetByNameError", + makeURL: func(_ uuid.UUID) string { return secretsPath + "/" + testSecretName }, + setup: func(svc *mockSecretService, _ uuid.UUID) { + svc.On("GetSecretByName", mock.Anything, testSecretName).Return(nil, errors.New(errors.NotFound, errNotFound)) + }, + want: http.StatusNotFound, + }, + { + name: "DeleteError", + makeURL: func(id uuid.UUID) string { return secretsPath + "/" + id.String() }, + setup: func(svc *mockSecretService, id uuid.UUID) { + svc.On("DeleteSecret", mock.Anything, id).Return(errors.New(errors.Internal, "internal error")) + }, + want: http.StatusInternalServerError, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + svc, handler, r := setupSecretHandlerTest(t) + r.DELETE(secretsPath+"/:id", handler.Delete) + id := uuid.New() + tc.setup(svc, id) + req, _ := http.NewRequest(http.MethodDelete, tc.makeURL(id), nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + assert.Equal(t, tc.want, w.Code) + svc.AssertExpectations(t) + }) + } }internal/handlers/rbac_handler_test.go (1)
314-497: Prefer a table-driven structure for error-path tests.This block is a good candidate for a table of
{name, method, path, setupMocks, body, wantStatus}to reduce repetition and keep additions consistent.As per coding guidelines, use table-driven tests in
*_test.gofiles.internal/handlers/notify_handler_test.go (1)
215-371: Consolidate error subtests into table-driven cases and assert mocks.
These blocks repeat setup/route wiring. A table-driven structure reduces duplication and makes it easy to addsvc.AssertExpectations(t)in cases that set expectations. As per coding guidelines, use table-driven tests in *_test.go.♻️ Example refactor for TestNotifyHandlerTopicErrors
func TestNotifyHandlerTopicErrors(t *testing.T) { - t.Run("CreateInvalidJSON", func(t *testing.T) { - _, handler, r := setupNotifyHandlerTest(t) - r.POST(topicsPath, handler.CreateTopic) - req, _ := http.NewRequest("POST", topicsPath, bytes.NewBufferString("invalid")) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusBadRequest, w.Code) - }) - - t.Run("CreateServiceError", func(t *testing.T) { - svc, handler, r := setupNotifyHandlerTest(t) - r.POST(topicsPath, handler.CreateTopic) - svc.On("CreateTopic", mock.Anything, mock.Anything).Return(nil, errors.New(errors.Internal, "error")) - body, _ := json.Marshal(map[string]interface{}{"name": "n"}) - req, _ := http.NewRequest("POST", topicsPath, bytes.NewBuffer(body)) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusInternalServerError, w.Code) - }) - // ... remaining subtests ... + deleteID := uuid.New() + cases := []struct { + name string + register func(*gin.Engine, *NotifyHandler) + setup func(*mockNotifyService) + request func() *http.Request + want int + }{ + { + name: "CreateInvalidJSON", + register: func(r *gin.Engine, h *NotifyHandler) { r.POST(topicsPath, h.CreateTopic) }, + request: func() *http.Request { req, _ := http.NewRequest("POST", topicsPath, bytes.NewBufferString("invalid")); return req }, + want: http.StatusBadRequest, + }, + { + name: "CreateServiceError", + register: func(r *gin.Engine, h *NotifyHandler) { r.POST(topicsPath, h.CreateTopic) }, + setup: func(svc *mockNotifyService) { svc.On("CreateTopic", mock.Anything, mock.Anything).Return(nil, errors.New(errors.Internal, "error")) }, + request: func() *http.Request { body, _ := json.Marshal(map[string]interface{}{"name": "n"}); req, _ := http.NewRequest("POST", topicsPath, bytes.NewBuffer(body)); return req }, + want: http.StatusInternalServerError, + }, + { + name: "ListServiceError", + register: func(r *gin.Engine, h *NotifyHandler) { r.GET(topicsPath, h.ListTopics) }, + setup: func(svc *mockNotifyService) { svc.On("ListTopics", mock.Anything).Return(nil, errors.New(errors.Internal, "error")) }, + request: func() *http.Request { req, _ := http.NewRequest("GET", topicsPath, nil); return req }, + want: http.StatusInternalServerError, + }, + { + name: "DeleteInvalidID", + register: func(r *gin.Engine, h *NotifyHandler) { r.DELETE(topicsPath+"/:id", h.DeleteTopic) }, + request: func() *http.Request { req, _ := http.NewRequest("DELETE", topicsPath+pathInvalid, nil); return req }, + want: http.StatusBadRequest, + }, + { + name: "DeleteServiceError", + register: func(r *gin.Engine, h *NotifyHandler) { r.DELETE(topicsPath+"/:id", h.DeleteTopic) }, + setup: func(svc *mockNotifyService) { svc.On("DeleteTopic", mock.Anything, deleteID).Return(errors.New(errors.Internal, "error")) }, + request: func() *http.Request { req, _ := http.NewRequest("DELETE", topicsPath+"/"+deleteID.String(), nil); return req }, + want: http.StatusInternalServerError, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + svc, handler, r := setupNotifyHandlerTest(t) + tc.register(r, handler) + if tc.setup != nil { + tc.setup(svc) + defer svc.AssertExpectations(t) + } + w := httptest.NewRecorder() + r.ServeHTTP(w, tc.request()) + assert.Equal(t, tc.want, w.Code) + }) + } }internal/handlers/gateway_handler_test.go (1)
222-244: Prefer table-driven subtests for error cases.These error-path tests are already using
t.Run; consolidating into a table-driven structure would reduce duplication and align with the project’s testing guideline.As per coding guidelines, use table-driven tests in test files.
Also applies to: 282-303
internal/handlers/subnet_handler_test.go (1)
135-160: Consider using typed errors frominternal/errorsfor consistency.This test uses
assert.AnErrorwhich works for coverage, but other handler tests (e.g.,volume_handler_test.go,function_handler_test.go) useerrors.New(errors.Internal, "...")from theinternal/errorspackage. Using typed errors ensures the error-to-HTTP-status mapping is properly exercised.♻️ Suggested improvement
+ "github.com/poyrazk/thecloud/internal/errors" "github.com/stretchr/testify/assert" ... - svc.On("CreateSubnet", mock.Anything, vpcID, testSubnetName, testutil.TestSubnetCIDR, mock.Anything).Return(nil, assert.AnError) + svc.On("CreateSubnet", mock.Anything, vpcID, testSubnetName, testutil.TestSubnetCIDR, mock.Anything).Return(nil, errors.New(errors.Internal, "service error"))internal/handlers/function_handler_test.go (1)
218-229: Blank identifier assignments in test setup.Lines 220-223 use
_ = writer.WriteField(...)which suppresses errors. While unlikely to fail in tests, this violates the coding guideline against silent failures. Consider usingrequire.NoErrorfor consistency with other tests in the file that do handle errors.♻️ Suggested improvement
+ require.NoError(t, writer.WriteField("name", "n")) + require.NoError(t, writer.WriteField("runtime", "r")) + require.NoError(t, writer.WriteField("handler", "h")) + require.NoError(t, writer.Close()) - _ = writer.WriteField("name", "n") - _ = writer.WriteField("runtime", "r") - _ = writer.WriteField("handler", "h") - _ = writer.Close()internal/handlers/stack_handler.go (1)
28-33: Consider addingomitemptyto the optionalParametersfield.Per coding guidelines, optional fields should include
omitemptyin their JSON tags. TheParametersfield is optional (nobinding:"required") but lacksomitempty.♻️ Suggested improvement
type CreateStackRequest struct { Name string `json:"name" binding:"required"` Template string `json:"template" binding:"required"` - Parameters map[string]string `json:"parameters"` + Parameters map[string]string `json:"parameters,omitempty"` }internal/repositories/libvirt/lb_proxy.go (1)
65-73: Silent failures during cleanup may mask issues.Lines 69 and 72 use blank identifiers to ignore errors. While cleanup code often uses best-effort semantics, silently ignoring failures violates the coding guideline and may mask debugging-relevant issues.
Consider logging these errors at debug/warn level instead of ignoring them completely.
♻️ Suggested improvement
if _, err := os.Stat(pidPath); err == nil { // Stop nginx: nginx -s stop -c ... configPath := filepath.Join(configDir, nginxConfigFileName) cmd := execCommandContext(ctx, "nginx", "-c", configPath, "-g", fmt.Sprintf("pid %s;", pidPath), "-s", "stop") - _ = cmd.Run() + if err := cmd.Run(); err != nil { + // Best-effort: log but continue with cleanup + // Consider adding logging here + } } - _ = os.RemoveAll(configDir) + if err := os.RemoveAll(configDir); err != nil { + // Best-effort: log but continue + // Consider adding logging here + } return nilinternal/handlers/container_handler_test.go (1)
170-282: Consolidate error-path cases into a table-driven suite.These error tests repeat a lot of setup and assertions; a table-driven list of cases would reduce duplication and make adding new cases easier. As per coding guidelines, consider table-driven tests in *_test.go files.
internal/handlers/database_handler_test.go (1)
170-272: Refactor error tests into table-driven cases.The error-path tests follow the same pattern; a table-driven approach would shrink duplication and improve readability. As per coding guidelines, consider table-driven tests in *_test.go files.
internal/handlers/cron_handler_test.go (2)
139-171: Use pause/resume suffix constants in request URLs for consistency.This avoids divergence between routes and request construction.
Suggested update
- req, err := http.NewRequest(http.MethodPost, cronPath+"/"+id.String()+"/pause", nil) + req, err := http.NewRequest(http.MethodPost, cronPath+"/"+id.String()+pauseSuffix, nil) ... - req, err := http.NewRequest(http.MethodPost, cronPath+"/"+id.String()+"/resume", nil) + req, err := http.NewRequest(http.MethodPost, cronPath+"/"+id.String()+resumeSuffix, nil)
190-315: Consider a table-driven structure for error tests.A table of cases (invalid JSON/ID, service error, not found) would reduce repeated setup and assertions. As per coding guidelines, consider table-driven tests in *_test.go files.
internal/repositories/libvirt/adapter_lifecycle_test.go (1)
337-358: Assert mock expectations in RunTask failure tests.Without
AssertExpectations, the test won’t fail if expected calls aren’t made.Suggested update
func TestRunTaskFailures(t *testing.T) { m := new(MockLibvirtClient) a := newTestAdapter(m) ctx := context.Background() @@ m.On("StorageVolCreateXML", ctx, pool, mock.Anything, uint32(0)).Return(libvirt.StorageVol{}, libvirt.Error{Code: 1}).Once() _, err = a.RunTask(ctx, opts) assert.Error(t, err) + + m.AssertExpectations(t) }internal/repositories/libvirt/adapter_volume_test.go (1)
53-62: Add AssertExpectations in failure-path tests.These tests set expectations but don’t verify them, which can allow false positives.
Suggested update
func TestStoragePoolNotFound(t *testing.T) { @@ err := a.CreateVolume(ctx, testVolName) assert.Error(t, err) + m.AssertExpectations(t) } @@ func TestDeleteVolumeFailures(t *testing.T) { @@ err = a.DeleteVolume(ctx, testVolName) assert.Error(t, err) + m.AssertExpectations(t) }Also applies to: 124-142
internal/repositories/libvirt/adapter_unit_test.go (2)
397-418: Consider handling file operation errors in test setup.Lines 399, 401-402: The
os.CreateTemperror andtmpFile.Close()error are silently ignored. While this is test code, failing early witht.Fatalon setup errors provides clearer diagnostics.Suggested improvement
func TestGetInstanceLogs(t *testing.T) { - tmpFile, _ := os.CreateTemp("", "log") + tmpFile, err := os.CreateTemp("", "log") + if err != nil { + t.Fatal(err) + } defer os.Remove(tmpFile.Name()) _, _ = tmpFile.WriteString("log data") - _ = tmpFile.Close() + if err := tmpFile.Close(); err != nil { + t.Fatal(err) + }
436-439: Consider verifying NewLibvirtAdapter behavior.Both return values are discarded, which provides minimal coverage. If the goal is to verify the function doesn't panic, consider at minimum checking the error or adding a comment explaining the intent.
Suggested improvement
func TestNewLibvirtAdapter(t *testing.T) { logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - _, _ = NewLibvirtAdapter(logger, "qemu:///system") + // This test verifies NewLibvirtAdapter doesn't panic with typical arguments. + // Connection failure is expected in test environment without libvirt daemon. + _, err := NewLibvirtAdapter(logger, "qemu:///system") + // We expect an error since libvirt socket won't be available in test env + assert.Error(t, err) }internal/repositories/libvirt/adapter_network_test.go (2)
61-89: Mock expectations may conflict across subtests.The mock is configured with multiple expectations but all three scenarios use the same mock instance without resetting between scenarios. The mocks for
invalidPortNameandinvalidIPNameboth set upDomainGetXMLDescandNetworkGetDhcpLeaseswithmock.Anything, which may cause unexpected matching. Consider using separate mock instances or table-driven subtests with fresh mocks.Suggested restructuring with isolated mocks
func TestSetupPortForwardingErrors(t *testing.T) { - m := new(MockLibvirtClient) - a := newTestAdapter(m) - a.ipWaitInterval = 1 * time.Microsecond - - // 1. IP Wait Failure - m.On("DomainLookupByName", mock.Anything, "fail").Return(libvirt.Domain{}, libvirt.Error{Code: 1}) - a.setupPortForwarding("fail", []string{"80:80"}) + t.Run("IP Wait Failure", func(t *testing.T) { + m := new(MockLibvirtClient) + a := newTestAdapter(m) + a.ipWaitInterval = 1 * time.Microsecond + m.On("DomainLookupByName", mock.Anything, "fail").Return(libvirt.Domain{}, libvirt.Error{Code: 1}) + a.setupPortForwarding("fail", []string{"80:80"}) + m.AssertExpectations(t) + }) - // 2. Invalid port format - ... + t.Run("Invalid port format", func(t *testing.T) { + // ... with fresh mock + })
91-98: Consider adding assertion or explicit documentation.The test verifies that
configureIptablesdoesn't crash when iptables isn't found, but there's no assertion. Consider at least adding at.Logor comment to make the test's purpose explicit in output.Suggested improvement
func TestConfigureIptablesNoPath(t *testing.T) { - // This test depends on environment, but we can verify it doesn't crash + // Verify configureIptables handles missing iptables gracefully without crashing m := new(MockLibvirtClient) a := newTestAdapter(m) a.configureIptables("test", "1.2.3.4", "80", 8080, 80) - // Should log error if iptables not found, but not crash + // If we reach here without panic, the test passes + // The method should log an error if iptables is not found }internal/repositories/libvirt/adapter.go (1)
130-134: Silent failures on cleanup could mask issues in development.Lines 131-132 silently ignore errors from
DomainUndefineandStorageVolDeleteduring domain creation failure. While acceptable for cleanup paths, consider at least logging these errors at debug/warn level for troubleshooting.Suggested improvement
if err := a.client.DomainCreate(ctx, dom); err != nil { - _ = a.client.DomainUndefine(ctx, dom) - _ = a.client.StorageVolDelete(ctx, vol, 0) + if undefErr := a.client.DomainUndefine(ctx, dom); undefErr != nil { + a.logger.Debug("cleanup: failed to undefine domain", "error", undefErr) + } + if delErr := a.client.StorageVolDelete(ctx, vol, 0); delErr != nil { + a.logger.Debug("cleanup: failed to delete volume", "error", delErr) + } return "", fmt.Errorf("failed to start domain: %w", err) }
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/load-tests.yml:85
- The workflow is using
pathparameter for the k6 action, but the correct parameter name isfilenameaccording to the action's documentation. This has been corrected in the change.
path: tests/load/api-smoke.js
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/handlers/rbac_handler_test.go`:
- Around line 366-373: The tests reference an undefined constant pathInvalid
causing compilation failures; add a missing constant (e.g., const pathInvalid =
"/invalid-id") to the file's constants block so tests like the
"UpdateRoleInvalidID" subtest and the other uses at the locations referenced
compile; update the same constants block where other test constants are declared
so handler.UpdateRole and the other tests using pathInvalid can use it.
🧹 Nitpick comments (4)
internal/handlers/stack_handler_test.go (1)
183-189: Consider settinggin.TestModein the setup function.Unlike the happy-path tests, the error-path tests don't set
gin.SetMode(gin.TestMode). This omission causes Gin to run in debug mode, producing noisy log output during test execution.♻️ Proposed fix
func TestStackHandlerErrorPaths(t *testing.T) { setup := func(_ *testing.T) (*mockStackService, *StackHandler, *gin.Engine) { + gin.SetMode(gin.TestMode) svc := new(mockStackService) handler := NewStackHandler(svc) r := gin.New() return svc, handler, r }internal/handlers/rbac_handler_test.go (1)
314-497: Good comprehensive error path coverage.The
TestRBACHandlerErrorPathsfunction provides thorough testing of error scenarios including invalid JSON, invalid IDs, and service errors. The use of subtests witht.Runand consistent error creation viaerrors.New(errors.Internal, "error")follows project conventions.Consider adding
defer svc.AssertExpectations(t)to subtests that set up mock expectations (e.g.,CreateRoleServiceError,ListRolesServiceError, etc.) to ensure mocks are called as expected:♻️ Example improvement for one subtest
t.Run("CreateRoleServiceError", func(t *testing.T) { svc, handler, r := setupRBACHandlerTest(t) + defer svc.AssertExpectations(t) r.POST(rolesPath, handler.CreateRole) svc.On("CreateRole", mock.Anything, mock.Anything).Return(errors.New(errors.Internal, "error"))internal/repositories/libvirt/adapter_test.go (2)
165-170: Preserve environment for the helper process.Overwriting
cmd.Envdrops existing variables (e.g., PATH, coverage envs). Appending to the current environment is safer and avoids flaky CI behavior.♻️ Suggested tweak
- cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"} + cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
183-196: Consider table-driven subtests for LBProxy operations.These three subtests share the same setup and assertion pattern; a table-driven loop would reduce repetition. As per coding guidelines, consider consolidating them into a table-driven test.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/handlers/container_handler_test.go`:
- Around line 215-225: In the NotFound subtest, the mock expectation uses an
undefined identifier errNotFound; update the mock return to use the string
literal "not found" instead (locate the t.Run("NotFound") test created by
setupContainerHandlerTest and the svc.On("GetDeployment", mock.Anything,
id).Return(...) call used with handler.GetDeployment) so the errors.New(...)
call receives a valid message.
In `@internal/handlers/rbac_handler_test.go`:
- Around line 325-334: The subtests (e.g., the "CreateRoleServiceError" case
using setupRBACHandlerTest and handler.CreateRole) set service expectations with
svc.On(...) but never assert them; update each such subtest to call
svc.AssertExpectations(t) via t.Cleanup(func(){ svc.AssertExpectations(t) }) (or
defer) immediately after configuring svc.On so the test will fail if the handler
doesn't call the mocked methods — apply the same pattern to the other listed
subtests that set svc.On.
🧹 Nitpick comments (3)
internal/handlers/image_handler_test.go (2)
343-347: Avoid silent failures with blank identifier assignments.Per coding guidelines, blank identifier assignments like
_, _ = part.Write(...)and_ = writer.Close()should be avoided. Useassert.NoErrorfor consistency with the success test at lines 134-137.♻️ Suggested fix
body := new(bytes.Buffer) writer := multipart.NewWriter(body) part, _ := writer.CreateFormFile("file", "image.qcow2") - _, _ = part.Write([]byte("image content")) - _ = writer.Close() + _, err := part.Write([]byte("image content")) + assert.NoError(t, err) + err = writer.Close() + assert.NoError(t, err)
360-365: Unused*testing.Tparameter in helper function.The
tparameter is discarded with a blank identifier. Either utilize it (e.g., callt.Helper()to improve stack traces) or remove it from the signature.♻️ Suggested fix
-func setupImageHandlerTest(_ *testing.T) (*mockImageService, *ImageHandler, *gin.Engine) { +func setupImageHandlerTest(t *testing.T) (*mockImageService, *ImageHandler, *gin.Engine) { + t.Helper() svc := new(mockImageService) handler := NewImageHandler(svc) r := gin.New() return svc, handler, r }internal/handlers/rbac_handler_test.go (1)
315-498: Consider a table-driven structure for the error-path suite.
TestRBACHandlerErrorPathsrepeats setup/teardown and request wiring. A table-driven loop would reduce duplication and make it easier to add more cases consistently. As per coding guidelines, prefer table-driven tests in*_test.go.
| t.Run("NotFound", func(t *testing.T) { | ||
| svc, handler, r := setupContainerHandlerTest(t) | ||
| r.GET(deploymentsPath+"/:id", handler.GetDeployment) | ||
| id := uuid.New() | ||
| svc.On("GetDeployment", mock.Anything, id).Return(nil, errors.New(errors.NotFound, errNotFound)) | ||
| req, _ := http.NewRequest(http.MethodGet, deploymentsPath+"/"+id.String(), nil) | ||
| w := httptest.NewRecorder() | ||
| r.ServeHTTP(w, req) | ||
| assert.Equal(t, http.StatusNotFound, w.Code) | ||
| svc.AssertExpectations(t) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for errNotFound definition in the httphandlers package and beyond
echo "=== Searching for errNotFound in internal/handlers/ ==="
rg -n 'errNotFound' internal/handlers/
echo ""
echo "=== Searching for errNotFound in entire repo ==="
rg -n 'errNotFound' --type go
echo ""
echo "=== Checking container_handler_test.go file content around lines 215-225 ==="
wc -l internal/handlers/container_handler_test.goRepository: PoyrazK/thecloud
Length of output: 2475
🏁 Script executed:
#!/bin/bash
# Check the imports and top-level definitions in the test file
echo "=== First 50 lines of container_handler_test.go (imports and constants) ==="
head -50 internal/handlers/container_handler_test.go
echo ""
echo "=== Lines around 215-225 context ==="
sed -n '205,235p' internal/handlers/container_handler_test.goRepository: PoyrazK/thecloud
Length of output: 2779
Fix undefined identifier errNotFound in test mock expectation.
The constant errNotFound is not defined in container_handler_test.go. Replace with the string literal "not found":
Proposed fix
- svc.On("GetDeployment", mock.Anything, id).Return(nil, errors.New(errors.NotFound, errNotFound))
+ svc.On("GetDeployment", mock.Anything, id).Return(nil, errors.New(errors.NotFound, "not found"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| t.Run("NotFound", func(t *testing.T) { | |
| svc, handler, r := setupContainerHandlerTest(t) | |
| r.GET(deploymentsPath+"/:id", handler.GetDeployment) | |
| id := uuid.New() | |
| svc.On("GetDeployment", mock.Anything, id).Return(nil, errors.New(errors.NotFound, errNotFound)) | |
| req, _ := http.NewRequest(http.MethodGet, deploymentsPath+"/"+id.String(), nil) | |
| w := httptest.NewRecorder() | |
| r.ServeHTTP(w, req) | |
| assert.Equal(t, http.StatusNotFound, w.Code) | |
| svc.AssertExpectations(t) | |
| }) | |
| t.Run("NotFound", func(t *testing.T) { | |
| svc, handler, r := setupContainerHandlerTest(t) | |
| r.GET(deploymentsPath+"/:id", handler.GetDeployment) | |
| id := uuid.New() | |
| svc.On("GetDeployment", mock.Anything, id).Return(nil, errors.New(errors.NotFound, "not found")) | |
| req, _ := http.NewRequest(http.MethodGet, deploymentsPath+"/"+id.String(), nil) | |
| w := httptest.NewRecorder() | |
| r.ServeHTTP(w, req) | |
| assert.Equal(t, http.StatusNotFound, w.Code) | |
| svc.AssertExpectations(t) | |
| }) |
🤖 Prompt for AI Agents
In `@internal/handlers/container_handler_test.go` around lines 215 - 225, In the
NotFound subtest, the mock expectation uses an undefined identifier errNotFound;
update the mock return to use the string literal "not found" instead (locate the
t.Run("NotFound") test created by setupContainerHandlerTest and the
svc.On("GetDeployment", mock.Anything, id).Return(...) call used with
handler.GetDeployment) so the errors.New(...) call receives a valid message.
| t.Run("CreateRoleServiceError", func(t *testing.T) { | ||
| svc, handler, r := setupRBACHandlerTest(t) | ||
| r.POST(rolesPath, handler.CreateRole) | ||
| svc.On("CreateRole", mock.Anything, mock.Anything).Return(errors.New(errors.Internal, "error")) | ||
| body, _ := json.Marshal(map[string]interface{}{"name": "n"}) | ||
| req, _ := http.NewRequest("POST", rolesPath, bytes.NewBuffer(body)) | ||
| w := httptest.NewRecorder() | ||
| r.ServeHTTP(w, req) | ||
| assert.Equal(t, http.StatusInternalServerError, w.Code) | ||
| }) |
There was a problem hiding this comment.
Assert mock expectations in subtests that set them.
Several error-path subtests configure svc.On(...) but never call svc.AssertExpectations(t), so a handler that doesn’t call the service would still pass. Consider adding t.Cleanup (or defer) for those subtests.
✅ Suggested pattern
t.Run("CreateRoleServiceError", func(t *testing.T) {
- svc, handler, r := setupRBACHandlerTest(t)
+ svc, handler, r := setupRBACHandlerTest(t)
+ t.Cleanup(func() { svc.AssertExpectations(t) })
r.POST(rolesPath, handler.CreateRole)
svc.On("CreateRole", mock.Anything, mock.Anything).Return(errors.New(errors.Internal, "error"))
body, _ := json.Marshal(map[string]interface{}{"name": "n"})
req, _ := http.NewRequest("POST", rolesPath, bytes.NewBuffer(body))
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusInternalServerError, w.Code)
})Also applies to: 336-344, 386-396, 407-416, 437-447, 458-467, 478-487, 489-497
🤖 Prompt for AI Agents
In `@internal/handlers/rbac_handler_test.go` around lines 325 - 334, The subtests
(e.g., the "CreateRoleServiceError" case using setupRBACHandlerTest and
handler.CreateRole) set service expectations with svc.On(...) but never assert
them; update each such subtest to call svc.AssertExpectations(t) via
t.Cleanup(func(){ svc.AssertExpectations(t) }) (or defer) immediately after
configuring svc.On so the test will fail if the handler doesn't call the mocked
methods — apply the same pattern to the other listed subtests that set svc.On.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This PR increases test coverage across multiple handlers to meet the 80% project target, resolves identified security hotspots in the libvirt adapter, and fixes CI/CD workflow version issues.
Closes #23
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.