feat: Coverage-Based Deduplication Support via runtime/coverage Package#202
feat: Coverage-Based Deduplication Support via runtime/coverage Package#202slayerjain merged 29 commits intomainfrom
Conversation
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
|
Keploy failed to create test cases for this PR. For retrying, please click here |
There was a problem hiding this comment.
Pull Request Overview
Adds a new Go package to collect and report coverage data via atomic counters and Unix sockets, replacing legacy mock and mode functionality.
- Removed legacy
mode.goandmock.goimplementations. - Introduced
keploy.gowhich starts a control server, clears/dumps coverage, processes profiles viacovdata, and sends JSON results. - Updated
go.modto remove unused dependencies and requiregolang.org/x/toolsfor coverage parsing.
Reviewed Changes
Copilot reviewed 4 out of 108 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| keploy/mode.go | Removed old mode enumeration and validation logic |
| keploy/mock.go | Removed legacy mock runner that directly invoked Keploy CLI |
| keploy/keploy.go | New control server and coverage reporting using runtime/coverage |
| go.mod | Cleaned out unused requires and added golang.org/x/tools |
Comments suppressed due to low confidence (4)
go.mod:3
- The module's Go version is set to 1.16 but the code imports runtime/coverage which requires Go 1.25 or later; please bump the Go version accordingly.
go 1.16
go.mod:5
- The commented replace directive appears outdated and can confuse maintainers; consider removing or updating it.
//replace go.keploy.io/server => ../keploy
keploy/keploy.go:1
- This new package implements significant functionality but currently lacks any unit or integration tests; consider adding tests for the control server, command handling, and coverage processing.
// To activate, simply import this package for its side effects:
keploy/keploy.go:8
- [nitpick] The
keploy.gofile is quite large and combines server setup, command handling, and coverage processing; consider refactoring into smaller, focused packages or files to improve maintainability.
package keploy
| const ( | ||
| // controlSocketPath is used by Keploy to send commands (START/END) to the app. | ||
| controlSocketPath = "/tmp/coverage_control.sock" | ||
| // dataSocketPath is used by the app to send coverage data back to Keploy. | ||
| dataSocketPath = "/tmp/keploy-coverage.sock" |
There was a problem hiding this comment.
Using fixed Unix socket paths under /tmp can lead to symlink or permission vulnerabilities; consider using a secure temporary directory or making the path configurable with proper permissions.
| const ( | |
| // controlSocketPath is used by Keploy to send commands (START/END) to the app. | |
| controlSocketPath = "/tmp/coverage_control.sock" | |
| // dataSocketPath is used by the app to send coverage data back to Keploy. | |
| dataSocketPath = "/tmp/keploy-coverage.sock" | |
| var ( | |
| // controlSocketPath is used by Keploy to send commands (START/END) to the app. | |
| controlSocketPath string | |
| // dataSocketPath is used by the app to send coverage data back to Keploy. | |
| dataSocketPath string | |
| // tempDir is the temporary directory for socket files. | |
| tempDir string |
| } | ||
| err := reportCoverage(id); |
There was a problem hiding this comment.
On receiving an END command with a mismatched test ID, the code logs a warning but still proceeds to report coverage for the provided ID; you may want to skip reporting in this case to avoid inconsistent state.
| } | |
| err := reportCoverage(id); | |
| // Reset the currentTestID to an empty string to indicate that no test is currently being recorded. | |
| currentTestID = "" | |
| return | |
| } | |
| err := reportCoverage(id) |
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
| for { | ||
| conn, err := ln.Accept() | ||
| if err != nil { | ||
| if strings.Contains(err.Error(), "use of closed network connection") { |
There was a problem hiding this comment.
Were we really getting this error here? "use of closed network connection", Because here you have just accepted the connection.
There was a problem hiding this comment.
no i did not get this error. this error can only happen when it gets closed manually.
| log.Printf("[Agent] Error clearing coverage counters: %v", err) | ||
| } | ||
| case "END": | ||
| if currentTestID != id { |
There was a problem hiding this comment.
You can use early return here. And then there is no need of else block.
| // reportCoverage dumps, processes, and sends the coverage data. | ||
| func reportCoverage(testID string) error { | ||
| // Create a temporary directory to store the coverage data. | ||
| tempDir, err := os.MkdirTemp("", "keploy-coverage-") |
There was a problem hiding this comment.
We can name the temp folder as keploy-coverage-<testID>
|
|
||
| // For each block in the profile, if the count is greater than 0, add the lines to the map. | ||
| for _, block := range profile.Blocks { | ||
| if block.Count > 0 { |
There was a problem hiding this comment.
you can write this condition like
if block.Count<=0{
continue
}Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
| @@ -1,14 +1,7 @@ | |||
| module github.com/keploy/go-sdk/v2 | |||
| @@ -1,22 +1,261 @@ | |||
| // To activate, simply import this package for its side effects: | |||
| // | |||
| // import _ "github.com/keploy/go-sdk/v2/keploy" | |||
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
This adds a Go package that—when imported and built with
-cover -covermode=atomic—automatically:START/END <testID>commands over a Unix socketgo tool covdata textfmt, parses profiles, and maps executed lines per file