[testify-expert] Improve Test Quality: pkg/workflow/tools_timeout_validation_test.go

[github-actions]

Current State

• File: pkg/workflow/tools_timeout_validation_test.go (104 LOC, 1 test function, 3 sub-cases)
• Source pair: pkg/workflow/frontmatter_extraction_metadata.go (extractToolsStartupTimeout, timeout extraction logic in extractToolTimeouts)
• Testify usage: none — file imports no testify packages; all assertions use raw t.Fatalf/t.Errorf.

Strengths

• Table-driven structure with clear name/workflowMd/shouldCompile/errorContains fields.
• Covers a valid case, an invalid timeout: 0 case, and a combined timeout + startup-timeout valid case.
• Proper temp-file cleanup via defer os.Remove(...).

Prioritized Improvements

1. Missing/high-value tests

The source (extractToolsStartupTimeout in frontmatter_extraction_metadata.go) validates several cases the test file never exercises:

• startup-timeout: 0 or negative → error "must be at least 1 second, got %d"
• startup-timeout as a non-integer string that isn't a GitHub Actions expression → error "must be an integer or a GitHub Actions expression ... got string"
• startup-timeout as a valid GHA expression string (e.g. "${{ inputs.startup-timeout }}") → should compile successfully
• startup-timeout provided with an invalid type (e.g. a list/map) → error "got %T"
• Negative tools.timeout value (only 0 is currently tested)

Suggested additional table cases

{
 name: "invalid startup-timeout - zero",
 workflowMd: `---
on: workflow_dispatch
engine: claude
tools:
 startup-timeout: 0
 github:
---

# Test
`,
 shouldCompile: false,
 errorContains: "must be at least 1 second, got 0",
},
{
 name: "startup-timeout as GitHub Actions expression",
 workflowMd: `---
on:
 workflow_dispatch:
 inputs:
 startup-timeout:
 default: "120"
engine: claude
tools:
 startup-timeout: "${{ inputs.startup-timeout }}"
 github:
---

# Test
`,
 shouldCompile: true,
},
{
 name: "startup-timeout as invalid string",
 workflowMd: `---
on: workflow_dispatch
engine: claude
tools:
 startup-timeout: "not-a-number"
 github:
---

# Test
`,
 shouldCompile: false,
 errorContains: "must be an integer or a GitHub Actions expression",
},

2. Testify assertion upgrades

Replace manual if err != nil { t.Fatalf/Errorf } checks with require/assert for consistency with the rest of the pkg/workflow suite (see labels_validation_test.go for the pattern already used nearby).

Before / after

Before:

tmpFile, err := os.CreateTemp("", "test-timeout-validation-*.md")
if err != nil {
 t.Fatalf("Failed to create temp file: %v", err)
}
...
if tt.shouldCompile {
 if err != nil {
 t.Errorf("Expected workflow to compile successfully, but got error: %v", err)
 }
} else {
 if err == nil {
 t.Errorf("Expected workflow compilation to fail, but it succeeded")
 } else if !strings.Contains(err.Error(), tt.errorContains) {
 t.Errorf("Expected error to contain '%s', but got: %v", tt.errorContains, err)
 }
}

After:

tmpFile, err := os.CreateTemp("", "test-timeout-validation-*.md")
require.NoError(t, err, "Failed to create temp file")
...
if tt.shouldCompile {
 assert.NoError(t, err, "Expected workflow to compile successfully")
} else {
 require.Error(t, err, "Expected workflow compilation to fail")
 assert.Contains(t, err.Error(), tt.errorContains)
}

3. Table-driven refactors

The table is already good; no structural refactor needed beyond adding the missing cases above. Consider adding a startupTimeout field alongside errorContains if timeout and startup-timeout cases grow enough to warrant separating into two tables (TestToolsTimeoutValidation / TestToolsStartupTimeoutValidation) for clearer failure attribution.

4. Organization/readability

• Rename the single test function's sub-cases so timeout vs. startup-timeout intent is unambiguous (e.g. "invalid timeout - zero" vs. a to-be-added "invalid startup-timeout - zero" — already distinguishable, but keep this naming convention as more cases are added).
• Add a short comment above the test explaining that this test exercises the frontmatter tools.timeout / tools.startup-timeout schema+extraction validation end-to-end via full compilation, since a reader unfamiliar with the compiler pipeline may not realize why a full CompileWorkflow call is needed instead of calling extractToolTimeouts directly.

Acceptance Checklist

• Add test cases for startup-timeout zero/negative value
• Add test case for startup-timeout as valid GitHub Actions expression
• Add test case for startup-timeout as invalid non-numeric string
• Replace t.Fatalf/t.Errorf with require/assert from testify
• Run make test-unit and confirm all tests pass
• Run make fmt after edits

Generated by 🧪 Daily Testify Uber Super Expert · auto · 23 AIC · ⌖ 6.82 AIC · ⊞ 7.3K · ◷

• expires on Aug 6, 2026, 10:39 AM UTC-08:00

[pelikhan]

⚠️ Assignment failed: Failed to assign copilot coding agent to this issue.

Error: Connect Timeout Error

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · gpt54 · 3.45 AIC · ⌖ 7.84 AIC · ⊞ 7.6K · ◷

[github-actions]

🍪 Issue Monster selected this for Copilot

I have identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · gpt54 · 3.95 AIC · ⌖ 11.2 AIC · ⊞ 9.8K · ◷