[cloclo] fix(types): use assert.InDelta and assert.Empty to fix lint in spec_test.go

[github-actions]

$(cat <<'EOF'
Fixes the lint-go failure in PR #27305 (spec-enforcer/2026-04-20-tty-types-workflow-b230cdde53b4c9a6).

What failed

The testifylint linter (enabled via enable-all: true in .golangci.yml) reported two categories of violations in pkg/types/spec_test.go:

• float-compare: assert.Equal was used to compare float64 values. The linter requires assert.InDelta or assert.InEpsilon for floating-point comparisons.
• nil-empty (sub-check of empty): assert.Equal(t, "", ...) was used to check for an empty string; the linter requires assert.Empty.

Changes

pkg/types/spec_test.go only:

• Replaced 10 assert.Equal(t, <float64>, ...) calls with assert.InDelta(t, <float64>, ..., 1e-9, ...) in TestSpec_Types_TokenWeights and TestSpec_Types_TokenClassWeights
• Replaced assert.Equal(t, "", cfg.Type, ...) with assert.Empty(t, cfg.Type, ...) in TestSpec_Types_ZeroValueSafety

Test plan

• CI lint-go job passes
• CI test job continues to pass (logic unchanged, only assertion style updated)

References:

• §24663160710 — original failing CGO run
• Fixes lint failure reported in PR [spec-enforcer] Enforce specifications for tty, types, workflow #27305
  EOF

🎤 Magnifique! Performance by /cloclo · ● 634.1K · ◷

• expires on Apr 22, 2026, 1:19 PM UTC

[Copilot]

Pull request overview

Updates specification-driven tests to satisfy testifylint requirements and keep spec tests aligned with documented package contracts.

Changes:

• Adds spec tests for tty, types, and workflow packages.
• Updates floating-point comparisons to use assert.InDelta and empty-string checks to use assert.Empty (per testifylint).
• Introduces/expands spec coverage for workflow permissions factories and safe-outputs config helpers.

Show a summary per file

File	Description
pkg/workflow/spec_test.go	Adds spec tests for permissions factory helpers and SafeOutputsConfigFromKeys.
pkg/types/spec_test.go	Adds/updates spec tests for MCP server/auth config and token weights, using linter-compliant assertions.
pkg/tty/spec_test.go	Adds spec tests for terminal detection behavior contracts.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (3)

pkg/types/spec_test.go:136

• In TestSpec_Types_MCPAuthConfig, the “auth without explicit audience” case expects Audience to be empty (wantAud: ""), but the package spec/README states the audience defaults to the server URL when omitted. This test currently contradicts that documented behavior (and the assertion message also mentions the default). Please align the expectation and/or the documentation so the spec test enforces the intended contract.

		{
			name: "auth without explicit audience",
			auth: types.MCPAuthConfig{
				Type: "github-oidc",
			},
			wantType: "github-oidc",
			wantAud:  "",
		},

pkg/types/spec_test.go:6

• The PR title/description say this is only a lint-fix in pkg/types/spec_test.go, but this PR also adds new spec tests in pkg/tty/spec_test.go and pkg/workflow/spec_test.go (and the full pkg/types/spec_test.go file). Please update the PR title/description to reflect the additional scope, or split the extra files into a separate PR if they’re unrelated to the lint fix.

//go:build !integration

package types_test

import (
	"encoding/json"

pkg/types/spec_test.go:88

• TestSpec_Types_BaseMCPServerConfig_JSONRoundTrip (and its comment) says it validates both json and yaml struct tags, but the test only does a JSON marshal/unmarshal. Either extend this test to also round-trip via YAML (or reflect on struct tags), or rename/reword it so it doesn’t claim YAML coverage.

// TestSpec_Types_BaseMCPServerConfig_JSONRoundTrip validates that BaseMCPServerConfig fields
// use both json and yaml struct tags as documented in the Design Notes section of the README.
// Spec: "All struct fields use both json and yaml struct tags so they can be round-tripped
// through both serialization formats."
func TestSpec_Types_BaseMCPServerConfig_JSONRoundTrip(t *testing.T) {
	original := types.BaseMCPServerConfig{
		Type:    "stdio",

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

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 76/100

⚠️ Acceptable, with suggestions

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

---

Test Classification Details

View all 17 test classifications

Test	File	Classification	Notes
TestSpec_PublicAPI_IsStdoutTerminal	pkg/tty/spec_test.go:15	✅ Design	Enforces consistency contract; no error case
TestSpec_PublicAPI_IsStderrTerminal	pkg/tty/spec_test.go:26	✅ Design	Same consistency contract as stdout variant
TestSpec_DesignDecision_NonTerminalReturnsFalse	pkg/tty/spec_test.go:39	✅ Design	Edge case: non-TTY returns false; uses t.Skip correctly
TestSpec_Types_BaseMCPServerConfig	pkg/types/spec_test.go:16	✅ Design	Table-driven; 3 rows (stdio, HTTP, container)
TestSpec_Types_BaseMCPServerConfig_JSONRoundTrip	pkg/types/spec_test.go:86	✅ Design	Verifies JSON serialization contract; includes error path
TestSpec_Types_MCPAuthConfig	pkg/types/spec_test.go:113	✅ Design	Table-driven; includes empty-audience edge case
TestSpec_Types_TokenWeights	pkg/types/spec_test.go:156	✅ Design	Verifies float fields via InDelta
TestSpec_Types_TokenClassWeights	pkg/types/spec_test.go:176	✅ Design	Verifies zero-value edge case + JSON field names
TestSpec_Types_ZeroValueSafety	pkg/types/spec_test.go:212	✅ Design	Zero-value edge case; prevents nil-dereference regressions
TestSpec_Permissions_ContentsWritePRWrite	pkg/workflow/spec_test.go:16	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsWriteIssuesWritePRWrite	pkg/workflow/spec_test.go:31	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsReadDiscussionsWrite	pkg/workflow/spec_test.go:49	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsReadIssuesWriteDiscussionsWrite	pkg/workflow/spec_test.go:64	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsReadPRWrite	pkg/workflow/spec_test.go:83	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsReadSecurityEventsWrite	pkg/workflow/spec_test.go:98	✅ Design	Verifies permission factory contract
TestSpec_Permissions_ContentsReadProjectsWrite	pkg/workflow/spec_test.go:115	✅ Design	Uses GetExplicit; verifies org-projects scope
TestSpec_SafeOutputs_SafeOutputsConfigFromKeys	pkg/workflow/spec_test.go:133	✅ Design	Table-driven; includes empty-key edge case

---

Flagged Tests — Suggestions for Review

⚠️ Duplicate cluster: 7 permission factory tests (pkg/workflow/spec_test.go)

TestSpec_Permissions_ContentsWritePRWrite through TestSpec_Permissions_ContentsReadProjectsWrite follow an identical structural pattern: call a factory, assert NotNil, then call p.Get() and assert the level. All 7 tests share this shape with only the permission scope constants differing.

What design invariant does this cover? Each test verifies that a named factory returns exactly the documented combination of permission scopes. This is valuable.

What would break if deleted? A factory that granted the wrong scope (e.g., write instead of read) would go undetected. High value individually.

Suggested improvement: Consider consolidating all 7 into a single table-driven test (TestSpec_Permissions_AllFactories) with one row per factory. This reduces duplication while preserving full coverage, and adds room to include a negative check — e.g., assert that a scope not in the combination returns (_, false) from p.Get(). Example:

func TestSpec_Permissions_AllFactories(t *testing.T) {
    tests := []struct {
        name        string
        factory     func() *workflow.Permissions
        wantScopes  map[workflow.PermissionScope]workflow.PermissionLevel
        absentScope workflow.PermissionScope // scope that must NOT be set
    }{
        {
            name:        "ContentsWritePRWrite",
            factory:     workflow.NewPermissionsContentsWritePRWrite,
            wantScopes:  map[...]{Contents: Write, PRs: Write},
            absentScope: workflow.PermissionIssues,
        },
        // ...
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            p := tt.factory()
            require.NotNil(t, p, ...)
            for scope, level := range tt.wantScopes {
                got, ok := p.Get(scope)
                assert.True(t, ok, ...)
                assert.Equal(t, level, got, ...)
            }
            _, ok := p.Get(tt.absentScope)
            assert.False(t, ok, "absent scope must not be set")
        })
    }
}

---

💡 Minor: TestSpec_PublicAPI_IsStdoutTerminal / TestSpec_PublicAPI_IsStderrTerminal (pkg/tty/spec_test.go)

These two tests are structurally identical, verifying the same "no-caching" invariant for stdout and stderr respectively. Only 2 tests, so below the 3+ duplication threshold — not penalized. But the pair could be a single table-driven test if more terminal types are ever added.

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 17 tests — all unit (//go:build !integration) ✅
• 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

---

Score Breakdown

Component	Score
Behavioral Coverage (17/17 design tests)	40/40
Error/Edge Case Coverage (6/17 tests have edge cases)	~11/30
Low Duplication (1 cluster × −5)	15/20
Proportional Growth (test-only PR, no inflation)	10/10
Total	76/100

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All tests verify behavioral contracts and all Go test files have the required //go:build !integration tag. The main opportunity is consolidating the 7 structurally-similar permission factory tests and adding negative assertions (absent scopes).

---

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

🧪 Test quality analysis by Test Quality Sentinel · ● 488.4K · ◷

[github-actions]

✅ Test Quality Sentinel: 76/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 17 tests verify behavioral contracts; all files have required build tags; no mock libraries used. See comment for improvement suggestions (consolidating the 7 permission factory tests into a table-driven test with negative assertions).