Fix the failure for lint/unit/integration

.golangci.yml

@@ -1,29 +1,30 @@
 # golangci-lint configuration for HyperFleet Pull Secret Job
 # https://golangci-lint.run/usage/configuration/
 
+version: 2
+
 run:
   timeout: 5m
   tests: true
   modules-download-mode: readonly
 
 linters:
   enable:
-    - gofmt          # Checks whether code was gofmt-ed
-    - goimports      # Checks import statements are formatted according to the 'goimport' command
     - govet          # Reports suspicious constructs
     - errcheck       # Checks for unchecked errors
     - staticcheck    # Static analysis checks
     - unused         # Checks for unused constants, variables, functions and types
-    - gosimple       # Simplify code
     - ineffassign    # Detects ineffectual assignments
-    - typecheck      # Type-checks Go code
     - misspell       # Finds commonly misspelled English words
     - revive         # Fast, configurable, extensible, flexible, and beautiful linter for Go
     - gocritic       # Provides diagnostics that check for bugs, performance and style issues
 
+formatters:
+  enable:
+    - gofmt          # Checks whether code was gofmt-ed
+    - goimports      # Checks import statements are formatted according to the 'goimport' command
+
 linters-settings:
-  gofmt:
-    simplify: true
 
   govet:
     enable:
@@ -58,14 +59,9 @@ issues:
       linters:
         - errcheck
         - gosec
-    # Exclude pkg/job and pkg/config (external framework code)
-    - path: pkg/
-      linters:
-        - revive
-        - goimports
 
 output:
   formats:
-    - format: colored-line-number
+    stdout: colored-line-number
   print-issued-lines: true
   print-linter-name: true

Makefile

@@ -39,6 +39,7 @@ help: ## Display this help
 	@echo "Build Targets:"
 	@echo "  make binary               compile pull-secret binary"
 	@echo "  make test                 run unit tests with coverage"
+	@echo "  make test-integration     run integration tests"
 	@echo "  make lint                 run golangci-lint"
 	@echo "  make image                build container image"
 	@echo "  make image-push           build and push container image"
@@ -48,6 +49,7 @@ help: ## Display this help
 	@echo "Examples:"
 	@echo "  make binary"
 	@echo "  make test"
+	@echo "  make test-integration"
 	@echo "  make lint"
 	@echo "  make image IMAGE_TAG=v1.0.0"
 	@echo "  make image-push IMAGE_TAG=v1.0.0"
@@ -88,16 +90,31 @@ binary: check-gopath
 # Test & Lint Targets
 ####################
 
-# Run unit tests with coverage
+# Run unit tests with coverage (excludes test/ directory)
 test:
-	@echo "Running tests with coverage..."
-	go test -v -race -coverprofile=coverage.txt -covermode=atomic ./...
+	@echo "Running unit tests with coverage..."
+	go test -v -race -coverprofile=coverage.txt -covermode=atomic $$(go list ./... | grep -v '/test')
 	@echo ""
 	@echo "Coverage report generated: coverage.txt"
 	@echo "View HTML coverage: go tool cover -html=coverage.txt"
 	@echo ""
 .PHONY: test
 
+# Run integration tests
+test-integration:
+	@echo "Running integration tests..."
+	@if [ -n "$$(find ./test -name '*_test.go' 2>/dev/null)" ]; then \
+		go test -v -race ./test/...; \
+		echo ""; \
+		echo "Integration tests complete."; \
+		echo ""; \
+	else \
+		echo "No integration tests found in ./test/"; \
+		echo "Create integration tests in ./test/ directory."; \
+		echo ""; \
+	fi
+.PHONY: test-integration
+
 # Run golangci-lint
 # Install: https://golangci-lint.run/usage/install/
 lint:

OWNERS

@@ -0,0 +1,33 @@
+approvers:
+- "86254860"
+- AlexVulaj
+- aredenba-rh
+- ciaranRoche
+- crizzo71
+- jsell-rh
+- mbrudnoy
+- Mischulee
+- rafabene
+- rh-amarin
+- tirthct
+- vkareh
+- xueli181114
+- yasun1
+- yingzhanredhat
+
+reviewers:
+- "86254860"
+- AlexVulaj
+- aredenba-rh
+- ciaranRoche
+- crizzo71
+- jsell-rh
+- mbrudnoy
+- Mischulee
+- rafabene
+- rh-amarin
+- tirthct
+- vkareh
+- xueli181114
+- yasun1
+- yingzhanredhat

pkg/config/job.go

@@ -1,19 +1,23 @@
+// Package config provides configuration types and utilities for job execution.
 package config
 
 import "github.com/spf13/pflag"
 
+// JobConfig holds the configuration options for job execution.
 type JobConfig struct {
 	DryRun      bool `json:"dry_run"`
 	WorkerCount int  `json:"worker_count"`
 }
 
+// NewJobConfig creates a new JobConfig with default values.
 func NewJobConfig() *JobConfig {
 	return &JobConfig{
 		DryRun:      true,
 		WorkerCount: 1,
 	}
 }
 
+// AddFlags registers the job configuration flags with the provided flag set.
 func (c *JobConfig) AddFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&c.DryRun, "dry-run", c.DryRun, "Show what would be changed by a run of this script.")
 	fs.IntVar(&c.WorkerCount, "worker-count", c.WorkerCount, "Number of concurrent workers.")

pkg/config/job_test.go

@@ -0,0 +1,64 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/spf13/pflag"
+)
+
+func TestNewJobConfig(t *testing.T) {
+	config := NewJobConfig()
+
+	if config == nil {
+		t.Fatal("NewJobConfig() returned nil")
+	}
+
+	if !config.DryRun {
+		t.Errorf("Expected DryRun to be true by default, got false")
+	}
+
+	if config.WorkerCount != 1 {
+		t.Errorf("Expected WorkerCount to be 1 by default, got %d", config.WorkerCount)
+	}
+}
+
+func TestJobConfig_AddFlags(t *testing.T) {
+	config := NewJobConfig()
+	fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
+
+	config.AddFlags(fs)
+
+	// Verify flags were added
+	if fs.Lookup("dry-run") == nil {
+		t.Error("Expected 'dry-run' flag to be registered")
+	}
+
+	if fs.Lookup("worker-count") == nil {
+		t.Error("Expected 'worker-count' flag to be registered")
+	}
+
+	// TODO: Add more comprehensive flag parsing tests
+}
+
+func TestJobConfig_FlagParsing(t *testing.T) {
+	config := NewJobConfig()
+	fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
+
+	config.AddFlags(fs)
+
+	// Test parsing custom values
+	args := []string{"--dry-run=false", "--worker-count=5"}
+	if err := fs.Parse(args); err != nil {
+		t.Fatalf("Failed to parse flags: %v", err)
+	}
+
+	if config.DryRun {
+		t.Errorf("Expected DryRun to be false after parsing, got true")
+	}
+
+	if config.WorkerCount != 5 {
+		t.Errorf("Expected WorkerCount to be 5 after parsing, got %d", config.WorkerCount)
+	}
+
+	// TODO: Add edge case tests (negative worker count, etc.)
+}

pkg/job/job.go

