Skip to content

[file-diet] File Diet: pkg/workflow/compiler_custom_jobs.go (1142 lines) needs refactoring #53387

Description

@github-actions

Overview

The file pkg/workflow/compiler_custom_jobs.go has grown to 1142 lines, making it difficult to maintain and test. This task involves refactoring it into smaller, focused files with improved test coverage.

Current State

  • File: pkg/workflow/compiler_custom_jobs.go
  • Size: 1142 lines
  • Test Coverage: pkg/workflow/compiler_custom_jobs_test.go exists at 1540 lines (~1.35x test-to-source ratio — healthy)
  • Complexity: 37 top-level functions spanning at least 5 distinct concerns: custom job orchestration, property extraction, step/execution configuration, builtin job augmentation, and step validation/insertion helpers.
Full File Analysis

Function distribution (37 functions total)

1. Custom job orchestration (lines 26–172, ~150 LOC)

  • buildCustomJobs, getCustomJobDependencySets, shouldSkipCustomJob, buildCustomJob, extractCustomJobNeeds, applyAutomaticActivationDependency
  • Responsibility: top-level driver that iterates configured custom jobs and wires dependency sets.

2. Custom job property extraction (lines 173–432, ~260 LOC)

  • extractCustomJobProperties, extractCustomJobCoreProperties, extractCustomJobRunsOn, extractCustomJobTimeoutMinutes, extractCustomJobConcurrency, extractCustomJobEnv, extractCustomJobContainer, extractCustomJobServices, extractCustomJobContinueOnError, extractCustomJobEnvironment, extractCustomJobOutputs
  • Responsibility: parses YAML config map fields (runs-on, timeout-minutes, concurrency, env, container, services, environment, outputs) into the Job struct. Highly repetitive per-field extraction pattern — a good candidate for a shared helper/table-driven approach.

3. Job execution / steps configuration (lines 433–597, ~165 LOC)

  • configureCustomJobExecution, configureCustomReusableWorkflow, configureCustomJobSteps, formatIndentedYAMLField
  • Responsibility: decides between reusable-workflow (uses:) jobs vs. step-based jobs, and formats YAML step blocks.

4. Builtin job augmentation (lines 598–939, ~340 LOC — largest cluster)

  • applyBuiltinJobPreSteps, insertActivationStepsBeforeArtifactStaging, normalizeBuiltinJobAlias, extractBuiltinJobNeedsAugmentation, extractBuiltinJobIfAugmentation, applyBuiltinJobAugmentations, applyBuiltinJobPermissionsAugmentation, guardIfAgainstStatusFuncBypass, ifExpressionContainsStatusFunc
  • Responsibility: allows workflow authors to augment already-generated builtin jobs (needs/if/permissions/pre-steps) rather than creating new jobs. Distinct domain from "custom job" creation — this is really "builtin job augmentation," and is the single largest concern in the file.

5. Step validation & insertion helpers (lines 940–1142, ~200 LOC)

  • validateRestrictedBuiltinSetupSteps, validateRestrictedBuiltinSteps, insertSetupStepsAtStart, insertPreStepsAtEarliestBoundary, extractPinnedJobSteps, ensureCheckoutPersistCredentials, isCheckoutAction
  • Responsibility: low-level step-list manipulation (insertion ordering, checkout-action detection, restricted-step validation) shared by both custom-job and builtin-job augmentation paths.

Coupling notes

Refactoring Strategy

