Skip to content

[dead-code] chore: remove dead functions — 2 functions removed#29929

Merged
pelikhan merged 1 commit intomainfrom
dead-code/remove-dead-functions-20260503-0942123b54a73024
May 3, 2026
Merged

[dead-code] chore: remove dead functions — 2 functions removed#29929
pelikhan merged 1 commit intomainfrom
dead-code/remove-dead-functions-20260503-0942123b54a73024

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented May 3, 2026

Dead Code Removal

This PR removes unreachable Go functions identified by the deadcode static analyzer.

Functions Removed

Function File
JobManager.RenderToYAML pkg/workflow/jobs.go
JobManager.renderJob pkg/workflow/jobs.go

Tests Removed

  • TestJobManager_RenderToYAML — exclusively tested the removed RenderToYAML method
  • Removed RenderToYAML/renderJob call sites in 7 other test files, replacing them with WriteJobsYAML/renderJobTo equivalents

Verification

  • go build ./... — passes
  • go vet ./... — passes
  • go vet -tags=integration ./... — passes
  • make fmt — no changes needed
  • ✅ Targeted tests (TestJobManager*, TestIfExpression*, etc.) — pass

Dead Function Count

  • Before this batch: 9 functions reported (7 excluding WASM-live functions)
  • Removed in this PR: 2 functions
  • Skipped: 5 (WASM-live: CompileToYAML, ParseWorkflowString, WithSkipValidation, WithNoEmit, WithWorkflowIdentifier; WASM code path: validatePiEngineRequirements; high change count: WithVersion — 224+ test call sites)

Automated by Dead Code Removal workflow — https://github.com/github/gh-aw/actions/runs/25281792468

Generated by Dead Code Removal Agent · ● 4.1M ·

  • expires on May 6, 2026, 2:40 PM UTC

Remove JobManager.RenderToYAML and JobManager.renderJob which are
unreachable from production binary entry points per deadcode analysis.
Replace test callers with WriteJobsYAML and renderJobTo equivalents.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review May 3, 2026 14:50
Copilot AI review requested due to automatic review settings May 3, 2026 14:50
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

Removes dead/unreachable JobManager YAML-rendering helpers and updates tests to use the remaining builder-based rendering APIs.

Changes:

  • Removed JobManager.RenderToYAML and JobManager.renderJob from pkg/workflow/jobs.go.
  • Updated multiple tests to render YAML via WriteJobsYAML / renderJobTo using a strings.Builder.
  • Removed the now-obsolete TestJobManager_RenderToYAML test and adjusted WriteJobsYAML test expectations accordingly.
Show a summary per file
File Description
pkg/workflow/jobs.go Deletes dead RenderToYAML and renderJob methods, leaving builder-based rendering paths.
pkg/workflow/jobs_test.go Removes the RenderToYAML-specific test and updates WriteJobsYAML test commentary/assertions accordingly.
pkg/workflow/zizmor_annotation_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.
pkg/workflow/threat_detection_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.
pkg/workflow/safe_outputs_call_workflow_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.
pkg/workflow/main_job_env_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.
pkg/workflow/if_expression_clean_test.go Switches single-job rendering from renderJob to renderJobTo + strings.Builder.
pkg/workflow/compiler_jobs_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.
pkg/workflow/call_workflow_permissions_test.go Switches YAML generation from RenderToYAML to WriteJobsYAML + strings.Builder.

Copilot's findings

Tip

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

  • Files reviewed: 9/9 changed files
  • Comments generated: 0

@github-actions github-actions Bot mentioned this pull request May 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions Bot commented May 3, 2026

🧪 Test Quality Sentinel Report

Test Quality Score: 90/100

Excellent — Dead-code removal with clean test maintenance

Metric Value
New test functions analyzed 0
Modified test functions (API adaptation) 8
Removed test functions 1 (TestJobManager_RenderToYAML)
✅ Design tests (behavioral contracts) N/A — no new tests added
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases N/A
Duplicate test clusters 0
Test inflation detected No (net −194 test lines vs −21 production lines)
🚨 Coding-guideline violations None

Test Classification Details

No new test functions were added in this PR. All test changes fall into two categories:

Change Files Classification
API call migration (RenderToYAML()WriteJobsYAML(&buf)) 7 files ✅ Correct maintenance
API call migration (renderJob()renderJobTo(&buf, job)) if_expression_clean_test.go ✅ Correct maintenance
Test function removal jobs_test.goTestJobManager_RenderToYAML ✅ Appropriate (dead code removed)
Redundant cross-check assertion removed jobs_test.goTestJobManager_WriteJobsYAML ✅ Appropriate (comparing against removed function)

Flagged Tests — Requires Review

No tests flagged. All modifications are syntactic API adaptations. The behavioral assertions in each test remain intact and unchanged.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 8 modified test files — all unit (//go:build !integration), no new files added
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 changes

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected. This PR correctly removes tests for dead code and adapts API calls in existing tests without weakening behavioral coverage.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.


References: §25282301221

🧪 Test quality analysis by Test Quality Sentinel · ● 531.9K ·

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). This is a dead-code removal PR with clean test maintenance: existing test API calls updated, one test appropriately removed for the deleted dead function. No coding-guideline violations detected.

@pelikhan pelikhan merged commit 5ed03e9 into main May 3, 2026
40 of 41 checks passed
@pelikhan pelikhan deleted the dead-code/remove-dead-functions-20260503-0942123b54a73024 branch May 3, 2026 14:56
Copilot AI pushed a commit that referenced this pull request May 3, 2026
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants