Daily Test Coverage Improver - Add unit tests for pkg/cli/jq.go

[github-actions]

Summary

This PR improves test coverage for pkg/cli/jq.go by splitting integration tests from unit tests and adding additional test cases.

Problems Found

The jq.go file had 0% test coverage in unit tests because:

• All tests were marked with //go:build integration tag
• Unit tests (make test-unit) skipped these tests entirely
• Coverage reports only counted code tested during unit test runs

Actions Taken

1. Split tests into two files:

   • pkg/cli/jq_test.go - Unit tests without integration build tag
   • pkg/cli/jq_integration_test.go - Integration tests with build tag

2. Added 5 new test cases to improve coverage:

   • Identity filter (.)
   • Extract specific field (.name)
   • Empty input handling
   • Array transformation (map(. * 2))
   • Empty filter error handling

3. Retained all original test cases:

   • Array element access
   • Array length counting
   • Map and select operations
   • Invalid filter syntax errors
   • Invalid JSON input errors
   • jq not available scenario

Test Coverage Results

Metric	Before	After	Change
pkg/cli/jq.go coverage	0.0%	87.5%	+87.5%
Overall project coverage	68.5%	68.5%	No change*

*The overall percentage didn't change because this is a small file, but we eliminated a 0% coverage file from the codebase.

Replicating the Test Coverage Measurements

Install dependencies:

make deps

Build the project:

make build

Run unit tests with coverage (BEFORE - from main branch):

git checkout main
go test -coverprofile=coverage-before.out ./...
go tool cover -func=coverage-before.out | grep "pkg/cli/jq.go"
# Output: ApplyJqFilter 0.0%

Run unit tests with coverage (AFTER - from this PR):

git checkout test-coverage/jq-unit-tests
go test -coverprofile=coverage-after.out ./...
go tool cover -func=coverage-after.out | grep "pkg/cli/jq.go"
# Output: ApplyJqFilter 87.5%

View coverage summary:

go tool cover -func=coverage-after.out | grep "total:"

Possible Other Areas for Future Improvement

Based on the coverage report, the following files have 0% coverage and would benefit from similar treatment:

1. pkg/cli/init_command.go - 0.0% coverage
2. pkg/cli/jq.go - ✅ Fixed in this PR
3. pkg/cli/mcp.go - 0.0% coverage
4. pkg/cli/mcp_server.go - 0.0% coverage
5. pkg/cli/pr_automerge.go - 0.0% coverage
6. pkg/cli/retry.go - 0.0% coverage
7. pkg/cli/run.go - 0.0% coverage
8. pkg/cli/status.go - 0.0% coverage
9. pkg/workflow/npm_validation.go - 0.0% coverage
10. pkg/workflow/pip_validation.go - 0.0% coverage

Additionally, many files have low coverage (< 30%) that could be improved with targeted unit tests:

• pkg/cli/trial_command.go - 8.5%
• pkg/cli/update_command.go - 9.6%
• pkg/cli/mcp_inspect_mcp.go - 11.6%

Commands Executed

# Check for existing test files
ls -la /home/runner/work/gh-aw/gh-aw/pkg/cli/jq*

# Create new branch
git checkout -b test-coverage/jq-unit-tests

# Run unit tests for jq
go test -v -run TestApplyJqFilter ./pkg/cli

# Run full unit test suite
make test-unit

# Generate coverage reports
go test -coverprofile=coverage.out ./...
go test -coverprofile=coverage-new.out ./...

# Check coverage for jq.go
go tool cover -func=coverage.out | grep "pkg/cli/jq.go"
go tool cover -func=coverage-new.out | grep "pkg/cli/jq.go"

# Compare overall coverage
go tool cover -func=coverage.out | grep "total:"
go tool cover -func=coverage-new.out | grep "total:"

# Format code
make fmt
gofmt -l pkg/cli/jq_test.go pkg/cli/jq_integration_test.go

# Commit changes
git add pkg/cli/jq_test.go pkg/cli/jq_integration_test.go
git commit -m "test: improve unit test coverage for pkg/cli/jq.go"

Web Searches Performed

None - This work was based on repository analysis and existing test patterns.

Web Pages Fetched

None - This work was based on repository analysis and existing test patterns.

AI generated by Daily Test Coverage Improver

[Copilot]

Pull Request Overview

This PR improves unit test coverage for pkg/cli/jq.go by separating integration tests from unit tests and adding new test cases to increase coverage from 0% to 87.5%.

Key changes:

• Split existing tests into two files: unit tests (jq_test.go) and integration tests (jq_integration_test.go)
• Added 5 new unit test cases covering identity filter, field extraction, empty input, array transformation, and empty filter scenarios
• Removed the //go:build integration tag from unit tests to ensure they run during standard test execution

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
pkg/cli/jq_test.go	Removed integration build tag, added 5 new unit test cases, removed integration test function (moved to separate file)
pkg/cli/jq_integration_test.go	Created new file with integration build tag containing MCP server integration tests

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The logical condition in this validation is incorrect. Using && (AND) with !strings.Contains() means the test will only fail if none of the values are present, but will pass if any one is present.

For example, if the output is [2,2,2], the test would pass because it contains "2", even though it doesn't contain "4" or "6". This doesn't properly validate the transformation map(. * 2) which should produce [2,4,6].

Consider using || (OR) instead to ensure all three expected values are present:

if !strings.Contains(output, "2") || !strings.Contains(output, "4") || !strings.Contains(output, "6") {
    t.Error("Expected transformed array output containing 2, 4, and 6")
}

Suggested change

	if !strings.Contains(output, "2") && !strings.Contains(output, "4") && !strings.Contains(output, "6") {
	t.Error("Expected transformed array output")
	if !strings.Contains(output, "2") || !strings.Contains(output, "4") || !strings.Contains(output, "6") {
	t.Error("Expected transformed array output containing 2, 4, and 6")

[Copilot]

The error from os.Getwd() is being silently ignored using the blank identifier _. If os.Getwd() fails, originalDir will be an empty string, and the deferred os.Chdir(originalDir) call will fail to restore the working directory, potentially affecting other tests.

Consider handling the error:

originalDir, err := os.Getwd()
if err != nil {
    t.Fatalf("Failed to get current directory: %v", err)
}
defer os.Chdir(originalDir)

Suggested change

	originalDir, _ := os.Getwd()
	originalDir, err := os.Getwd()
	if err != nil {
	t.Fatalf("Failed to get current directory: %v", err)
	}

[Copilot]

Accessing result.Content[0] without checking if the slice is empty could cause a panic if result.Content is empty or nil. Consider adding a bounds check before accessing the first element:

if len(result.Content) == 0 {
    t.Fatal("Expected content in result, but got empty content")
}
if textContent, ok := result.Content[0].(*mcp.TextContent); ok {
    // ... validation logic
} else {
    t.Error("Expected text content from status tool with jq filter")
}

[Copilot]

Accessing result.Content[0] without checking if the slice is empty could cause a panic if result.Content is empty or nil. Consider adding a bounds check before accessing the first element:

if len(result.Content) == 0 {
    t.Fatal("Expected content in result, but got empty content")
}
if textContent, ok := result.Content[0].(*mcp.TextContent); ok {
    // ... validation logic
} else {
    t.Error("Expected text content from status tool with jq count filter")
}

Suggested change

	// Verify result contains a number
	// Verify result contains a number
	if len(result.Content) == 0 {
	t.Fatal("Expected content in result, but got empty content")
	}