Skip to content

Align compiler_filters test file with validation naming convention#10908

Merged
pelikhan merged 3 commits intomainfrom
copilot/add-test-file-for-compiler-filters
Jan 20, 2026
Merged

Align compiler_filters test file with validation naming convention#10908
pelikhan merged 3 commits intomainfrom
copilot/add-test-file-for-compiler-filters

Conversation

Copy link
Contributor

Copilot AI commented Jan 20, 2026

Validation files in pkg/workflow/ follow the pattern *_validation.go*_validation_test.go. The filter validation tests were named compiler_filters_test.go, breaking this convention.

Changes

  • Renamed: pkg/workflow/compiler_filters_test.gopkg/workflow/compiler_filters_validation_test.go

Context

Existing convention in the codebase:

dangerous_permissions_validation.go → dangerous_permissions_validation_test.go ✓
engine_validation.go                → engine_validation_test.go                ✓
features_validation.go              → features_validation_test.go              ✓
compiler_filters_validation.go      → compiler_filters_test.go                ✗

Now consistent:

compiler_filters_validation.go      → compiler_filters_validation_test.go     ✓

Test coverage remains comprehensive (327 lines, 25 test cases covering all validation paths).

Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Create missing test file for compiler_filters_validation.go</issue_title>
<issue_description>## Description

The validation file pkg/workflow/compiler_filters_validation.go has no test coverage despite being a validation-focused file. This is a critical gap for code that validates GitHub Actions requirements.

Problem

  • Zero test coverage for validation logic
  • File validates mutual exclusivity of event filters (branches vs branches-ignore, paths vs paths-ignore)
  • Risk: Validation bugs may slip into production
  • Cannot verify correctness of GitHub Actions requirement checks

Files Affected

  • pkg/workflow/compiler_filters_validation.go (106 lines, 31% comment density - excellent docs)
  • Missing: pkg/workflow/compiler_filters_validation_test.go (needs to be created)

Required Test Cases

Create comprehensive table-driven tests covering:

  1. Valid configurations:

    • ✅ branches only
    • ✅ branches-ignore only
    • ✅ paths only
    • ✅ paths-ignore only
    • ✅ no filters
    • ✅ multiple events with valid filters
  2. Invalid configurations (should error):

    • ❌ both branches and branches-ignore
    • ❌ both paths and paths-ignore
  3. Edge cases:

    • Empty filter arrays
    • Non-map event values
    • Missing 'on' key

Suggested Implementation

func TestValidateEventFilters(t *testing.T) {
    tests := []struct {
        name      string
        frontmatter map[string]any
        shouldErr bool
    }{
        {
            name: "valid - branches only",
            frontmatter: map[string]any{
                "on": map[string]any{
                    "push": map[string]any{
                        "branches": []string{"main"},
                    },
                },
            },
            shouldErr: false,
        },
        {
            name: "invalid - both branches and branches-ignore",
            frontmatter: map[string]any{
                "on": map[string]any{
                    "push": map[string]any{
                        "branches": []string{"main"},
                        "branches-ignore": []string{"dev"},
                    },
                },
            },
            shouldErr: true,
        },
        // ... more test cases
    }
    
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            err := ValidateEventFilters(tt.frontmatter)
            if tt.shouldErr {
                assert.Error(t, err, "Should error for mutually exclusive filters")
            } else {
                assert.NoError(t, err, "Should not error for valid filters")
            }
        })
    }
}

Success Criteria

  • Test file created: pkg/workflow/compiler_filters_validation_test.go
  • All validation logic paths covered with tests
  • Tests follow patterns in specs/testing.md (table-driven, testify assertions)
  • All tests pass: make test-unit
  • Clear test names explain what's being validated

Source

Extracted from Daily Compiler Code Quality Report discussion githubnext/gh-aw#10786

Priority

Critical - Validation logic without tests is a significant risk

Estimated Effort

1-2 hours (straightforward table-driven tests)

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 3, 2026, 2:10 PM UTC

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…o for consistency

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Add missing test file for compiler_filters_validation.go Align compiler_filters test file with validation naming convention Jan 20, 2026
Copilot AI requested a review from mnkiefer January 20, 2026 22:12
@pelikhan pelikhan marked this pull request as ready for review January 20, 2026 22:42
@pelikhan pelikhan merged commit 76148c6 into main Jan 20, 2026
48 checks passed
@pelikhan pelikhan deleted the copilot/add-test-file-for-compiler-filters branch January 20, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Create missing test file for compiler_filters_validation.go

3 participants