@@ -1,3 +1,4 @@
+// Package job provides a framework for defining and executing concurrent jobs with task queues and worker pools.
 package job
 
 import (
@@ -43,39 +44,47 @@ type CommandBuilder struct {
 	// panicHandler is an optional function that accepts interface and can deal with it how it wants.
 	// An example use can be to capture any errors and report it to sentry. Not setting panicHandler means any panics
 	// encountered will be silently recovered.
-	panicHandler    func(ctx context.Context, any interface{})
+	panicHandler    func(ctx context.Context, panicValue interface{})
 	metricsReporter MetricsReporter
 }
 
+// SetRegistry sets the job registry for the command builder.
 func (b *CommandBuilder) SetRegistry(registry JobRegistry) *CommandBuilder {
 	b.registry = registry
 	return b
 }
 
+// SetContext sets the context for the command builder.
 func (b *CommandBuilder) SetContext(ctx context.Context) *CommandBuilder {
 	b.ctx = ctx
 	return b
 }
 
+// SetBeforeJob sets the hook function to execute before job execution.
 func (b *CommandBuilder) SetBeforeJob(fn func(ctx context.Context) error) *CommandBuilder {
 	b.beforeJob = fn
 	return b
 }
+
+// SetAfterJob sets the hook function to execute after job execution.
 func (b *CommandBuilder) SetAfterJob(fn func(ctx context.Context)) *CommandBuilder {
 	b.afterJob = fn
 	return b
 }
 
-func (b *CommandBuilder) SetPanicHandler(fn func(ctx context.Context, any interface{})) *CommandBuilder {
+// SetPanicHandler sets the panic handler function for the command builder.
+func (b *CommandBuilder) SetPanicHandler(fn func(ctx context.Context, panicValue interface{})) *CommandBuilder {
 	b.panicHandler = fn
 	return b
 }
 
+// SetMetricsReporter sets the metrics reporter for the command builder.
 func (b *CommandBuilder) SetMetricsReporter(reporter MetricsReporter) *CommandBuilder {
 	b.metricsReporter = reporter
 	return b
 }
 
+// Build creates and returns a Cobra command with all registered jobs as subcommands.
 func (b *CommandBuilder) Build() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "run-job",
@@ -89,7 +98,7 @@ func (b *CommandBuilder) Build() *cobra.Command {
 			Long: job.GetMetadata().Description,
 			// We don't need this info if job fails.
 			SilenceUsage: true,
-			RunE: func(cmd *cobra.Command, args []string) error {
+			RunE: func(_ *cobra.Command, _ []string) error {
 				err := validateJob(job)
 				if err != nil {
 					return err
@@ -118,14 +127,19 @@ func validateJob(job Job) error {
 	return nil
 }
 
+// JobRegistry maintains a collection of jobs that can be registered and executed.
+//
+//nolint:revive // JobRegistry is preferred over Registry for clarity in external usage
 type JobRegistry struct {
 	jobs []Job
 }
 
+// NewJobRegistry creates a new job registry.
 func NewJobRegistry() *JobRegistry {
 	return &JobRegistry{}
 }
 
+// AddJob adds a job to the registry.
 func (r *JobRegistry) AddJob(job Job) {
 	if job == nil {
 		return
@@ -171,7 +185,7 @@ func newTaskQueue() *taskQueue {
 type workerPool struct {
 	Queue            *taskQueue
 	Workers          int
-	PanicHandler     func(ctx context.Context, any interface{})
+	PanicHandler     func(ctx context.Context, panicValue interface{})
 	MetricsCollector *MetricsCollector
 }
 
@@ -192,11 +206,11 @@ func (wp *workerPool) Run(ctx context.Context) {
 					return
 				}
 				func() {
-					taskId := ksuid.New().String()
+					taskID := ksuid.New().String()
 
 					taskCtx := AddTraceContext(ctx, "workerId", strconv.Itoa(workerId))
 					taskCtx = AddTraceContext(taskCtx, "taskName", task.TaskName())
-					taskCtx = AddTraceContext(taskCtx, "taskId", taskId)
+					taskCtx = AddTraceContext(taskCtx, "taskId", taskID)
 
 					defer func(taskCtx context.Context) {
 						if err := recover(); err != nil {
@@ -208,7 +222,7 @@ func (wp *workerPool) Run(ctx context.Context) {
 						}
 					}(taskCtx)
 
-					logger.NewOCMLogger(taskCtx).Contextual().Info("Processing task", "workerId", workerId, "taskId", taskId)
+					logger.NewOCMLogger(taskCtx).Contextual().Info("Processing task", "workerId", workerId, "taskId", taskID)
 					err := task.Process(taskCtx)
 					if err != nil {
 						wp.MetricsCollector.IncTaskFailed()
@@ -235,7 +249,7 @@ var _ runner = &TestRunner{}
 type jobRunner struct {
 	BeforeJob       func(ctx context.Context) error
 	AfterJob        func(ctx context.Context)
-	PanicHandler    func(ctx context.Context, any interface{})
+	PanicHandler    func(ctx context.Context, panicValue interface{})
 	MetricsReporter MetricsReporter
 }
 
@@ -283,7 +297,7 @@ func (jr jobRunner) Run(ctx context.Context, job Job, workerCount int) error {
 	}
 	for _, task := range tasks {
 		taskQueue.Add(task)
-		taskTotal += 1
+		taskTotal++
 	}
 	metricsCollector := NewMetricsCollector(job.GetMetadata().Use)
 	metricsCollector.SetTaskTotal(uint32(taskTotal))
@@ -320,6 +334,7 @@ func (jr jobRunner) Run(ctx context.Context, job Job, workerCount int) error {
 // TestRunner is a lightweight JobRunner implementation to enable for easy testing of job logic.
 type TestRunner struct{}
 
+// Run executes the job in test mode without lifecycle hooks.
 func (tr TestRunner) Run(ctx context.Context, job Job, workerCount int) error {
 	taskTotal := 0
 	taskQueue := newTaskQueue()
@@ -331,14 +346,19 @@ func (tr TestRunner) Run(ctx context.Context, job Job, workerCount int) error {
 	}
 	for _, task := range tasks {
 		taskQueue.Add(task)
-		taskTotal += 1
+		taskTotal++
 	}
 	metricsCollector := NewMetricsCollector(job.GetMetadata().Use)
 	metricsCollector.SetTaskTotal(uint32(taskTotal))
 
 	pool := workerPool{Queue: taskQueue, Workers: workerCount, PanicHandler: nil, MetricsCollector: metricsCollector}
 	pool.Run(ctx)
 
+	if metricsCollector.taskTotal == 0 {
+		// No tasks to run
+		return nil
+	}
+	// For now return error when all tasks fail. This can be configurable for e.g. return error when 80% of tasks fail.
 	if metricsCollector.taskFailed == metricsCollector.taskTotal {
 		err := errors.New("all tasks failed")
 		return err