Skip to content

Daily Test Coverage Improver - Add tests for packages.go include dependency functions#3590

Merged
pelikhan merged 1 commit intomainfrom
test-coverage-packages-1762828638-2bc220b15171d5cc
Nov 11, 2025
Merged

Daily Test Coverage Improver - Add tests for packages.go include dependency functions#3590
pelikhan merged 1 commit intomainfrom
test-coverage-packages-1762828638-2bc220b15171d5cc

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Daily Test Coverage Improver - Include Dependency Functions

This PR adds comprehensive unit tests for previously untested functions in pkg/cli/packages.go.

Summary of Changes

Added pkg/cli/packages_test.go with 519 lines of test code covering two critical low-coverage functions:

collectPackageIncludesRecursive() Tests

  • Single includes: Basic @include directive processing
  • Optional includes: @include? with missing files
  • Section references: Include with #section syntax
  • Nested includes: Recursive include processing
  • Duplicate handling: Seen map preventing infinite loops
  • Circular references: Protection against circular include chains
  • Whitespace variations: Robust parsing with extra whitespace

copyIncludeDependenciesFromPackageWithForce() Tests

  • Single dependency: Basic file copying
  • Force flag behavior: Overwriting vs. skipping existing files
  • Content comparison: Skip when source and target are identical
  • Optional files: Graceful handling of missing optional includes
  • Nested directories: Automatic directory creation
  • FileTracker integration: Created/modified file tracking

includePattern Regex Tests

  • Pattern matching: Validate @include and @include? patterns
  • Section references: Handle #section syntax
  • Whitespace tolerance: Parse with varying whitespace

Problems Found

During coverage analysis, I identified two critical functions in pkg/cli/packages.go with very low test coverage:

  • collectPackageIncludesRecursive() - 14.7% coverage
  • copyIncludeDependenciesFromPackageWithForce() - 6.1% coverage

These functions handle complex file system operations including:

  • Recursive include directive parsing
  • Circular reference detection
  • Optional vs. required file handling
  • Force-overwrite logic
  • File tracking integration

The low coverage created risk for edge case failures and regression bugs.

Actions Taken

Created comprehensive unit tests covering:

  1. 107 test cases across 10 test functions
  2. Edge case coverage: circular references, missing files, identical content, nested paths
  3. File system operations: temp directories, file creation, content comparison
  4. Integration testing: FileTracker created/modified tracking

All tests follow Go testing best practices:

  • Table-driven test design for parameterized scenarios
  • Descriptive test names indicating expected behavior
  • Proper cleanup with t.TempDir()
  • Isolated test cases with no shared state

Test Coverage Results

Metric Before After Change
Overall Coverage 68.0% 68.2% +0.2%
collectPackageIncludesRecursive() 14.7% 79.4% +64.7%
copyIncludeDependenciesFromPackageWithForce() 6.1% 78.8% +72.7%

Replicating the Test Coverage Measurements

# Before measurements (baseline from main branch)
cd /home/runner/work/gh-aw/gh-aw
git checkout main
go test -coverprofile=coverage-before.out -covermode=atomic ./...
go tool cover -func=coverage-before.out | tail -1
# Output: total: (statements) 68.0%

go tool cover -func=coverage-before.out | grep "pkg/cli/packages.go" | grep -E "(collectPackageIncludesRecursive|copyIncludeDependenciesFromPackageWithForce)"
# Output:
#   collectPackageIncludesRecursive                 14.7%
#   copyIncludeDependenciesFromPackageWithForce     6.1%

# After measurements (this PR)
git checkout test-coverage-packages-1762828638
go test -coverprofile=coverage-after.out -covermode=atomic ./...
go tool cover -func=coverage-after.out | tail -1
# Output: total: (statements) 68.2%

go tool cover -func=coverage-after.out | grep "pkg/cli/packages.go" | grep -E "(collectPackageIncludesRecursive|copyIncludeDependenciesFromPackageWithForce)"
# Output:
#   collectPackageIncludesRecursive                 79.4%
#   copyIncludeDependenciesFromPackageWithForce     78.8%

