From 246f9efac81e8593a9fcaca38eda4ddf4d91c046 Mon Sep 17 00:00:00 2001 From: Andrea Angiolillo Date: Fri, 15 Mar 2024 16:13:49 +0000 Subject: [PATCH] test: added additional tests (#12) --- tools/cli/Makefile | 2 +- tools/cli/go.mod | 1 - tools/cli/internal/cli/merge/merge_test.go | 64 +++++ tools/cli/internal/openapi/oasdiff.go | 8 + tools/cli/internal/openapi/oasdiff_test.go | 288 ++++++++++++++++++--- 5 files changed, 328 insertions(+), 35 deletions(-) diff --git a/tools/cli/Makefile b/tools/cli/Makefile index d7fb5ad..448e385 100644 --- a/tools/cli/Makefile +++ b/tools/cli/Makefile @@ -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 diff --git a/tools/cli/go.mod b/tools/cli/go.mod index eaa5391..42f46ba 100644 --- a/tools/cli/go.mod +++ b/tools/cli/go.mod @@ -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 ( diff --git a/tools/cli/internal/cli/merge/merge_test.go b/tools/cli/internal/cli/merge/merge_test.go index cad106f..4990089 100644 --- a/tools/cli/internal/cli/merge/merge_test.go +++ b/tools/cli/internal/cli/merge/merge_test.go @@ -15,6 +15,7 @@ package merge import ( + "fmt" "testing" "github.com/getkin/kin-openapi/openapi3" @@ -22,6 +23,7 @@ import ( "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" ) @@ -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) { + 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, @@ -64,3 +91,40 @@ func TestCreateBuilder(t *testing.T) { []string{flag.Base, flag.External, flag.Output}, ) } + +func TestOpts_PreRunE(t *testing.T) { + testCases := []struct { + wantErr require.ErrorAssertionFunc + basePath string + name string + externalPaths []string + }{ + { + wantErr: require.Error, + basePath: "test", + name: "NoExternalPaths", + externalPaths: nil, + }, + { + wantErr: require.Error, + name: "NoBasePath", + externalPaths: []string{"test"}, + }, + { + wantErr: require.NoError, + basePath: "../../../test/data/base.json", + name: "Successful", + externalPaths: []string{"test"}, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + o := &Opts{ + basePath: tt.basePath, + externalPaths: tt.externalPaths, + } + tt.wantErr(t, o.PreRunE(nil)) + }) + } +} diff --git a/tools/cli/internal/openapi/oasdiff.go b/tools/cli/internal/openapi/oasdiff.go index 4ee8a63..04c08e5 100644 --- a/tools/cli/internal/openapi/oasdiff.go +++ b/tools/cli/internal/openapi/oasdiff.go @@ -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 { diff --git a/tools/cli/internal/openapi/oasdiff_test.go b/tools/cli/internal/openapi/oasdiff_test.go index 4392788..fc69356 100644 --- a/tools/cli/internal/openapi/oasdiff_test.go +++ b/tools/cli/internal/openapi/oasdiff_test.go @@ -17,18 +17,20 @@ package openapi import ( "testing" + "github.com/stretchr/testify/require" + "github.com/getkin/kin-openapi/openapi3" "github.com/mongodb/openapi/tools/cli/internal/pointer" "github.com/tufin/oasdiff/diff" "github.com/tufin/oasdiff/load" ) -func Test_MergePaths(t *testing.T) { +func TestOasDiff_mergePaths(t *testing.T) { testCases := []struct { inputBase *load.SpecInfo inputExternal *load.SpecInfo + wantErr require.ErrorAssertionFunc name string - error bool }{ { name: "SuccessfulMerge", @@ -46,7 +48,7 @@ func Test_MergePaths(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithEmptyPaths", @@ -64,7 +66,7 @@ func Test_MergePaths(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithEmptyBasePaths", @@ -82,7 +84,7 @@ func Test_MergePaths(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithEmptyExternalPaths", @@ -100,7 +102,7 @@ func Test_MergePaths(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "FailedMerge", @@ -118,30 +120,29 @@ func Test_MergePaths(t *testing.T) { }, Version: "3.0.1", }, - error: true, + wantErr: require.Error, }, } for _, tc := range testCases { + tc := tc // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#what-happened t.Run(tc.name, func(t *testing.T) { + t.Parallel() o := OasDiff{ base: tc.inputBase, external: tc.inputExternal, } - err := o.mergePaths() - if err != nil && !tc.error { - t.Errorf("No error expected but got the error %v", err) - } + tc.wantErr(t, o.mergePaths()) }) } } -func Test_MergeTags(t *testing.T) { +func TestOasDiff_mergeTags(t *testing.T) { testCases := []struct { inputBase *load.SpecInfo inputExternal *load.SpecInfo + wantErr require.ErrorAssertionFunc name string - error bool }{ { name: "SuccessfulMerge", @@ -179,7 +180,7 @@ func Test_MergeTags(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeEmptyTags", @@ -197,7 +198,7 @@ func Test_MergeTags(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeEmptyBaseTags", @@ -235,7 +236,7 @@ func Test_MergeTags(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeEmptyExternalTags", @@ -253,7 +254,7 @@ func Test_MergeTags(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "FailedMerge", @@ -291,31 +292,30 @@ func Test_MergeTags(t *testing.T) { }, Version: "3.0.1", }, - error: true, + wantErr: require.Error, }, } for _, tc := range testCases { + tc := tc // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#what-happened t.Run(tc.name, func(t *testing.T) { + t.Parallel() o := OasDiff{ base: tc.inputBase, external: tc.inputExternal, } - err := o.mergeTags() - if err != nil && !tc.error { - t.Errorf("No error expected but got the error %v", err) - } + tc.wantErr(t, o.mergeTags()) }) } } -func Test_MergeResponses(t *testing.T) { +func TestOasDiff_mergeResponses(t *testing.T) { testCases := []struct { inputBase *load.SpecInfo inputExternal *load.SpecInfo + wantErr require.ErrorAssertionFunc diff *diff.Diff name string - error bool }{ { name: "SuccessfulMergeWithEmptyResponses", @@ -338,7 +338,7 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithEmpyBaseResponses", @@ -367,7 +367,7 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithEmpyExternalResponses", @@ -396,7 +396,7 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeIdenticalResponses", @@ -447,7 +447,7 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMerge", @@ -486,7 +486,7 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: false, + wantErr: require.NoError, }, { name: "SuccessfulMergeWithNoIdenticalResponses", @@ -540,21 +540,243 @@ func Test_MergeResponses(t *testing.T) { }, Version: "3.0.1", }, - error: true, + wantErr: require.Error, }, } for _, tc := range testCases { + tc := tc // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#what-happened t.Run(tc.name, func(t *testing.T) { + t.Parallel() o := OasDiff{ base: tc.inputBase, external: tc.inputExternal, specDiff: tc.diff, } - err := o.mergeResponses() - if err != nil && !tc.error { - t.Errorf("No error expected but got the error %v", err) + tc.wantErr(t, o.mergeResponses()) + }) + } +} + +func TestOasDiff_mergeSchemas(t *testing.T) { + testCases := []struct { + inputBase *load.SpecInfo + inputExternal *load.SpecInfo + wantErr require.ErrorAssertionFunc + diff *diff.Diff + name string + }{ + { + 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", + }, + wantErr: require.NoError, + }, + { + 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", + }, + wantErr: require.NoError, + }, + { + 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", + }, + wantErr: require.NoError, + }, + { + 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", + }, + wantErr: require.NoError, + }, + { + 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", + }, + wantErr: require.NoError, + }, + { + 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", + }, + wantErr: require.Error, + }, + } + + for _, tc := range testCases { + tc := tc // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#what-happened + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + o := OasDiff{ + base: tc.inputBase, + external: tc.inputExternal, + specDiff: tc.diff, } + tc.wantErr(t, o.mergeSchemas()) }) } }