Skip to content

Commit 0a5d7df

Browse files
blvablva
committed
Add tests
1 parent 5ded458 commit 0a5d7df

File tree

5 files changed

+292
-20
lines changed

5 files changed

+292
-20
lines changed

‎tools/cli/internal/openapi/filter/mock_filter.go‎

Lines changed: 4 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tools/cli/internal/openapi/mock_paths.go‎

Lines changed: 56 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tools/cli/internal/openapi/oasdiff.go‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ import (
2828
)
2929

3030
type OasDiff struct {
31-
base *load.SpecInfo
32-
external *load.SpecInfo
33-
config *diff.Config
34-
specDiff *diff.Diff
35-
parser Parser
31+
base *load.SpecInfo
32+
external *load.SpecInfo
33+
config *diff.Config
34+
specDiff *diff.Diff
35+
noExtDiff NoExtensionDiff
36+
parser Parser
3637
}
3738

3839
type OasDiffResult struct {
@@ -177,7 +178,7 @@ func (o OasDiff) handlePathConflict(basePath *openapi3.PathItem, basePathName st
177178
return err
178179
}
179180

180-
log.Printf("Skipping conflict for path: %s", basePathName)
181+
log.Printf("Skipping conflict for path: %s, pathsAreIdentical: %v", basePathName, pathsAreIdentical)
181182
if pathsAreIdentical {
182183
return nil
183184
}
@@ -189,7 +190,7 @@ func (o OasDiff) handlePathConflict(basePath *openapi3.PathItem, basePathName st
189190
}
190191
}
191192

