-
Notifications
You must be signed in to change notification settings - Fork 30
feat: Coverage-Based Deduplication Support via runtime/coverage Package #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
keploy/keploy.go
Outdated
| 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" |
Copilot
AI
Jul 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
keploy/keploy.go
Outdated
| } | ||
| err := reportCoverage(id); |
Copilot
AI
Jul 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use early return here. And then there is no need of else block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@officialasishkumar please address this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
keploy/keploy.go
Outdated
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can name the temp folder as keploy-coverage-<testID>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@officialasishkumar please address this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
keploy/keploy.go
Outdated
|
|
||
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
go.mod
Outdated
| @@ -1,14 +1,7 @@ | |||
| module github.com/keploy/go-sdk/v2 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the module v3
keploy/keploy.go
Outdated
| @@ -1,22 +1,261 @@ | |||
| // To activate, simply import this package for its side effects: | |||
| // | |||
| // import _ "github.com/keploy/go-sdk/v2/keploy" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add /v3 here as well
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