Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: added additional tests #12

Merged
merged 6 commits into from Mar 15, 2024
Merged

test: added additional tests #12

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
17e3eaa
test: added additional tests
andreaangiolillo Mar 15, 2024
c101a73
Update go.mod
andreaangiolillo Mar 15, 2024
db0c97a
Merge remote-tracking branch 'origin/main' into add_unit_test_2
andreaangiolillo Mar 15, 2024
59f9849
Update merge_test.go
andreaangiolillo Mar 15, 2024
245fb65
addressed PR comments
andreaangiolillo Mar 15, 2024
a063b74
Update oasdiff_test.go
andreaangiolillo Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion tools/cli/Makefile
View file
Open in desktop
Expand Up @@ -63,7 +63,7 @@ list: ## List all make targets
.PHONY: unit-test
unit-test: ## Run unit-tests
@echo "==> Running unit tests..."
$(TEST_CMD) -race ./...
$(TEST_CMD) -race -cover ./...

.PHONY: gen-mocks
gen-mocks: ## Generate mocks
Expand Down
1 change: 0 additions & 1 deletion tools/cli/go.mod
View file
Open in desktop
Expand Up @@ -9,7 +9,6 @@ require (
github.com/stretchr/testify v1.9.0
github.com/tufin/oasdiff v1.10.11
go.uber.org/mock v0.4.0
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a
)

require (
Expand Down
27 changes: 27 additions & 0 deletions tools/cli/internal/cli/merge/merge_test.go
View file
Open in desktop
Expand Up @@ -15,13 +15,15 @@
package merge

import (
"fmt"
"testing"

"github.com/getkin/kin-openapi/openapi3"
"github.com/mongodb/openapi/tools/cli/internal/cli/flag"
"github.com/mongodb/openapi/tools/cli/internal/cli/validator"
"github.com/mongodb/openapi/tools/cli/internal/openapi"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
"github.com/tufin/oasdiff/load"
"go.uber.org/mock/gomock"
)
Expand Down Expand Up @@ -56,6 +58,31 @@ func TestSuccessfulMerge_Run(t *testing.T) {
}
}

func TestNoBaseSpecMerge_PreRun(t *testing.T) {
externalPaths := []string{"external.json"}
opts := &Opts{
outputPath: "foas.json",
externalPaths: externalPaths,
}

err := opts.PreRunE(nil)
require.Error(t, err)
require.EqualError(t, err, fmt.Sprintf("no base OAS detected. "+
"Please, use the flag %s to include the base OAS", flag.Base))
}

func TestNoExternalSpecMerge_PreRun(t *testing.T) {
andreaangiolillo marked this conversation as resolved.
Show resolved Hide resolved
opts := &Opts{
outputPath: "foas.json",
basePath: "base.json",
}

err := opts.PreRunE(nil)
require.Error(t, err)
require.EqualError(t, err, fmt.Sprintf("no external OAS detected. "+
"Please, use the flag %s to include at least one OAS", flag.External))
}

