Skip to content

Conversation

@officialasishkumar
Copy link
Member

This adds a Go package that—when imported and built with -cover -covermode=atomic—automatically:

  • Listens for START/END <testID> commands over a Unix socket
  • Clears and dumps atomic coverage counters
  • Runs go tool covdata textfmt, parses profiles, and maps executed lines per file
  • Sends JSON payloads (testID + coverage) back via a data socket

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>
Copilot AI review requested due to automatic review settings July 14, 2025 06:29
@keploy
Copy link

keploy bot commented Jul 14, 2025

Keploy failed to create test cases for this PR. For retrying, please click here

Copy link

Copilot AI left a 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.go and mock.go implementations.
  • Introduced keploy.go which starts a control server, clears/dumps coverage, processes profiles via covdata, and sends JSON results.
  • Updated go.mod to remove unused dependencies and require golang.org/x/tools for 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.go file 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
Comment on lines 28 to 32
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"
Copy link

Copilot AI Jul 14, 2025

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
keploy/keploy.go Outdated
Comment on lines 106 to 107
}
err := reportCoverage(id);
Copy link

Copilot AI Jul 14, 2025

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.

Suggested change
}
err := reportCoverage(id);
// Reset the currentTestID to an empty string to indicate that no test is currently being recorded.
currentTestID = ""
return
}
err := reportCoverage(id)

Copilot uses AI. Check for mistakes.
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") {
Copy link
Member

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.

Copy link
Member Author

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 {
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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-")
Copy link
Member

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>

Copy link
Member

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.

Copy link
Member Author

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 {
Copy link
Member

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
Copy link
Member

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"
Copy link
Member

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>
@slayerjain slayerjain merged commit 78d4555 into main Jul 31, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants