Skip to content

Commit 0cae2ba

Browse files
TristanSpeakEasyTristanSpeakEasy
authored
refactor: move to surfacing core validation errors through the high model validation methods (#15)
1 parent 430acc3 commit 0cae2ba

19 files changed

+360
-93
lines changed

‎.github/workflows/test.yaml

Lines changed: 143 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,163 @@ on:
99
jobs:
1010
test-and-build:
1111
runs-on: ubuntu-latest
12+
permissions:
13+
contents: read
14+
pull-requests: write
1215

1316
steps:
14-
- uses: actions/checkout@v3
17+
- uses: actions/checkout@v4
1518

1619
- name: Set up Go
17-
uses: actions/setup-go@v4
20+
uses: actions/setup-go@v5
1821
with:
1922
go-version: "1.23"
2023

2124
- name: Install dependencies
2225
run: go mod download
2326

2427
- name: Run golangci-lint
25-
uses: golangci/golangci-lint-action@v3
28+
uses: golangci/golangci-lint-action@v6
2629
with:
2730
version: latest
2831

29-
- name: Run tests
30-
run: go test -v ./...
32+
- name: Run tests with coverage
33+
run: |
34+
go test -v -race -coverprofile=coverage.out -covermode=atomic ./...
35+
go tool cover -html=coverage.out -o coverage.html
36+
37+
- name: Calculate coverage
38+
id: coverage
39+
run: |
40+
COVERAGE=$(go tool cover -func=coverage.out | grep total | awk '{print $3}')
41+
echo "coverage=$COVERAGE" >> $GITHUB_OUTPUT
42+
echo "Coverage: $COVERAGE"
43+
44+
- name: Get main branch coverage
45+
if: github.event_name == 'pull_request'
46+
id: main-coverage
47+
run: |
48+
# Store current working directory
49+
CURRENT_DIR=$(pwd)
50+
51+
# Fetch main branch
52+
git fetch origin main:main
53+
54+
# Checkout main branch in a temporary directory
55+
git worktree add /tmp/main-branch main
56+
57+
# Run tests on main branch to get coverage
58+
cd /tmp/main-branch
59+
go test -coverprofile=main-coverage.out -covermode=atomic ./... > /dev/null 2>&1 || echo "Main branch tests failed"
60+
61+
if [ -f main-coverage.out ]; then
62+
MAIN_COVERAGE=$(go tool cover -func=main-coverage.out | grep total | awk '{print $3}' || echo "0.0%")
63+
echo "main-coverage=$MAIN_COVERAGE" >> $GITHUB_OUTPUT
64+
echo "Main branch coverage: $MAIN_COVERAGE"
65+
66+
# Copy main coverage file back to current directory
67+
cp main-coverage.out "$CURRENT_DIR/main-coverage.out"
68+
else
69+
echo "main-coverage=0.0%" >> $GITHUB_OUTPUT
70+
echo "Could not get main branch coverage"
71+
fi
72+
73+
# Return to original directory
74+
cd "$CURRENT_DIR"
75+
76+
# Clean up worktree (force removal to handle modified files)
77+
git worktree remove --force /tmp/main-branch || rm -rf /tmp/main-branch
78+
79+
- name: Generate coverage summary
80+
id: coverage-summary
81+
run: |
82+
echo "## 📊 Test Coverage Report" > coverage-summary.md
83+
echo "" >> coverage-summary.md
84+
85+
# Current coverage
86+
CURRENT_COV="${{ steps.coverage.outputs.coverage }}"
87+
echo "**Current Coverage:** \`$CURRENT_COV\`" >> coverage-summary.md
88+
89+
# Compare with main if this is a PR
90+
if [ "${{ github.event_name }}" = "pull_request" ] && [ "${{ steps.main-coverage.outputs.main-coverage }}" != "" ]; then
91+
MAIN_COV="${{ steps.main-coverage.outputs.main-coverage }}"
92+
echo "**Main Branch Coverage:** \`$MAIN_COV\`" >> coverage-summary.md
93+
echo "" >> coverage-summary.md
94+
95+
# Calculate difference
96+
CURRENT_NUM=$(echo $CURRENT_COV | sed 's/%//')
97+
MAIN_NUM=$(echo $MAIN_COV | sed 's/%//')
98+
99+
if [ "$CURRENT_NUM" != "" ] && [ "$MAIN_NUM" != "" ]; then
100+
DIFF=$(echo "$CURRENT_NUM - $MAIN_NUM" | bc -l 2>/dev/null || echo "0")
101+
102+
if [ "$(echo "$DIFF > 0" | bc -l 2>/dev/null)" = "1" ]; then
103+
echo "**Coverage Change:** 📈 +${DIFF}% (improved)" >> coverage-summary.md
104+
elif [ "$(echo "$DIFF < 0" | bc -l 2>/dev/null)" = "1" ]; then
105+
DIFF_ABS=$(echo "$DIFF * -1" | bc -l 2>/dev/null || echo "${DIFF#-}")
106+
echo "**Coverage Change:** 📉 -${DIFF_ABS}% (decreased)" >> coverage-summary.md
107+
else
108+
echo "**Coverage Change:** ✅ No change" >> coverage-summary.md
109+
fi
110+
fi
111+
fi
112+
113+
echo "" >> coverage-summary.md
114+
echo "### Coverage by Package" >> coverage-summary.md
115+
echo "\`\`\`" >> coverage-summary.md
116+
go tool cover -func=coverage.out >> coverage-summary.md
117+
echo "\`\`\`" >> coverage-summary.md
118+
echo "" >> coverage-summary.md
119+
echo "- 🧪 All tests passed" >> coverage-summary.md
120+
echo "- 📈 Full coverage report available in workflow artifacts" >> coverage-summary.md
121+
echo "" >> coverage-summary.md
122+
echo "_Generated by GitHub Actions_" >> coverage-summary.md
123+
124+
- name: Comment PR with coverage
125+
if: github.event_name == 'pull_request'
126+
uses: actions/github-script@v7
127+
with:
128+
script: |
129+
const fs = require('fs');
130+
const coverageSummary = fs.readFileSync('coverage-summary.md', 'utf8');
131+
132+
// Look for existing coverage comment
133+
const comments = await github.rest.issues.listComments({
134+
issue_number: context.issue.number,
135+
owner: context.repo.owner,
136+
repo: context.repo.repo,
137+
});
138+
139+
const botComment = comments.data.find(comment =>
140+
comment.user.type === 'Bot' &&
141+
comment.body.includes('📊 Test Coverage Report')
142+
);
143+
144+
if (botComment) {
145+
// Update existing comment
146+
await github.rest.issues.updateComment({
147+
comment_id: botComment.id,
148+
owner: context.repo.owner,
149+
repo: context.repo.repo,
150+
body: coverageSummary
151+
});
152+
} else {
153+
// Create new comment
154+
await github.rest.issues.createComment({
155+
issue_number: context.issue.number,
156+
owner: context.repo.owner,
157+
repo: context.repo.repo,
158+
body: coverageSummary
159+
});
160+
}
161+
162+
- name: Upload coverage artifact
163+
uses: actions/upload-artifact@v4
164+
with:
165+
name: coverage-report
166+
path: |
167+
coverage.out
168+
coverage.html
31169
32170
- name: Build
33171
run: go build -v ./...

‎arazzo/arazzo.go

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@ import (
99
"fmt"
1010
"io"
1111
"slices"
12-
"strconv"
13-
"strings"
1412

1513
"github.com/speakeasy-api/openapi/arazzo/core"
1614
"github.com/speakeasy-api/openapi/extensions"
1715
"github.com/speakeasy-api/openapi/internal/interfaces"
16+
"github.com/speakeasy-api/openapi/internal/utils"
1817
"github.com/speakeasy-api/openapi/marshaller"
1918
"github.com/speakeasy-api/openapi/validation"
2019
"github.com/speakeasy-api/openapi/yml"
@@ -70,8 +69,6 @@ func Unmarshal(ctx context.Context, doc io.Reader, opts ...Option[unmarshalOptio
7069
opt(&o)
7170
}
7271

73-
ctx = validation.ContextWithValidationContext(ctx)
74-
7572
c, err := core.Unmarshal(ctx, doc)
7673
if err != nil {
7774
return nil, nil, err
@@ -84,7 +81,6 @@ func Unmarshal(ctx context.Context, doc io.Reader, opts ...Option[unmarshalOptio
8481

8582
var validationErrs []error
8683
if !o.skipValidation {
87-
validationErrs = validation.GetValidationErrors(ctx)
8884
validationErrs = append(validationErrs, arazzo.Validate(ctx)...)
8985
slices.SortFunc(validationErrs, func(a, b error) int {
9086
var aValidationErr *validation.Error
@@ -145,15 +141,16 @@ func (a *Arazzo) Marshal(ctx context.Context, w io.Writer) error {
145141
func (a *Arazzo) Validate(ctx context.Context, opts ...validation.Option) []error {
146142
opts = append(opts, validation.WithContextObject(a))
147143

148-
errs := []error{}
144+
core := a.GetCore()
145+
errs := core.GetValidationErrors()
149146

150-
arazzoMajor, arazzoMinor, arazzoPatch, err := parseVersion(a.Arazzo)
147+
arazzoMajor, arazzoMinor, arazzoPatch, err := utils.ParseVersion(a.Arazzo)
151148
if err != nil {
152-
errs = append(errs, validation.NewValueError(fmt.Sprintf("invalid Arazzo version in document %s: %s", a.Arazzo, err.Error()), a.GetCore(), a.GetCore().Arazzo))
149+
errs = append(errs, validation.NewValueError(fmt.Sprintf("invalid Arazzo version in document %s: %s", a.Arazzo, err.Error()), core, core.Arazzo))
153150
}
154151

155152
if arazzoMajor != VersionMajor || arazzoMinor != VersionMinor || arazzoPatch > VersionPatch {
156-
errs = append(errs, validation.NewValueError(fmt.Sprintf("only Arazzo version %s and below is supported", Version), a.GetCore(), a.GetCore().Arazzo))
153+
errs = append(errs, validation.NewValueError(fmt.Sprintf("only Arazzo version %s and below is supported", Version), core, core.Arazzo))
157154
}
158155

159156
errs = append(errs, a.Info.Validate(ctx, opts...)...)
@@ -164,7 +161,7 @@ func (a *Arazzo) Validate(ctx context.Context, opts ...validation.Option) []erro
164161
errs = append(errs, sourceDescription.Validate(ctx, opts...)...)
165162

166163
if _, ok := sourceDescriptionNames[sourceDescription.Name]; ok {
167-
errs = append(errs, validation.NewSliceError(fmt.Sprintf("sourceDescription name %s is not unique", sourceDescription.Name), a.GetCore(), a.GetCore().SourceDescriptions, i))
164+
errs = append(errs, validation.NewSliceError(fmt.Sprintf("sourceDescription name %s is not unique", sourceDescription.Name), core, core.SourceDescriptions, i))
168165
}
169166

170167
sourceDescriptionNames[sourceDescription.Name] = true
@@ -176,7 +173,7 @@ func (a *Arazzo) Validate(ctx context.Context, opts ...validation.Option) []erro
176173
errs = append(errs, workflow.Validate(ctx, opts...)...)
177174

178175
if _, ok := workflowIds[workflow.WorkflowID]; ok {
179-
errs = append(errs, validation.NewSliceError(fmt.Sprintf("workflowId %s is not unique", workflow.WorkflowID), a.GetCore(), a.GetCore().Workflows, i))
176+
errs = append(errs, validation.NewSliceError(fmt.Sprintf("workflowId %s is not unique", workflow.WorkflowID), core, core.Workflows, i))
180177
}
181178

182179
workflowIds[workflow.WorkflowID] = true
@@ -186,31 +183,7 @@ func (a *Arazzo) Validate(ctx context.Context, opts ...validation.Option) []erro
186183
errs = append(errs, a.Components.Validate(ctx, opts...)...)
187184
}
188185

189-
a.Valid = len(errs) == 0 && a.GetCore().GetValid()
186+
a.Valid = len(errs) == 0 && core.GetValid()
190187

191188
return errs
192189
}
193-
194-
func parseVersion(version string) (int, int, int, error) {
195-
parts := strings.Split(version, ".")
196-
if len(parts) != 3 {
197-
return 0, 0, 0, fmt.Errorf("invalid version %s", version)
198-
}
199-
200-
major, err := strconv.Atoi(parts[0])
201-
if err != nil {
202-
return 0, 0, 0, fmt.Errorf("invalid major version %s: %w", parts[0], err)
203-
}
204-
205-
minor, err := strconv.Atoi(parts[1])
206-
if err != nil {
207-
return 0, 0, 0, fmt.Errorf("invalid minor version %s: %w", parts[1], err)
208-
}
209-
210-
patch, err := strconv.Atoi(parts[2])
211-
if err != nil {
212-
return 0, 0, 0, fmt.Errorf("invalid patch version %s: %w", parts[2], err)
213-
}
214-
215-
return major, minor, patch, nil
216-
}

‎arazzo/components.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ type componentKey struct {
4040

4141
// Validate validates the Components object.
4242
func (c *Components) Validate(ctx context.Context, opts ...validation.Option) []error {
43-
errs := []error{}
4443
core := c.GetCore()
44+
errs := core.GetValidationErrors()
4545

4646
for key, input := range c.Inputs.All() {
4747
if !componentNameRegex.MatchString(key) {

‎arazzo/criterion/criterion.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ type CriterionExpressionType struct {
5151

5252
// Validate will validate the criterion expression type object against the Arazzo specification.
5353
func (c *CriterionExpressionType) Validate(opts ...validation.Option) []error {
54-
errs := []error{}
5554
core := c.GetCore()
55+
errs := core.GetValidationErrors()
5656

5757
switch c.Type {
5858
case CriterionTypeJsonPath:
@@ -186,8 +186,8 @@ func (c *Criterion) GetCondition() (*Condition, error) {
186186

187187
// Validate will validate the criterion object against the Arazzo specification.
188188
func (c *Criterion) Validate(opts ...validation.Option) []error {
189-
errs := []error{}
190189
core := c.GetCore()
190+
errs := core.GetValidationErrors()
191191

192192
if c.Condition == "" {
193193
errs = append(errs, validation.NewValueError("condition is required", core, core.Condition))

‎arazzo/failureaction.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ func (f *FailureAction) Validate(ctx context.Context, opts ...validation.Option)
6666
}
6767
}
6868

69-
errs := []error{}
7069
core := f.GetCore()
70+
errs := core.GetValidationErrors()
7171

7272
if core.Name.Present && f.Name == "" {
7373
errs = append(errs, validation.NewValueError("name is required", core, core.Name))

‎arazzo/info.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ var _ interfaces.Model[core.Info] = (*Info)(nil)
3030

3131
// Validate will validate the Info object against the Arazzo Specification.
3232
func (i *Info) Validate(ctx context.Context, opts ...validation.Option) []error {
33-
errs := []error{}
3433
core := i.GetCore()
34+
errs := core.GetValidationErrors()
3535

3636
if core.Title.Present && i.Title == "" {
3737
errs = append(errs, validation.NewValueError("title is required", core, core.Title))

‎arazzo/parameter.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ var _ interfaces.Model[core.Parameter] = (*Parameter)(nil)
4646
// If an Workflow or Step object is provided via validation options with validation.WithContextObject() then
4747
// it will be validated in the context of that object.
4848
func (p *Parameter) Validate(ctx context.Context, opts ...validation.Option) []error {
49-
errs := []error{}
49+
core := p.GetCore()
50+
errs := core.GetValidationErrors()
5051

5152
o := validation.NewOptions(opts...)
5253

5354
w := validation.GetContextObject[Workflow](o)
5455
s := validation.GetContextObject[Step](o)
55-
core := p.GetCore()
5656

5757
if core.Name.Present && p.Name == "" {
5858
errs = append(errs, validation.NewValueError("name is required", core, core.Name))

‎arazzo/payloadreplacement.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ var _ interfaces.Model[core.PayloadReplacement] = (*PayloadReplacement)(nil)
2727

2828
// Validate will validate the payload replacement object against the Arazzo specification.
2929
func (p *PayloadReplacement) Validate(ctx context.Context, opts ...validation.Option) []error {
30-
errs := []error{}
3130
core := p.GetCore()
31+
errs := core.GetValidationErrors()
3232

3333
if core.Target.Present && p.Target == "" {
3434
errs = append(errs, validation.NewValueError("target is required", core, core.Target))

‎arazzo/requestbody.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ var _ interfaces.Model[core.RequestBody] = (*RequestBody)(nil)
3232

3333
// Validate will validate the request body object against the Arazzo specification.
3434
func (r *RequestBody) Validate(ctx context.Context, opts ...validation.Option) []error {
35-
errs := []error{}
3635
core := r.GetCore()
36+
errs := core.GetValidationErrors()
3737

3838
if r.ContentType != nil {
3939
_, _, err := mime.ParseMediaType(*r.ContentType)

‎arazzo/reusable.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ func (r *Reusable[T, V, C]) Validate(ctx context.Context, opts ...validation.Opt
108108
}
109109
}
110110

111-
errs := []error{}
112111
core := r.GetCore()
112+
errs := core.GetValidationErrors()
113113

114114
switch reflect.TypeOf((*T)(nil)).Elem().Name() {
115115
case "Parameter":

0 commit comments

Comments
 (0)