func TestCreateBuilder(t *testing.T) {
validator.ValidateSubCommandsAndFlags(
t,
Expand Down
8 changes: 8 additions & 0 deletions tools/cli/internal/openapi/oasdiff.go
View file
Open in desktop
Expand Up @@ -200,7 +200,15 @@ func (o OasDiff) mergeResponses() error {

func (o OasDiff) mergeSchemas() error {
extSchemas := o.external.Spec.Components.Schemas
if len(extSchemas) == 0 {
return nil
}

baseSchemas := o.base.Spec.Components.Schemas
if len(baseSchemas) == 0 {
o.base.Spec.Components.Schemas = extSchemas
return nil
}

for k, schemaToMerge := range extSchemas {
if _, ok := baseSchemas[k]; !ok {
Expand Down
224 changes: 224 additions & 0 deletions tools/cli/internal/openapi/oasdiff_test.go
View file
Open in desktop
Expand Up @@ -559,6 +559,230 @@ func Test_MergeResponses(t *testing.T) {
}
}

func Test_MergeSchemas(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func Test_MergeSchemas(t *testing.T) {
func TestOasDiff_mergeSchemas(t *testing.T) {

testCases := []struct {
inputBase *load.SpecInfo
inputExternal *load.SpecInfo
diff *diff.Diff
name string
error bool
andreaangiolillo marked this conversation as resolved.
Show resolved Hide resolved
}{
{
name: "SuccessfulMergeWithEmptySchemas",
diff: &diff.Diff{},
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: nil,
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: nil,
},
},
Version: "3.0.1",
},
error: false,
andreaangiolillo marked this conversation as resolved.
Show resolved Hide resolved
},
{
name: "SuccessfulMergeWithEmpyBaseSchema",
diff: &diff.Diff{},
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{},
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"ext1": {
Value: &openapi3.Schema{Description: "ext1"},
},
},
},
},
Version: "3.0.1",
},
error: false,
},
{
name: "SuccessfulMergeWithEmpyExternalSchema",
diff: &diff.Diff{},
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"base1": {
Value: &openapi3.Schema{Description: "base1"},
},
},
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{},
},
},
Version: "3.0.1",
},
error: false,
},
{
name: "SuccessfulMergeIdenticalSchemas",
diff: &diff.Diff{
ComponentsDiff: diff.ComponentsDiff{
SchemasDiff: &diff.SchemasDiff{
Modified: map[string]*diff.SchemaDiff{},
},
},
},
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"base1": {
Value: &openapi3.Schema{Description: "base1"},
},
},
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"base1": {
Value: &openapi3.Schema{Description: "base1"},
},
},
},
},
Version: "3.0.1",
},
error: false,
},
{
name: "SuccessfulMerge",
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Responses: map[string]*openapi3.ResponseRef{
"base": {
Value: &openapi3.Response{
Description: pointer.Get("base"),
},
},
},
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Responses: map[string]*openapi3.ResponseRef{
"external1": {
Value: &openapi3.Response{
Description: pointer.Get("external1"),
},
},
"external2": {
Value: &openapi3.Response{
Description: pointer.Get("external2"),
},
},
},
},
},
Version: "3.0.1",
},
error: false,
},
{
name: "SuccessfulMergeWithNoIdenticalResponses",
diff: &diff.Diff{
ComponentsDiff: diff.ComponentsDiff{
SchemasDiff: &diff.SchemasDiff{
Modified: map[string]*diff.SchemaDiff{
"base1": {Base: nil},
"base2": {Base: nil},
},
},
},
},
inputBase: &load.SpecInfo{
Url: "base",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"base1": {
Value: &openapi3.Schema{Description: "base1"},
},
"base2": {
Value: &openapi3.Schema{Description: "base1"},
},
},
},
},
Version: "3.0.1",
},
inputExternal: &load.SpecInfo{
Url: "external",
Spec: &openapi3.T{
Components: &openapi3.Components{
Schemas: openapi3.Schemas{
"base1": {
Value: &openapi3.Schema{Description: "base1"},
},
"base2": {
Value: &openapi3.Schema{Description: "base1"},
},
},
},
},
Version: "3.0.1",
},
error: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
o := OasDiff{
andreaangiolillo marked this conversation as resolved.
Show resolved Hide resolved
base: tc.inputBase,
external: tc.inputExternal,
specDiff: tc.diff,
}
err := o.mergeSchemas()
if err != nil && !tc.error {
t.Errorf("No error expected but got the error %v", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
err := o.mergeSchemas()
if err != nil && !tc.error {
t.Errorf("No error expected but got the error %v", err)
}
tc.wantErr(o.mergeSchemas())

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the above may produce lint warnings/unexpected test results do to parallel and need some clousure with tc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

})
}
}

func newBaseSpecPaths(t *testing.T) *openapi3.Paths {
t.Helper()
inputPath := &openapi3.Paths{}
Expand Down