[formal-spec] Add replace-label formal model test suite (P1–P15 + edge cases) - #42527
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Go test file intended to codify formal-spec predicates for the replace_label safe-output type, including validation config assertions and a set of modeled behavioral invariants.
Changes:
- Adds
pkg/workflow/replace_label_formal_test.gowith tests coveringreplace_labelvalidation config (ValidationConfig) and compiler parsing for staged mode. - Adds a set of “formal” helper functions used to model allow/blocklist matching, gating checks, label-set computation, and targeting logic.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/replace_label_formal_test.go | Adds a formal-spec-oriented test suite for replace_label, including validation config checks and modeled predicate-based tests |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Low
| func formalRequiredNonEmptyLabel(s string) bool { | ||
| return strings.TrimSpace(s) != "" | ||
| } | ||
|
|
||
| func formalLabelAndRepoLengthsValid(labelToRemove, labelToAdd, repo string) bool { | ||
| return len(labelToRemove) <= 128 && len(labelToAdd) <= 128 && len(repo) <= 256 | ||
| } | ||
|
|
||
| func formalMatchAnyPattern(value string, patterns []string) bool { | ||
| for _, p := range patterns { | ||
| matched, err := path.Match(p, value) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if matched { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func formalValidateSingleLabel(labelName string, allowedPatterns, blockedPatterns []string, fieldName string) error { | ||
| if formalMatchAnyPattern(labelName, blockedPatterns) { | ||
| return fmt.Errorf("%s %q matches a blocked pattern", fieldName, labelName) | ||
| } | ||
| if len(allowedPatterns) > 0 && !formalMatchAnyPattern(labelName, allowedPatterns) { | ||
| return fmt.Errorf("%s %q is not in the allowed list", fieldName, labelName) | ||
| } | ||
| return nil | ||
| } |
| func formalMatchAnyPattern(value string, patterns []string) bool { | ||
| for _, p := range patterns { | ||
| matched, err := path.Match(p, value) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if matched { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
| func TestFormalReplaceLabelEdge_NonExistentLabelHTTP422(t *testing.T) { | ||
| outcome := formalReplaceLabelOutcome{Success: false, Skipped: false} | ||
| assert.False(t, outcome.Success) | ||
| assert.False(t, outcome.Skipped) | ||
| } |
|
Hey This PR looks ready for review:
Nothing stands out as needing changes — this is clean and well-scoped.
|
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… test, add scope doc comment Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Addressed all three in-scope review threads in commit
Local validation ( |
🤖 PR Triage
Score breakdown: Impact 22/50 · Urgency 10/30 · Quality 12/20 Rationale: Adds 21 formal-spec-aligned tests for
|
|
🎉 This pull request is included in a new release. Release: |
This PR adds a formal-spec-aligned Go test suite for
replace_label, mapping the extracted predicate set to concrete unit tests inpkg/workflow. It codifies the spec’s behavioral invariants around validation, gating, transition semantics, and cross-repo/target constraints.Formal predicate coverage
pkg/workflow/replace_label_formal_test.gowith 21 tests coveringP1..P15plus edge cases from the formal-spec issue.Policy and gating semantics
allowed-add,allowed-remove,blocked).required-labels,required-title-prefix) vs hard-error outcomes.Execution semantics invariants
(current - remove) ∪ {add}, deduplication, idempotent remove-missing behavior, and single-occurrence guarantee forlabel_to_add.Targeting and repo constraints
target-repo/allowed-repos) and target mode resolution (triggeringvs*).Spec/shape conformance checks
item_numberasIssueNumberOrTemporaryID) andReplaceLabelConfigfield presence expected by the spec.