Skip to content

Improve test quality for pkg/parser/import_cache_test.go#17742

Merged
pelikhan merged 2 commits intomainfrom
copilot/improve-import-cache-tests
Feb 22, 2026
Merged

Improve test quality for pkg/parser/import_cache_test.go#17742
pelikhan merged 2 commits intomainfrom
copilot/improve-import-cache-tests

Conversation

Copy link
Contributor

Copilot AI commented Feb 22, 2026

import_cache_test.go used raw t.Fatalf/t.Errorf assertions throughout and had zero coverage for sanitizePath and validatePathComponents — the two security-critical path traversal prevention functions.

Changes

  • Testify assertions — replaced all raw t.Fatal/t.Error calls with require.*/assert.* with descriptive messages
  • Modern temp dirs — replaced os.MkdirTemp + defer os.RemoveAll with t.TempDir() across all tests
  • TestSanitizePath — table-driven, 4 cases: simple path, nested, flat, dot-cleaned
  • TestValidatePathComponents — table-driven, 6 cases: valid, empty owner/sha, path traversal in owner/path, absolute sha
  • TestImportCacheSet_Validation — table-driven, 4 cases covering Set error paths: oversized content (>10MB), path traversal in owner/path, empty owner
// Before
if !found {
    t.Error("Cache entry not found after Set")
}