Proposed File Splits

  1. compiler_custom_jobs.go (kept, trimmed to orchestration only)

    • Functions: buildCustomJobs, getCustomJobDependencySets, shouldSkipCustomJob, buildCustomJob, extractCustomJobNeeds, applyAutomaticActivationDependency
    • Responsibility: top-level driver for building custom jobs from workflow config
    • Estimated LOC: ~150
  2. compiler_custom_job_properties.go

    • Functions: extractCustomJobProperties, extractCustomJobCoreProperties, extractCustomJobRunsOn, extractCustomJobTimeoutMinutes, extractCustomJobConcurrency, extractCustomJobEnv, extractCustomJobContainer, extractCustomJobServices, extractCustomJobContinueOnError, extractCustomJobEnvironment, extractCustomJobOutputs
    • Responsibility: parse individual YAML config fields into Job struct fields
    • Estimated LOC: ~260
  3. compiler_custom_job_execution.go

    • Functions: configureCustomJobExecution, configureCustomReusableWorkflow, configureCustomJobSteps, formatIndentedYAMLField
    • Responsibility: decide reusable-workflow vs. step-based execution and render step YAML
    • Estimated LOC: ~165
  4. compiler_builtin_job_augmentation.go

    • Functions: applyBuiltinJobPreSteps, insertActivationStepsBeforeArtifactStaging, normalizeBuiltinJobAlias, extractBuiltinJobNeedsAugmentation, extractBuiltinJobIfAugmentation, applyBuiltinJobAugmentations, applyBuiltinJobPermissionsAugmentation, guardIfAgainstStatusFuncBypass, ifExpressionContainsStatusFunc
    • Responsibility: augment already-generated builtin jobs (needs/if/permissions/pre-steps)
    • Estimated LOC: ~340 (may warrant a further split if it stays this large — e.g. separating the if-guard logic (guardIfAgainstStatusFuncBypass, ifExpressionContainsStatusFunc) into its own small file)
  5. compiler_job_step_helpers.go

    • Functions: validateRestrictedBuiltinSetupSteps, validateRestrictedBuiltinSteps, insertSetupStepsAtStart, insertPreStepsAtEarliestBoundary, extractPinnedJobSteps, ensureCheckoutPersistCredentials, isCheckoutAction
    • Responsibility: shared low-level step-list manipulation and validation used by both custom job and builtin augmentation paths
    • Estimated LOC: ~200

Shared Utilities

  • Extract the common configMap field-extraction pattern (used repeatedly in compiler_custom_job_properties.go) into a small generic helper in pkg/workflow/config_map_helpers.go (or add to existing helpers file) to reduce duplication across the ~11 extraction functions.

Interface Abstractions

  • No new interfaces are strictly required; this is primarily a cohesion-based split by responsibility. Keep all functions as methods/functions on *Compiler as they are today to avoid changing the public API.
Test Coverage Plan

Split pkg/workflow/compiler_custom_jobs_test.go (1540 lines) to mirror the new source files:

  1. compiler_custom_jobs_test.go (trimmed)

    • Test cases: buildCustomJobs orchestration, dependency-set computation, skip logic, activation dependency wiring
    • Target coverage: >80%
  2. compiler_custom_job_properties_test.go

    • Test cases: each extraction function with valid/invalid/missing config values (runs-on, timeout, concurrency, env, container, services, continue-on-error, environment, outputs)
    • Target coverage: >80%
  3. compiler_custom_job_execution_test.go

    • Test cases: reusable workflow vs. step-based execution selection, YAML step formatting/indentation edge cases
    • Target coverage: >80%
  4. compiler_builtin_job_augmentation_test.go

    • Test cases: pre-step insertion ordering, alias normalization, needs/if/permissions augmentation, if-guard bypass detection
    • Target coverage: >80%
  5. compiler_job_step_helpers_test.go

    • Test cases: restricted step validation (setup/other), step insertion ordering, checkout-action detection, persist-credentials enforcement
    • Target coverage: >80%

Implementation Guidelines

  1. Preserve Behavior: Ensure all existing functionality works identically
  2. Maintain Exports: Keep public API unchanged (exported functions/types)
  3. Add Tests First: Write tests for each new file before refactoring
  4. Incremental Changes: Split one module at a time
  5. Run Tests Frequently: Verify make test-unit passes after each split
  6. Update Imports: Ensure all import paths are correct
  7. Document Changes: Add comments explaining module boundaries

Acceptance Criteria

  • Original file is split into 5 focused files
  • Each new file is under 500 lines (note: compiler_builtin_job_augmentation.go may need a further sub-split to hit this target)
  • All tests pass (make test-unit)
  • Test coverage is ≥80% for new files
  • No breaking changes to public API
  • Code passes linting (make lint)
  • Build succeeds (make build)
Additional Context
  • Repository Guidelines: Follow patterns in .github/agents/developer.instructions.agent.md
  • Code Organization: Prefer many small files grouped by functionality
  • Testing: Match existing test patterns in pkg/workflow/*_test.go

Priority: Medium
Effort: Medium (5-way split across ~1140 LOC + matching test split, some functions have tight coupling via *Compiler method receiver and shared helpers)
Expected Impact: Improved maintainability, easier testing, reduced complexity

Generated by 🧹 Daily File Diet · auto · 71.1 AIC · ⌖ 3.45 AIC · ⊞ 10K ·

  • expires on Aug 19, 2026, 5:04 AM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions