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

(feature) Add optional checker for tag removal #208

Merged
merged 4 commits into from
Mar 26, 2023
Merged

(feature) Add optional checker for tag removal #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
44316f6
Add optional checker for tag removal
blva Mar 25, 2023
4bc359c
Add tests for tag addition
blva Mar 25, 2023
df1f047
Split errors per tag
blva Mar 26, 2023
2e19fd6
remove redundant check
Mar 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions breaking-changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
These examples are automatically generated from unit tests.
## Examples of breaking changes
[adding a new required property in request body is breaking](checker/checker_breaking_property_test.go?plain=1#L194)
[adding a pattern to a schema is breaking for recursive properties](checker/checker_breaking_test.go?plain=1#L391)
[adding a pattern to a schema is breaking](checker/checker_breaking_test.go?plain=1#L357)
[adding a pattern to a schema is breaking](checker/checker_breaking_test.go?plain=1#L373)
[adding a pattern to a schema is breaking for recursive properties](checker/checker_breaking_test.go?plain=1#L409)
[adding a pattern to a schema is breaking](checker/checker_breaking_test.go?plain=1#L375)
[adding a pattern to a schema is breaking](checker/checker_breaking_test.go?plain=1#L391)
[adding a required request body is breaking](checker/checker_breaking_test.go?plain=1#L61)
[changing a required property in response body to optional and also deleting it is breaking](checker/checker_breaking_property_test.go?plain=1#L122)
[changing an existing header param from optional to required is breaking](checker/checker_breaking_test.go?plain=1#L163)
Expand All @@ -16,7 +16,7 @@ These examples are automatically generated from unit tests.
[changing an existing response header from required to optional is breaking](checker/checker_breaking_test.go?plain=1#L187)
[changing max length in request from nil to any value is breaking](checker/checker_breaking_min_max_test.go?plain=1#L110)
[changing max length in response from any value to nil is breaking](checker/checker_breaking_min_max_test.go?plain=1#L160)
[deleting a media-type from response is breaking](checker/checker_breaking_test.go?plain=1#L327)
[deleting a media-type from response is breaking](checker/checker_breaking_test.go?plain=1#L345)
[deleting a path is breaking](checker/checker_breaking_test.go?plain=1#L39)
[deleting a path with some operations having sunset date in the future is breaking](checker/checker_deprecation_test.go?plain=1#L251)
[deleting a required property in request is breaking with warn](checker/checker_breaking_property_test.go?plain=1#L210)
Expand All @@ -30,75 +30,78 @@ These examples are automatically generated from unit tests.
[deprecating an operation with a deprecation policy but without specifying sunset date is breaking](checker/checker_deprecation_test.go?plain=1#L68)
[increasing max length in response is breaking](checker/checker_breaking_min_max_test.go?plain=1#L93)
[increasing min items in request is breaking](checker/checker_breaking_min_max_test.go?plain=1#L236)
[modifying a pattern in a schema is breaking](checker/checker_breaking_test.go?plain=1#L407)
[modifying a pattern in request parameter is breaking](checker/checker_breaking_test.go?plain=1#L423)
[modifying the default value of an optional request parameter is breaking](checker/checker_breaking_test.go?plain=1#L453)
[modifying a pattern in a schema is breaking](checker/checker_breaking_test.go?plain=1#L425)
[modifying a pattern in request parameter is breaking](checker/checker_breaking_test.go?plain=1#L441)
[modifying the default value of an optional request parameter is breaking](checker/checker_breaking_test.go?plain=1#L471)
[new required header param is breaking](checker/checker_breaking_test.go?plain=1#L147)
[new required path param is breaking](checker/checker_breaking_test.go?plain=1#L131)
[new required property in request header is breaking](checker/checker_breaking_property_test.go?plain=1#L17)
[reducing max in request is breaking](checker/checker_breaking_min_max_test.go?plain=1#L264)
[reducing max length in request is breaking](checker/checker_breaking_min_max_test.go?plain=1#L12)
[reducing min items in response is breaking](checker/checker_breaking_min_max_test.go?plain=1#L220)
[reducing min length in response is breaking](checker/checker_breaking_min_max_test.go?plain=1#L62)
[removing an existing optional response header is breaking as warn](checker/checker_breaking_test.go?plain=1#L308)
[removing an existing optional response header is breaking as warn](checker/checker_breaking_test.go?plain=1#L326)
[removing an existing required response header is breaking as error](checker/checker_breaking_test.go?plain=1#L203)
[removing an existing response with non-successful status is breaking (optional)](checker/checker_breaking_test.go?plain=1#L240)
[removing an existing response with successful status is breaking](checker/checker_breaking_test.go?plain=1#L222)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not alpha stability level](checker/checker_deprecation_test.go?plain=1#L121)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not draft stability level](checker/checker_deprecation_test.go?plain=1#L173)
[removing/updating a tag is breaking (optional)](checker/checker_breaking_test.go?plain=1#L276)
[removing/updating an operation id is breaking (optional)](checker/checker_breaking_test.go?plain=1#L258)

## Examples of non-breaking changes
[adding a media-type to response is not breaking](checker/checker_not_breaking_test.go?plain=1#L165)
[adding a media-type to response is not breaking](checker/checker_not_breaking_test.go?plain=1#L166)
[adding a new required property in response body is not breaking](checker/checker_breaking_property_test.go?plain=1#L227)
[adding a new required property under AllOf in response body is not breaking](checker/checker_breaking_property_test.go?plain=1#L257)
[adding a new required read-only property in request body is not breaking](checker/checker_breaking_property_test.go?plain=1#L311)
[adding a non-existent required property in request body is not breaking](checker/checker_breaking_property_test.go?plain=1#L136)
[adding an enum value is not breaking](checker/checker_not_breaking_test.go?plain=1#L65)
[adding an optional request body is not breaking](checker/checker_not_breaking_test.go?plain=1#L19)
[adding a tag is not breaking with "api-tag-removed" check](checker/checker_not_breaking_test.go?plain=1#L262)
[adding a tag is not breaking](checker/checker_not_breaking_test.go?plain=1#L247)
[adding an enum value is not breaking](checker/checker_not_breaking_test.go?plain=1#L66)
[adding an optional request body is not breaking](checker/checker_not_breaking_test.go?plain=1#L20)
[both max lengths in request are nil is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L178)
[both max lengths in response are nil is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L192)
[changing a link to operation ID is not breaking](checker/checker_not_breaking_test.go?plain=1#L156)
[changing an existing header param to optional is not breaking](checker/checker_not_breaking_test.go?plain=1#L115)
[changing a link to operation ID is not breaking](checker/checker_not_breaking_test.go?plain=1#L157)
[changing an existing header param to optional is not breaking](checker/checker_not_breaking_test.go?plain=1#L116)
[changing an existing property in request body to optional is not breaking](checker/checker_breaking_property_test.go?plain=1#L164)
[changing an existing property in request header to optional is not breaking](checker/checker_breaking_property_test.go?plain=1#L82)
[changing an existing property in response body to required is not breaking](checker/checker_breaking_property_test.go?plain=1#L150)
[changing an existing read-only property in request body to required is not breaking](checker/checker_breaking_property_test.go?plain=1#L325)
[changing an existing request body from required to optional is not breaking](checker/checker_not_breaking_test.go?plain=1#L34)
[changing an existing request body from required to optional is not breaking](checker/checker_not_breaking_test.go?plain=1#L35)
[changing an existing required property in response body to write-only is not breaking](checker/checker_breaking_property_test.go?plain=1#L387)
[changing an existing write-only property in response body to optional is not breaking](checker/checker_breaking_property_test.go?plain=1#L373)
[changing comments is not breaking](checker/checker_not_breaking_test.go?plain=1#L89)
[changing extensions is not breaking](checker/checker_not_breaking_test.go?plain=1#L77)
[changing comments is not breaking](checker/checker_not_breaking_test.go?plain=1#L90)
[changing extensions is not breaking](checker/checker_not_breaking_test.go?plain=1#L78)
[changing max length in request from any value to nil is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L144)
[changing max length in response from nil to any value is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L128)
[changing operation ID is not breaking](checker/checker_not_breaking_test.go?plain=1#L147)
[changing servers is not breaking](checker/checker_not_breaking_test.go?plain=1#L232)
[changing operation ID is not breaking](checker/checker_not_breaking_test.go?plain=1#L148)
[changing servers is not breaking](checker/checker_not_breaking_test.go?plain=1#L233)
[deleting a non-required non-write-only property in response body is not breaking](checker/checker_breaking_property_test.go?plain=1#L356)
[deleting a path after sunset date of all contained operations is not breaking](checker/checker_deprecation_test.go?plain=1#L236)
[deleting a pattern from a schema is not breaking](checker/checker_breaking_test.go?plain=1#L343)
[deleting a pattern from a schema is not breaking](checker/checker_breaking_test.go?plain=1#L361)
[deleting a required write-only property in response body is not breaking](checker/checker_breaking_property_test.go?plain=1#L339)
[deleting a tag is not breaking](checker/checker_not_breaking_test.go?plain=1#L53)
[deleting a tag is not breaking](checker/checker_not_breaking_test.go?plain=1#L54)
[deleting an operation after sunset date is not breaking](checker/checker_deprecation_test.go?plain=1#L53)
[deprecating a header is not breaking](checker/checker_not_breaking_test.go?plain=1#L206)
[deprecating a parameter is not breaking](checker/checker_not_breaking_test.go?plain=1#L193)
[deprecating a schema is not breaking](checker/checker_not_breaking_test.go?plain=1#L219)
[deprecating an operation is not breaking](checker/checker_not_breaking_test.go?plain=1#L179)
[deprecating a header is not breaking](checker/checker_not_breaking_test.go?plain=1#L207)
[deprecating a parameter is not breaking](checker/checker_not_breaking_test.go?plain=1#L194)
[deprecating a schema is not breaking](checker/checker_not_breaking_test.go?plain=1#L220)
[deprecating an operation is not breaking](checker/checker_not_breaking_test.go?plain=1#L180)
[deprecating an operation with a deprecation policy and sunset date after required deprecation period is not breaking](checker/checker_deprecation_test.go?plain=1#L217)
[deprecating an operation without a deprecation policy and without specifying sunset date is not breaking for draft level](checker/checker_deprecation_test.go?plain=1#L137)
[deprecating an operation without a deprecation policy and without specifying sunset date is not breaking](checker/checker_deprecation_test.go?plain=1#L87)
[increasing max length in request is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L76)
[increasing min items in response is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L250)
[modifying a pattern to ".*"" in a schema is not breaking](checker/checker_breaking_test.go?plain=1#L439)
[modifying the default value of a required request parameter is not breaking](checker/checker_breaking_test.go?plain=1#L469)
[new optional header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L101)
[modifying a pattern to ".*"" in a schema is not breaking](checker/checker_breaking_test.go?plain=1#L457)
[modifying the default value of a required request parameter is not breaking](checker/checker_breaking_test.go?plain=1#L487)
[new optional header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L102)
[new optional property in request header is not breaking](checker/checker_breaking_property_test.go?plain=1#L38)
[new required response header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L133)
[no change is not breaking](checker/checker_not_breaking_test.go?plain=1#L14)
[new required response header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L134)
[no change is not breaking](checker/checker_not_breaking_test.go?plain=1#L15)
[reducing max in response is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L281)
[reducing max length in response is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L31)
[reducing min items in request is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L206)
[reducing min length in request is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L48)
[removing an existing response with error status is not breaking](checker/checker_breaking_test.go?plain=1#L292)
[removing an existing response with unparseable status is not breaking](checker/checker_breaking_test.go?plain=1#L276)
[removing an existing response with error status is not breaking](checker/checker_breaking_test.go?plain=1#L310)
[removing an existing response with unparseable status is not breaking](checker/checker_breaking_test.go?plain=1#L294)
[removing the path without a deprecation policy and without specifying sunset date is not breaking for alpha level](checker/checker_deprecation_test.go?plain=1#L102)
[removing the path without a deprecation policy and without specifying sunset date is not breaking for draft level](checker/checker_deprecation_test.go?plain=1#L154)
6 changes: 3 additions & 3 deletions checker/check-api-operation-id-removed.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

const (
id = "api-operation-id-removed"
apiOperationRemovedCheckId = "api-operation-id-removed"
)

func APIOperationIdRemovedCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config BackwardCompatibilityCheckConfig) []BackwardCompatibilityError {
Expand All @@ -30,9 +30,9 @@ func APIOperationIdRemovedCheck(diffReport *diff.Diff, operationsSources *diff.O
}

result = append(result, BackwardCompatibilityError{
Id: id,
Id: apiOperationRemovedCheckId,
Level: ERR,
Text: fmt.Sprintf(config.i18n(id), ColorizedValue(operationItem.Base.OperationID), ColorizedValue(operationItem.Revision.OperationID)),
Text: fmt.Sprintf(config.i18n(apiOperationRemovedCheckId), ColorizedValue(operationItem.Base.OperationID), ColorizedValue(operationItem.Revision.OperationID)),
Operation: operation,
Path: path,
Source: source,
Expand Down
46 changes: 46 additions & 0 deletions checker/check-api-tag-removed.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package checker

import (
"fmt"

"github.com/tufin/oasdiff/diff"
)

const (
apiTagRemovedCheckId = "api-tag-removed"
)

func APITagRemovedCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config BackwardCompatibilityCheckConfig) []BackwardCompatibilityError {
result := make([]BackwardCompatibilityError, 0)
if diffReport.PathsDiff == nil {
return result
}

for path, pathItem := range diffReport.PathsDiff.Modified {
if pathItem.OperationsDiff == nil {
continue
}

for operation, operationItem := range pathItem.OperationsDiff.Modified {
op := pathItem.Base.Operations()[operation]
source := (*operationsSources)[op]

if operationItem.TagsDiff == nil {
continue
}

for _, tag := range operationItem.TagsDiff.Deleted {
result = append(result, BackwardCompatibilityError{
Id: apiTagRemovedCheckId,
Level: ERR,
Text: fmt.Sprintf(config.i18n(apiTagRemovedCheckId), ColorizedValue(tag)),
Operation: operation,
Path: path,
Source: source,
})

}
}
}
return result
}
18 changes: 18 additions & 0 deletions checker/checker_breaking_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,24 @@ func TestBreaking_OperationIdRemoved(t *testing.T) {
require.Equal(t, "api-operation-id-removed", errs[0].Id)
}

// BC: removing/updating a tag is breaking (optional)
func TestBreaking_TagRemoved(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)

s2.Spec.Paths[securityScorePath].Get.Tags[0] = "newTag"

d, osm, err := diff.GetWithOperationsSourcesMap(&diff.Config{}, &s1, &s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(checker.GetChecks(utils.StringList{"api-tag-removed"}), d, osm)
for _, err := range errs {
require.Equal(t, checker.ERR, err.Level)
}
require.NotEmpty(t, errs)
require.Len(t, errs, 1)
require.Equal(t, "api-tag-removed", errs[0].Id)
}

// BC: removing an existing response with unparseable status is not breaking
func TestBreaking_ResponseUnparseableStatusRemoved(t *testing.T) {
s1 := l(t, 1)
Expand Down
31 changes: 31 additions & 0 deletions checker/checker_not_breaking_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/tufin/oasdiff/checker"
"github.com/tufin/oasdiff/diff"
"github.com/tufin/oasdiff/utils"
)

// BC: no change is not breaking
Expand Down Expand Up @@ -242,3 +243,33 @@ func TestBreaking_Servers(t *testing.T) {
errs := checker.CheckBackwardCompatibility(checker.GetDefaultChecks(), d, osm)
require.Empty(t, errs)
}

// BC: adding a tag is not breaking
func TestBreaking_TagAdded(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)

s2.Spec.Paths[securityScorePath].Get.Tags = append(s2.Spec.Paths[securityScorePath].Get.Tags, "newTag")
d, osm, err := diff.GetWithOperationsSourcesMap(&diff.Config{}, &s1, &s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(checker.GetDefaultChecks(), d, osm)
for _, err := range errs {
require.Equal(t, checker.ERR, err.Level)
}
require.Empty(t, errs)
}

// BC: adding a tag is not breaking with "api-tag-removed" check
func TestBreaking_TagAddedWithCustomCheck(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)

s2.Spec.Paths[securityScorePath].Get.Tags = append(s2.Spec.Paths[securityScorePath].Get.Tags, "newTag")
d, osm, err := diff.GetWithOperationsSourcesMap(&diff.Config{}, &s1, &s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(checker.GetChecks(utils.StringList{"api-tag-removed"}), d, osm)
for _, err := range errs {
require.Equal(t, checker.ERR, err.Level)
}
require.Empty(t, errs)
}
1 change: 1 addition & 0 deletions checker/default_checks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func GetChecks(includeChecks utils.StringList) BackwardCompatibilityCheckConfig
var optionalChecks = map[string]BackwardCompatibilityCheck{
"response-non-success-status-removed": ResponseNonSuccessStatusRemoved,
"api-operation-id-removed": APIOperationIdRemovedCheck,
"api-tag-removed": APITagRemovedCheck,
}

func ValidateIncludeChecks(includeChecks utils.StringList) utils.StringList {
Expand Down
Loading