// After
assert.True(t, found, "cache entry should be found after Set")
// New: security-critical functions now tested
func TestValidatePathComponents(t *testing.T) {
    tests := []struct{ name, owner, repo, path, sha string; shouldErr bool; errMsg string }{
        {"path traversal in owner", "../etc", "repo", "test.md", "abc123", true, ".."},
        {"absolute path in sha",   "owner",   "repo", "test.md", "/abs",   true, "absolute path"},
        // ...
    }
    // ...
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE 64/bin/go echo inte�� GOMODCACHE go /usr/bin/git WcV6/ohkHzwdoE9Jgit GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha runs/20260222-154946-27321/test-1837266060/.github/workflows GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go l GOMOD GOMODCACHE go env -json (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha util.test GO111MODULE ortcfg.link GOINSECURE GOMOD GOMODCACHE QaOsozRER3G3MIniHc/GufYOpZ-nZgV6X4DmJOt/koqjVT7pz4U_34s9HZ9T env -json GO111MODULE g_.a GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha -json GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet env runs/20260222-154946-27321/test-3385248809/.github/workflows .cfg 6649903/b355/vet.cfg GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha -json GO111MODULE Name,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 6649903/b364/vet.cfg GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha -m Initial commit /usr/bin/git "prettier" --chegit GOPROXY 64/bin/go git rev-�� --git-dir /opt/hostedtoolcconfig /usr/bin/git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha -bool -buildtags /usr/bin/git -errorsas -ifaceassert -nilfunc git rev-�� --show-toplevel -tests 6649903/b424/vet.cfg -json GO111MODULE 64/bin/go git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha user.name Test User /usr/bin/git --check scripts/**/*.js 64/bin/go git rev-�� --show-toplevel go /usr/bin/git ub/workflows GO111MODULE 64/bin/go /usr/bin/git (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v8
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE ache/go/1.25.0/xGO111MODULE env 0443186/b387/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE b/gh-aw/pkg/consenv GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE npx (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 6649903/b363/vet.cfg GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/setup-node/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha 09 GO111MODULE Name,createdAt,startedAt,updated-test.short=true GOINSECURE GOMOD GOMODCACHE go env 4946-27321/test-2717655473 GO111MODULE 6649903/b359/vet.cfg GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env hub/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env ty-test.md GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE lj/mxXq840b1E1KaJyHxqD8/NawLaW_qZIqiXQp2Uc0n (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts
    • Triggering command: /usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path g/timeutil/formaGOINSECURE g/timeutil/formaGOMOD 64/bin/go GOINSECURE GOMOD erignore ache/go/1.25.0/xGO111MODULE env 0443186/b412/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE I4/X0HLTQaA8PlvKCwosL9U/WoEm3ctorev-parse env 2951381812/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/nonexistent/repo/actions/runs/12345
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go GOINSECURE GOMOD erignore 5EBHId6yOoQu env 0443186/b417/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go GOINSECURE GOMOD erignore ache/go/1.25.0/xGO111MODULE env 0443186/b381/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/contents/file.md
    • Triggering command: /tmp/go-build3056649903/b381/cli.test /tmp/go-build3056649903/b381/cli.test -test.testlogfile=/tmp/go-build3056649903/b381/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true GOINSECURE GOMOD GOMODCACHE erignore env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/test-owner/test-repo/actions/secrets
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name 35126d5394e2a1caGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD erignore ache/go/1.25.0/xGO111MODULE env 0443186/b409/_pkGOINSECURE GO111MODULE 64/bin/go GOINSECURE b/gh-aw/pkg/test-V=full GOMODCACHE go (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[testify-expert] Improve Test Quality: pkg/parser/import_cache_test.go</issue_title>
<issue_description>The test file pkg/parser/import_cache_test.go has been selected for quality improvement by the Testify Uber Super Expert 🧪✨. This issue provides specific, actionable recommendations to enhance test quality, coverage, and maintainability using testify best practices.

Current State

  • Test File: pkg/parser/import_cache_test.go
  • Source File: pkg/parser/import_cache.go
  • Test Functions: 4 test functions (TestImportCache, TestImportCacheDirectory, TestImportCacheMissingFile, TestImportCacheEmptyCache)
  • Lines of Code: 161 lines (test), 175 lines (source)
  • Exported Functions Tested: NewImportCache, Get, Set, GetCacheDir
  • Internal Functions NOT Tested: sanitizePath, validatePathComponents, ensureGitAttributes

Strengths ✅

  1. Good structure: Each test function focuses on a distinct scenario (set/get, directory creation, missing file, empty cache).
  2. Proper cleanup: All tests use defer os.RemoveAll(tempDir) to clean up temp directories.
  3. Build tag present: File correctly includes //go:build !integration at the top.

Areas for Improvement 🎯

1. Replace Raw Go Testing with Testify Assertions

The entire file uses raw t.Fatalf, t.Errorf, and t.Error instead of the testify library that is standard across this codebase.

Current anti-patterns throughout the file:

// ❌ CURRENT (anti-pattern) - raw error handling
if err != nil {
    t.Fatalf("Failed to create temp dir: %v", err)
}
if _, err := os.Stat(cachedPath); os.IsNotExist(err) {
    t.Errorf("Cache file was not created: %s", cachedPath)
}
if string(content) != string(testContent) {
    t.Errorf("Content mismatch. Expected %q, got %q", testContent, content)
}
if !found {
    t.Error("Cache entry not found after Set")
}

Recommended replacement:

// ✅ IMPROVED - testify assertions
require.NoError(t, err, "creating temp dir should succeed")
require.FileExists(t, cachedPath, "cache file should be created at expected path")
assert.Equal(t, string(testContent), string(content), "cached content should match original")
assert.True(t, found, "cache entry should be found after Set")

Why this matters: Testify provides clearer error messages, better test output, and is the standard used throughout this codebase (see scratchpad/testing.md).

Required import:

import (
    "testing"
    
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"
)

2. Table-Driven Tests for validatePathComponents and sanitizePath

Both sanitizePath and validatePathComponents are internal functions in the same package and are directly testable, yet have zero test coverage. These are security-critical functions (path traversal prevention) that deserve thorough testing.

Recommended: Add table-driven tests for sanitizePath:

func TestSanitizePath(t *testing.T) {
    tests := []struct {
        name     string
        input    string
        expected string
    }{
        {
            name:     "simple path",
            input:    "workflows/test.md",
            expected: "workflows_test.md",
        },
        {
            name:     "nested path",
            input:    "a/b/c/file.md",
            expected: "a_b_c_file.md",
        },
        {
            name:     "already flat",
            input:    "file.md",
            expected: "file.md",
        },
        {
            name:     "path with dots cleaned",
            input:    "a/./b/file.md",
            expected: "a_b_file.md",
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            result := sanitizePath(tt.input)
            assert.Equal(t, tt.expected, result, "sanitizePath(%q) should return expected value", tt.input)
        })
    }
}

Recommended: Add table-driven tests for validatePathComponents:

func TestValidatePathComponents(t *testing.T) {
    tests := []struct {
        name      string
        owner     string
        repo      string
        path      string
        sha       string
        shouldErr bool
        errMsg    string
    }{
        {
            name:      "valid components",
            owner:     "testowner",
            repo:      "testrepo",
            path:      "workflows/test.md",
            sha:       "abc123",
            shouldErr: false,
        },
        {
            name:      "empty owner",
            owner:     "",
            repo:      "testrepo",
            path:      "test.md",
            sha:       "abc123",
            shouldErr: true,
            errMsg:    "empty component",
        },
        {
            name:      "path traversal in owner",
            owner:     "../etc",
            repo:      "testrepo",
    ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes github/gh-aw#17739

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality for import_cache_test.go Improve test quality for pkg/parser/import_cache_test.go Feb 22, 2026
@pelikhan pelikhan marked this pull request as ready for review February 22, 2026 15:57
Copilot AI review requested due to automatic review settings February 22, 2026 15:57
Copy link
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 improves test quality for pkg/parser/import_cache_test.go by migrating to testify assertions, modernizing temp directory handling, and adding comprehensive test coverage for previously untested security-critical functions.

Changes:

  • Migrated all raw t.Fatal/t.Error assertions to testify require.*/assert.* with descriptive messages
  • Replaced manual temp directory creation with t.TempDir() for automatic cleanup
  • Added comprehensive table-driven tests for sanitizePath, validatePathComponents, and validation in Set

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

@pelikhan pelikhan merged commit 77c9417 into main Feb 22, 2026
120 checks passed
@pelikhan pelikhan deleted the copilot/improve-import-cache-tests branch February 22, 2026 16:07
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.

3 participants