192-
d, err := o.getDiffWithoutExtensions()
193+
d, err := o.noExtDiff.GetPathDiffWithoutExtensions(o.base.Spec, o.external.Spec)
193194
if err != nil {
194195
return err
195196
}
@@ -486,22 +487,22 @@ func (o OasDiff) areSchemaIdentical(name string) bool {
486487
// arePathsIdenticalWithExcludeExtensions checks if the paths are identical with the extensions excluded
487488
func (o OasDiff) arePathsIdenticalWithExcludeExtensions(name string) (bool, error) {
488489
// If the diff only has extensions diff, then we consider the paths to be identical
489-
d, err := o.getDiffWithoutExtensions()
490+
d, err := o.noExtDiff.GetPathDiffWithoutExtensions(o.base.Spec, o.external.Spec)
490491
if err != nil {
491492
return false, err
492493
}
493494

494495
if d.Empty() || d.PathsDiff.Empty() {
495496
return true, nil
496497
}
497-
_, ok := d.PathsDiff.Modified[name]
498-
return !ok, nil
499-
}
498+
v, ok := d.PathsDiff.Modified[name]
499+
if ok {
500+
if v.Empty() {
501+
return true, nil
502+
}
503+
}
500504

501-
func (o OasDiff) getDiffWithoutExtensions() (*diff.Diff, error) {
502-
exclude := []string{"extensions"}
503-
customConfig := diff.NewConfig().WithExcludeElements(exclude)
504-
return diff.Get(customConfig, o.base.Spec, o.external.Spec)
505+
return !ok, nil
505506
}
506507

507508
type ByName []*openapi3.Tag

‎tools/cli/internal/openapi/oasdiff_test.go‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@ import (
1919
"testing"
2020

2121
"github.com/getkin/kin-openapi/openapi3"
22+
"github.com/mongodb/openapi/tools/cli/internal/openapi/errors"
2223
"github.com/mongodb/openapi/tools/cli/internal/pointer"
2324
"github.com/stretchr/testify/assert"
2425
"github.com/stretchr/testify/require"
2526
"github.com/tufin/oasdiff/diff"
2627
"github.com/tufin/oasdiff/load"
28+
"github.com/tufin/oasdiff/utils"
29+
gomock "go.uber.org/mock/gomock"
2730
)
2831

2932
func TestOasDiff_mergePaths(t *testing.T) {
@@ -1142,3 +1145,198 @@ func TestUpdateExternalRefReqBody(t *testing.T) {
11421145
})
11431146
}
11441147
}
1148+
1149+
func TestHandlePathConflict(t *testing.T) {
1150+
testCases := []struct {
1151+
name string
1152+
basePath *openapi3.PathItem
1153+
basePathName string
1154+
specDiff *diff.Diff
1155+
expectedError error
1156+
}{
1157+
{
1158+
name: "No Conflict - Identical Paths",
1159+
basePath: &openapi3.PathItem{
1160+
Get: &openapi3.Operation{
1161+
Extensions: map[string]interface{}{
1162+
"x-xgen-soa-migration": map[string]interface{}{
1163+
"allowDocsDiff": "false",
1164+
},
1165+
},
1166+
},
1167+
},
1168+
basePathName: "/test",
1169+
specDiff: &diff.Diff{
1170+
PathsDiff: &diff.PathsDiff{
1171+
Modified: map[string]*diff.PathDiff{},
1172+
},
1173+
},
1174+
expectedError: nil,
1175+
},
1176+
{
1177+
name: "Conflict with AllowDocsDiff",
1178+
basePath: &openapi3.PathItem{
1179+
Get: &openapi3.Operation{
1180+
Extensions: map[string]interface{}{
1181+
"x-xgen-soa-migration": map[string]interface{}{
1182+
"allowDocsDiff": "true",
1183+
},
1184+
},
1185+
},
1186+
},
1187+
basePathName: "/test",
1188+
specDiff: &diff.Diff{
1189+
PathsDiff: &diff.PathsDiff{
1190+
Modified: map[string]*diff.PathDiff{
1191+
"/test": {
1192+
OperationsDiff: &diff.OperationsDiff{
1193+
Modified: diff.ModifiedOperations{
1194+
"get": {
1195+
TagsDiff: &diff.StringsDiff{
1196+
Added: utils.StringList{"tag1"},
1197+
},
1198+
},
1199+
},
1200+
},
1201+
},
1202+
},
1203+
},
1204+
},
1205+
expectedError: errors.AllowDocsDiffNotSupportedError{
1206+
Entry: "/test",
1207+
},
1208+
},
1209+
{
1210+
name: "Conflict with Different Operations",
1211+
basePath: &openapi3.PathItem{
1212+
Get: &openapi3.Operation{
1213+
Extensions: map[string]interface{}{
1214+
"x-xgen-soa-migration": map[string]interface{}{
1215+
"allowDocsDiff": "false",
1216+
},
1217+
},
1218+
},
1219+
},
1220+
basePathName: "/test",
1221+
specDiff: &diff.Diff{
1222+
PathsDiff: &diff.PathsDiff{
1223+
Modified: map[string]*diff.PathDiff{
1224+
"/test": {
1225+
OperationsDiff: &diff.OperationsDiff{
1226+
Added: utils.StringList{"get"},
1227+
Deleted: utils.StringList{},
1228+
},
1229+
},
1230+
},
1231+
},
1232+
},
1233+
expectedError: errors.PathConflictError{
1234+
Entry: "/test",
1235+
},
1236+
},
1237+
{
1238+
name: "Conflict with Different Path Operation",
1239+
basePath: &openapi3.PathItem{
1240+
Get: &openapi3.Operation{
1241+
Extensions: map[string]interface{}{
1242+
"x-xgen-soa-migration": map[string]interface{}{
1243+
"allowDocsDiff": "false",
1244+
},
1245+
},
1246+
},
1247+
},
1248+
basePathName: "/test",
1249+
specDiff: &diff.Diff{
1250+
PathsDiff: &diff.PathsDiff{
1251+
Modified: map[string]*diff.PathDiff{
1252+
"/test": {
1253+
OperationsDiff: &diff.OperationsDiff{
1254+
Modified: diff.ModifiedOperations{
1255+
"get": {
1256+
TagsDiff: &diff.StringsDiff{
1257+
Added: utils.StringList{"tag1"},
1258+
},
1259+
},
1260+
},
1261+
},
1262+
},
1263+
},
1264+
},
1265+
},
1266+
expectedError: errors.PathDocsDiffConflictError{
1267+
Entry: "/test",
1268+
Diff: &diff.Diff{
1269+
PathsDiff: &diff.PathsDiff{
1270+
Modified: map[string]*diff.PathDiff{
1271+
"/test": {
1272+
OperationsDiff: &diff.OperationsDiff{
1273+
Modified: diff.ModifiedOperations{
1274+
"get": {
1275+
TagsDiff: &diff.StringsDiff{
1276+
Added: utils.StringList{"tag1"},
1277+
},
1278+
},
1279+
},
1280+
},
1281+
},
1282+
},
1283+
},
1284+
},
1285+
},
1286+
},
1287+
{
1288+
name: "Identical Paths with Extensions",
1289+
basePath: &openapi3.PathItem{
1290+
Get: &openapi3.Operation{
1291+
Extensions: map[string]interface{}{
1292+
"x-xgen-soa-migration": map[string]interface{}{
1293+
"allowDocsDiff": "false",
1294+
},
1295+
},
1296+
},
1297+
},
1298+
basePathName: "/test",
1299+
specDiff: &diff.Diff{
1300+
PathsDiff: &diff.PathsDiff{
1301+
Modified: map[string]*diff.PathDiff{},
1302+
},
1303+
},
1304+
expectedError: nil,
1305+
},
1306+
}
1307+
1308+
for _, tc := range testCases {
1309+
t.Run(tc.name, func(t *testing.T) {
1310+
ctrl := gomock.NewController(t)
1311+
mockNoExtensionDiff := NewMockNoExtensionDiff(ctrl)
1312+
o := OasDiff{
1313+
base: &load.SpecInfo{
1314+
Spec: &openapi3.T{
1315+
Paths: &openapi3.Paths{},
1316+
},
1317+
},
1318+
external: &load.SpecInfo{
1319+
Spec: &openapi3.T{
1320+
Paths: &openapi3.Paths{},
1321+
},
1322+
},
1323+
specDiff: tc.specDiff,
1324+
noExtDiff: mockNoExtensionDiff,
1325+
}
1326+
1327+
mockNoExtensionDiff.
1328+
EXPECT().
1329+
GetPathDiffWithoutExtensions(o.base.Spec, o.external.Spec).
1330+
Return(tc.specDiff, nil).
1331+
AnyTimes()
1332+
1333+
err := o.handlePathConflict(tc.basePath, tc.basePathName)
1334+
if tc.expectedError != nil {
1335+
assert.Error(t, err)
1336+
assert.IsType(t, tc.expectedError, err)
1337+
} else {
1338+
assert.NoError(t, err)
1339+
}
1340+
})
1341+
}
1342+
}