# Run the specific new tests
go test -v -run "TestCollectPackageIncludesRecursive|TestCopyIncludeDependenciesFromPackageWithForce|TestIncludePattern" ./pkg/cli/
# All 107 test cases pass

Possible Other Areas for Future Improvement

Based on coverage analysis, here are high-value targets for future test additions:

CLI Package (low-hanging fruit):

  1. pkg/cli/init_command.go - 0% coverage - Init command functionality
  2. pkg/cli/jq.go - 0% coverage - JQ filtering utility
  3. pkg/cli/mcp.go - 0% coverage - MCP command entry point
  4. pkg/cli/mcp_server.go - 0% coverage - MCP server management
  5. pkg/cli/status.go - 0% coverage - Workflow status display
  6. pkg/cli/update_command.go - 9.6% coverage - Update command

Workflow Package:
7. pkg/workflow/npm_validation.go - 0% coverage - NPM package validation
8. pkg/workflow/pip_validation.go - 0% coverage - PIP package validation
9. pkg/cli/packages.go:listWorkflowsInPackage - 42.9% coverage - Workflow listing

Verification

All tests run successfully:

cd /home/runner/work/gh-aw/gh-aw
make test-unit
# All tests pass including 107 new test cases

make fmt
# No formatting changes needed (already formatted)

AI generated by Daily Test Coverage Improver

- Add tests for collectPackageIncludesRecursive covering:
  * Single includes
  * Optional includes with missing files
  * Include with section references
  * Nested includes
  * Duplicate include handling
  * Circular reference protection
  * Whitespace variations

- Add tests for copyIncludeDependenciesFromPackageWithForce covering:
  * Single dependency copying
  * Skipping existing files without force flag
  * Overwriting with force flag
  * Optional missing file handling
  * Content comparison (skip if identical)
  * Nested directory creation
  * FileTracker integration (created/modified tracking)

- Add tests for includePattern regex validation

Coverage improvements:
- collectPackageIncludesRecursive: 14.7% → 79.4% (+64.7%)
- copyIncludeDependenciesFromPackageWithForce: 6.1% → 78.8% (+72.7%)
- Overall: 68.0% → 68.2% (+0.2%)
@pelikhan pelikhan marked this pull request as ready for review November 11, 2025 15:34
Copilot AI review requested due to automatic review settings November 11, 2025 15:34
@pelikhan pelikhan merged commit 62ca114 into main Nov 11, 2025
6 checks passed
@pelikhan pelikhan deleted the test-coverage-packages-1762828638-2bc220b15171d5cc branch November 11, 2025 15:34
Copy link
Copy Markdown
Contributor

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

This PR adds comprehensive unit test coverage for two critical functions in pkg/cli/packages.go that were previously undertested: collectPackageIncludesRecursive() (14.7% → 79.4% coverage) and copyIncludeDependenciesFromPackageWithForce() (6.1% → 78.8% coverage). These functions handle recursive include directive parsing and file copying operations for package-based workflows.

Key Changes

  • Added 519 lines of test code covering 107 test cases across multiple scenarios
  • Implemented table-driven tests for include dependency collection with nested, optional, and circular reference handling
  • Added tests for file copying operations including force flag behavior, content comparison, and FileTracker integration
  • Included regex pattern validation tests for the @include directive parsing

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

Comment on lines +347 to +352
filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error {
if err == nil && !info.IsDir() {
fileCount++
}
return nil
})
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filepath.Walk function can return an error, but this implementation ignores it by always returning nil in the walk function. The error parameter should be checked, and if non-nil, the function should return it immediately to stop the walk and propagate the error.

Consider updating to:

err := filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error {
	if err != nil {
		return err
	}
	if !info.IsDir() {
		fileCount++
	}
	return nil
})
if err != nil {
	t.Errorf("Failed to walk target directory: %v", err)
}
Suggested change
filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error {
if err == nil && !info.IsDir() {
fileCount++
}
return nil
})
err = filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
fileCount++
}
return nil
})
if err != nil {
t.Errorf("Failed to walk target directory: %v", err)
}

Copilot uses AI. Check for mistakes.
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.

2 participants