Skip to content

[dead-code] chore: remove dead functions — 1 function removed#28257

Merged
pelikhan merged 1 commit intomainfrom
dead-code/remove-extractengineconfig-a8b3c4592d2d401e
Apr 24, 2026
Merged

[dead-code] chore: remove dead functions — 1 function removed#28257
pelikhan merged 1 commit intomainfrom
dead-code/remove-extractengineconfig-a8b3c4592d2d401e

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Dead Code Removal

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

Functions Removed

Function File
extractEngineConfig pkg/cli/audit_expanded.go

Tests Removed

None. Test files that called extractEngineConfig were updated to call extractEngineConfigWithInferredEngine directly (the function it was wrapping).

Verification

  • go build ./... — passes
  • go vet ./... — passes
  • go vet -tags=integration ./... — passes
  • make fmt — no changes needed
  • go test -run TestExtractEngineConfig ./pkg/cli/... — passes

Dead Function Count

  • Before this batch: 7 functions (5 are false positives used in WASM binary or non-test code)
  • Removed in this PR: 1 function
  • Remaining: ~6 functions (most are WASM/test-infrastructure that cannot be removed)

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

Generated by Dead Code Removal Agent · ● 1.1M ·

  • expires on Apr 27, 2026, 12:09 PM UTC

The extractEngineConfig function was a thin wrapper around
extractEngineConfigWithInferredEngine that was only called from test
files. Tests now call extractEngineConfigWithInferredEngine directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor Author

🎉 Great work, Dead Code Removal Agent! This PR is a clean, focused removal of the extractEngineConfig wrapper function — the diff is minimal, the test call sites are properly updated, and the verification steps are all documented. This looks ready for maintainer review and merge!

Generated by Contribution Check · ● 1.3M ·

@github-actions github-actions Bot added the lgtm label Apr 24, 2026
@pelikhan pelikhan marked this pull request as ready for review April 24, 2026 13:44
Copilot AI review requested due to automatic review settings April 24, 2026 13:44
@pelikhan pelikhan merged commit 4800975 into main Apr 24, 2026
19 checks passed
@pelikhan pelikhan deleted the dead-code/remove-extractengineconfig-a8b3c4592d2d401e branch April 24, 2026 13:44
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 an unreachable Go helper (extractEngineConfig) identified by static analysis, and updates tests to call the underlying function directly.

Changes:

  • Deleted the dead wrapper function extractEngineConfig from pkg/cli/audit_expanded.go.
  • Updated pkg/cli/audit_expanded_test.go to call extractEngineConfigWithInferredEngine directly.
Show a summary per file
File Description
pkg/cli/audit_expanded.go Removes the dead extractEngineConfig wrapper function.
pkg/cli/audit_expanded_test.go Updates tests to invoke extractEngineConfigWithInferredEngine directly.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment on lines 56 to 58
if tt.expectNil && tt.awInfoContent == "" {
result := extractEngineConfig("")
result := extractEngineConfigWithInferredEngine("", "")
assert.Nil(t, result, "Should return nil for empty logs path")
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The test is still named TestExtractEngineConfig, but it now directly calls extractEngineConfigWithInferredEngine (since extractEngineConfig was removed). Renaming the test (and any related subtest descriptions) would avoid confusion about which function/API is being exercised.

Copilot uses AI. Check for mistakes.
@github-actions github-actions Bot mentioned this pull request Apr 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
TestExtractEngineConfig pkg/cli/audit_expanded_test.go:16 ✅ Design None — well-structured table-driven test

Analysis Notes

This PR removes the dead function extractEngineConfig and updates TestExtractEngineConfig to call extractEngineConfigWithInferredEngine instead (3 lines added / 3 lines deleted). The test itself is high quality:

  • Table-driven with 5 cases covering happy-path, minimal config, empty/nil path, subdirectory lookup, and edge cases (expectNil: true)
  • Behavioral contract: asserts observable output (EngineID, Model) from real filesystem I/O — no mocking
  • Error coverage: includes nil/empty cases (expectNil: true)
  • Assertion messages: all assert.*/require.* calls include descriptive messages ✅
  • Build tag: //go:build !integration present on line 1 ✅
  • No mock libraries

Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 1 modified test function

Verdict

Check passed. 0% of new/modified tests are implementation tests (threshold: 30%). The change is a clean dead-code removal with a minimal, correct test update.


📖 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: §24892766039

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

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: 100/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). The single modified test is a well-structured table-driven behavioral test with proper assertion messages, build tags, and error/edge case coverage.

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