‎tools/cli/internal/openapi/paths.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
package openapi
1616

1717
import (
18+
"fmt"
1819
"log"
1920

2021
"github.com/getkin/kin-openapi/openapi3"
22+
"github.com/tufin/oasdiff/diff"
2123
)
2224

25+
//go:generate mockgen -destination=../openapi/mock_paths.go -package=openapi github.com/mongodb/openapi/tools/cli/internal/openapi NoExtensionDiff
26+
2327
const (
2428
xgenSoaMigration = "x-xgen-soa-migration"
2529
allowDocsDiff = "allowDocsDiff"
@@ -133,3 +137,17 @@ func getOperationExtensionProperty(operation *openapi3.Operation, extensionName,
133137
}
134138
return ""
135139
}
140+
141+
type NoExtensionDiff interface {
142+
GetPathDiffWithoutExtensions(base, external *openapi3.T) (*diff.Diff, error)
143+
}
144+
145+
func GetPathDiffWithoutExtensions(base, external *openapi3.T) (*diff.Diff, error) {
146+
exclude := []string{"extensions"}
147+
customConfig := diff.NewConfig().WithExcludeElements(exclude)
148+
if base == nil || external == nil {
149+
return nil, fmt.Errorf("base or external spec is nil")
150+
}
151+
152+
return diff.Get(customConfig, base, external)
153+
}

0 commit comments

Comments
 (0)