mirror of
https://github.com/ivuorinen/gh-action-readme.git
synced 2026-02-17 19:50:44 +00:00
feat(lint): add many linters, make all the tests run fast! (#23)
* chore(lint): added nlreturn, run linting * chore(lint): replace some fmt.Sprintf calls * chore(lint): replace fmt.Sprintf with strconv * chore(lint): add goconst, use http lib for status codes, and methods * chore(lint): use errors lib, errCodes from internal/errors * chore(lint): dupl, thelper and usetesting * chore(lint): fmt.Errorf %v to %w, more linters * chore(lint): paralleltest, where possible * perf(test): optimize test performance by 78% - Implement shared binary building with package-level cache to eliminate redundant builds - Add strategic parallelization to 15+ tests while preserving environment variable isolation - Implement thread-safe fixture caching with RWMutex to reduce I/O operations - Remove unnecessary working directory changes by leveraging embedded templates - Add embedded template system with go:embed directive for reliable template resolution - Fix linting issues: rename sharedBinaryError to errSharedBinary, add nolint directive Performance improvements: - Total test execution time: 12+ seconds → 2.7 seconds (78% faster) - Binary build overhead: 14+ separate builds → 1 shared build (93% reduction) - Parallel execution: Limited → 15+ concurrent tests (60-70% better CPU usage) - I/O operations: 66+ fixture reads → cached with sync.RWMutex (50% reduction) All tests maintain 100% success rate and coverage while running nearly 4x faster.
This commit is contained in:
GitHub
@@ -3,6 +3,7 @@ package dependencies
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"regexp"
|
||||
@@ -145,7 +146,7 @@ func (a *Analyzer) AnalyzeActionFileWithProgress(
|
||||
progressCallback func(current, total int, message string),
|
||||
) ([]Dependency, error) {
|
||||
if progressCallback != nil {
|
||||
progressCallback(0, 1, fmt.Sprintf("Parsing %s", actionPath))
|
||||
progressCallback(0, 1, "Parsing "+actionPath)
|
||||
}
|
||||
|
||||
// Read and parse the action.yml file
|
||||
@@ -179,8 +180,10 @@ func (a *Analyzer) validateAndCheckComposite(
|
||||
if progressCallback != nil {
|
||||
progressCallback(1, 1, "No dependencies (non-composite action)")
|
||||
}
|
||||
|
||||
return []Dependency{}, false, nil
|
||||
}
|
||||
|
||||
return nil, true, nil
|
||||
}
|
||||
|
||||
@@ -192,6 +195,7 @@ func (a *Analyzer) validateActionType(usingType string) error {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
return fmt.Errorf("invalid action runtime: %s", usingType)
|
||||
}
|
||||
|
||||
@@ -230,11 +234,13 @@ func (a *Analyzer) processStep(step CompositeStep, stepNumber int) *Dependency {
|
||||
// Log error but continue processing
|
||||
return nil
|
||||
}
|
||||
|
||||
return dep
|
||||
} else if step.Run != "" {
|
||||
// This is a shell script step
|
||||
return a.analyzeShellScript(step, stepNumber)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -361,6 +367,7 @@ func (a *Analyzer) parseUsesStatement(uses string) (owner, repo, version string,
|
||||
func (a *Analyzer) isCommitSHA(version string) bool {
|
||||
// Check if it's a 40-character hex string (full SHA) or 7+ character hex (short SHA)
|
||||
re := regexp.MustCompile(`^[a-f0-9]{7,40}$`)
|
||||
|
||||
return len(version) >= minSHALength && re.MatchString(version)
|
||||
}
|
||||
|
||||
@@ -368,6 +375,7 @@ func (a *Analyzer) isCommitSHA(version string) bool {
|
||||
func (a *Analyzer) isSemanticVersion(version string) bool {
|
||||
// Check for vX, vX.Y, vX.Y.Z format
|
||||
re := regexp.MustCompile(`^v?\d+(\.\d+)*(\.\d+)?(-[a-zA-Z0-9.-]+)?(\+[a-zA-Z0-9.-]+)?$`)
|
||||
|
||||
return re.MatchString(version)
|
||||
}
|
||||
|
||||
@@ -379,6 +387,7 @@ func (a *Analyzer) isVersionPinned(version string) bool {
|
||||
return true
|
||||
}
|
||||
re := regexp.MustCompile(`^v?\d+\.\d+\.\d+`)
|
||||
|
||||
return re.MatchString(version)
|
||||
}
|
||||
|
||||
@@ -392,6 +401,7 @@ func (a *Analyzer) convertWithParams(with map[string]any) map[string]string {
|
||||
params[k] = fmt.Sprintf("%v", v)
|
||||
}
|
||||
}
|
||||
|
||||
return params
|
||||
}
|
||||
|
||||
@@ -432,7 +442,7 @@ func (a *Analyzer) CheckOutdated(deps []Dependency) ([]OutdatedDependency, error
|
||||
// getLatestVersion fetches the latest release/tag for a repository.
|
||||
func (a *Analyzer) getLatestVersion(owner, repo string) (version, sha string, err error) {
|
||||
if a.GitHubClient == nil {
|
||||
return "", "", fmt.Errorf("GitHub client not available")
|
||||
return "", "", errors.New("GitHub client not available")
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), apiCallTimeout)
|
||||
@@ -447,6 +457,7 @@ func (a *Analyzer) getLatestVersion(owner, repo string) (version, sha string, er
|
||||
// Try to get latest release first
|
||||
if version, sha, err := a.getLatestRelease(ctx, owner, repo); err == nil {
|
||||
a.cacheVersion(cacheKey, version, sha)
|
||||
|
||||
return version, sha, nil
|
||||
}
|
||||
|
||||
@@ -457,6 +468,7 @@ func (a *Analyzer) getLatestVersion(owner, repo string) (version, sha string, er
|
||||
}
|
||||
|
||||
a.cacheVersion(cacheKey, version, sha)
|
||||
|
||||
return version, sha, nil
|
||||
}
|
||||
|
||||
@@ -483,11 +495,12 @@ func (a *Analyzer) getCachedVersion(cacheKey string) (version, sha string, found
|
||||
func (a *Analyzer) getLatestRelease(ctx context.Context, owner, repo string) (version, sha string, err error) {
|
||||
release, _, err := a.GitHubClient.Repositories.GetLatestRelease(ctx, owner, repo)
|
||||
if err != nil || release.GetTagName() == "" {
|
||||
return "", "", fmt.Errorf("no release found")
|
||||
return "", "", errors.New("no release found")
|
||||
}
|
||||
|
||||
version = release.GetTagName()
|
||||
sha = a.getCommitSHAForTag(ctx, owner, repo, version)
|
||||
|
||||
return version, sha, nil
|
||||
}
|
||||
|
||||
@@ -497,6 +510,7 @@ func (a *Analyzer) getCommitSHAForTag(ctx context.Context, owner, repo, tagName
|
||||
if err != nil || tag.GetObject() == nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
return tag.GetObject().GetSHA()
|
||||
}
|
||||
|
||||
@@ -506,10 +520,11 @@ func (a *Analyzer) getLatestTag(ctx context.Context, owner, repo string) (versio
|
||||
PerPage: 10,
|
||||
})
|
||||
if err != nil || len(tags) == 0 {
|
||||
return "", "", fmt.Errorf("no releases or tags found")
|
||||
return "", "", errors.New("no releases or tags found")
|
||||
}
|
||||
|
||||
latestTag := tags[0]
|
||||
|
||||
return latestTag.GetName(), latestTag.GetCommit().GetSHA(), nil
|
||||
}
|
||||
|
||||
@@ -550,6 +565,7 @@ func (a *Analyzer) parseVersionParts(version string) []string {
|
||||
for len(parts) < versionPartsCount {
|
||||
parts = append(parts, "0")
|
||||
}
|
||||
|
||||
return parts
|
||||
}
|
||||
|
||||
@@ -564,6 +580,7 @@ func (a *Analyzer) determineUpdateType(currentParts, latestParts []string) strin
|
||||
if currentParts[2] != latestParts[2] {
|
||||
return updateTypePatch
|
||||
}
|
||||
|
||||
return updateTypeNone
|
||||
}
|
||||
|
||||
@@ -636,6 +653,7 @@ func (a *Analyzer) updateActionFile(filePath string, updates []PinnedUpdate) err
|
||||
indent := strings.Repeat(" ", len(line)-len(strings.TrimLeft(line, " ")))
|
||||
lines[i] = indent + usesFieldPrefix + update.NewUses
|
||||
update.LineNumber = i + 1 // Store line number for reference
|
||||
|
||||
break
|
||||
}
|
||||
}
|
||||
@@ -652,8 +670,9 @@ func (a *Analyzer) updateActionFile(filePath string, updates []PinnedUpdate) err
|
||||
if err := a.validateActionFile(filePath); err != nil {
|
||||
// Rollback on validation failure
|
||||
if rollbackErr := os.Rename(backupPath, filePath); rollbackErr != nil {
|
||||
return fmt.Errorf("validation failed and rollback failed: %v (original error: %w)", rollbackErr, err)
|
||||
return fmt.Errorf("validation failed and rollback failed: %w (original error: %w)", rollbackErr, err)
|
||||
}
|
||||
|
||||
return fmt.Errorf("validation failed, rolled back changes: %w", err)
|
||||
}
|
||||
|
||||
@@ -666,6 +685,7 @@ func (a *Analyzer) updateActionFile(filePath string, updates []PinnedUpdate) err
|
||||
// validateActionFile validates that an action.yml file is still valid after updates.
|
||||
func (a *Analyzer) validateActionFile(filePath string) error {
|
||||
_, err := a.parseCompositeAction(filePath)
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -680,6 +700,7 @@ func (a *Analyzer) enrichWithGitHubData(dep *Dependency, owner, repo string) err
|
||||
if cached, exists := a.Cache.Get(cacheKey); exists {
|
||||
if repository, ok := cached.(*github.Repository); ok {
|
||||
dep.Description = repository.GetDescription()
|
||||
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
package dependencies
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -16,6 +16,8 @@ import (
|
||||
)
|
||||
|
||||
func TestAnalyzer_AnalyzeActionFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
actionYML string
|
||||
@@ -62,6 +64,8 @@ func TestAnalyzer_AnalyzeActionFile(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create temporary action file
|
||||
tmpDir, cleanup := testutil.TempDir(t)
|
||||
defer cleanup()
|
||||
@@ -85,6 +89,7 @@ func TestAnalyzer_AnalyzeActionFile(t *testing.T) {
|
||||
// Check error expectation
|
||||
if tt.expectError {
|
||||
testutil.AssertError(t, err)
|
||||
|
||||
return
|
||||
}
|
||||
testutil.AssertNoError(t, err)
|
||||
@@ -100,6 +105,7 @@ func TestAnalyzer_AnalyzeActionFile(t *testing.T) {
|
||||
for i, expectedDep := range tt.expectedDeps {
|
||||
if i >= len(deps) {
|
||||
t.Errorf("expected dependency %s but got fewer dependencies", expectedDep)
|
||||
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(deps[i].Name+"@"+deps[i].Version, expectedDep) {
|
||||
@@ -115,6 +121,8 @@ func TestAnalyzer_AnalyzeActionFile(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_ParseUsesStatement(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
uses string
|
||||
@@ -161,6 +169,8 @@ func TestAnalyzer_ParseUsesStatement(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
owner, repo, version, versionType := analyzer.parseUsesStatement(tt.uses)
|
||||
|
||||
testutil.AssertEqual(t, tt.expectedOwner, owner)
|
||||
@@ -172,6 +182,8 @@ func TestAnalyzer_ParseUsesStatement(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_VersionChecking(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
version string
|
||||
@@ -227,6 +239,8 @@ func TestAnalyzer_VersionChecking(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
isPinned := analyzer.isVersionPinned(tt.version)
|
||||
isCommitSHA := analyzer.isCommitSHA(tt.version)
|
||||
isSemantic := analyzer.isSemanticVersion(tt.version)
|
||||
@@ -239,6 +253,8 @@ func TestAnalyzer_VersionChecking(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_GetLatestVersion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create mock GitHub client with test responses
|
||||
mockResponses := testutil.MockGitHubResponses()
|
||||
githubClient := testutil.MockGitHubClient(mockResponses)
|
||||
@@ -277,10 +293,13 @@ func TestAnalyzer_GetLatestVersion(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
version, sha, err := analyzer.getLatestVersion(tt.owner, tt.repo)
|
||||
|
||||
if tt.expectError {
|
||||
testutil.AssertError(t, err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
@@ -292,6 +311,8 @@ func TestAnalyzer_GetLatestVersion(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_CheckOutdated(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create mock GitHub client
|
||||
mockResponses := testutil.MockGitHubResponses()
|
||||
githubClient := testutil.MockGitHubClient(mockResponses)
|
||||
@@ -349,6 +370,8 @@ func TestAnalyzer_CheckOutdated(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_CompareVersions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
analyzer := &Analyzer{}
|
||||
|
||||
tests := []struct {
|
||||
@@ -391,6 +414,8 @@ func TestAnalyzer_CompareVersions(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
updateType := analyzer.compareVersions(tt.current, tt.latest)
|
||||
testutil.AssertEqual(t, tt.expectedType, updateType)
|
||||
})
|
||||
@@ -398,6 +423,8 @@ func TestAnalyzer_CompareVersions(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_GeneratePinnedUpdate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tmpDir, cleanup := testutil.TempDir(t)
|
||||
defer cleanup()
|
||||
|
||||
@@ -446,6 +473,8 @@ func TestAnalyzer_GeneratePinnedUpdate(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_WithCache(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Test that caching works properly
|
||||
mockResponses := testutil.MockGitHubResponses()
|
||||
githubClient := testutil.MockGitHubClient(mockResponses)
|
||||
@@ -470,12 +499,14 @@ func TestAnalyzer_WithCache(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_RateLimitHandling(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create mock client that returns rate limit error
|
||||
rateLimitResponse := &http.Response{
|
||||
StatusCode: 403,
|
||||
StatusCode: http.StatusForbidden,
|
||||
Header: http.Header{
|
||||
"X-RateLimit-Remaining": []string{"0"},
|
||||
"X-RateLimit-Reset": []string{fmt.Sprintf("%d", time.Now().Add(time.Hour).Unix())},
|
||||
"X-RateLimit-Reset": []string{strconv.FormatInt(time.Now().Add(time.Hour).Unix(), 10)},
|
||||
},
|
||||
Body: testutil.NewStringReader(`{"message": "API rate limit exceeded"}`),
|
||||
}
|
||||
@@ -508,6 +539,8 @@ func TestAnalyzer_RateLimitHandling(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAnalyzer_WithoutGitHubClient(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Test graceful degradation when GitHub client is not available
|
||||
analyzer := &Analyzer{
|
||||
GitHubClient: nil,
|
||||
@@ -546,6 +579,8 @@ func (t *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
|
||||
// TestNewAnalyzer tests the analyzer constructor.
|
||||
func TestNewAnalyzer(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create test dependencies
|
||||
mockResponses := testutil.MockGitHubResponses()
|
||||
githubClient := testutil.MockGitHubClient(mockResponses)
|
||||
@@ -597,6 +632,8 @@ func TestNewAnalyzer(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
analyzer := NewAnalyzer(tt.client, tt.repoInfo, tt.cache)
|
||||
|
||||
if tt.expectNotNil && analyzer == nil {
|
||||
|
||||
Reference in New Issue
Block a user