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.

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

Add convenience functions for ErrorsCount, WarningsCount, Errors andWarnings #392

Merged
merged 4 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/392.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
diag: `ErrorsCount`, `WarningsCount`, `Errors` and `Warnings` functions have been added to `diag.Diagnostics`
```
36 changes: 36 additions & 0 deletions diag/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,39 @@ func (diags Diagnostics) HasError() bool {

return false
}

// ErrorsCount returns the number of Diagnostic in Diagnostics that are SeverityError.
func (diags Diagnostics) ErrorsCount() int {
return len(diags.Errors())
}

// WarningsCount returns the number of Diagnostic in Diagnostics that are SeverityWarning.
func (diags Diagnostics) WarningsCount() int {
return len(diags.Warnings())
}

// Errors returns all the Diagnostic in Diagnostics that are SeverityError.
func (diags Diagnostics) Errors() Diagnostics {
dd := Diagnostics{}

for _, d := range diags {
if SeverityError == d.Severity() {
dd = append(dd, d)
}
}

return dd
}

// Warnings returns all the Diagnostic in Diagnostics that are SeverityWarning.
func (diags Diagnostics) Warnings() Diagnostics {
dd := Diagnostics{}

for _, d := range diags {
if SeverityWarning == d.Severity() {
dd = append(dd, d)
}
}

return dd
}
177 changes: 177 additions & 0 deletions diag/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
)
Expand Down Expand Up @@ -518,3 +519,179 @@ func TestDiagnosticsHasError(t *testing.T) {
})
}
}

func TestDiagnosticsErrorsCount(t *testing.T) {
t.Parallel()

type testCase struct {
diags diag.Diagnostics
expected int
}
tests := map[string]testCase{
"nil": {
diags: nil,
expected: 0,
},
"empty": {
diags: diag.Diagnostics{},
expected: 0,
},
"errors": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Similar to the Errors testing, I think we should cover some additional cases just to ensure it returns 0 instead of panics, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional test coverage.

diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
expected: 1,
},
"warnings": {
diags: diag.Diagnostics{
diag.NewWarningDiagnostic("Error Summary", "Error detail."),
},
expected: 0,
},
}

for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
got := test.diags.ErrorsCount()

if diff := cmp.Diff(test.expected, got); diff != "" {
t.Fatalf("expected: %q, got: %q", test.expected, got)
}
})
}
}

func TestDiagnosticsWarningsCount(t *testing.T) {
t.Parallel()

type testCase struct {
diags diag.Diagnostics
expected int
}
tests := map[string]testCase{
"nil": {
diags: nil,
expected: 0,
},
"empty": {
diags: diag.Diagnostics{},
expected: 0,
},
"errors": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Similar to the Warning testing, I think we should cover some additional cases just to ensure it returns 0 instead of panics, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional test coverage.

diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
expected: 1,
},
"warnings": {
diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
},
expected: 0,
},
}

for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
got := test.diags.WarningsCount()

if diff := cmp.Diff(test.expected, got); diff != "" {
t.Fatalf("expected: %q, got: %q", test.expected, got)
}
})
}
}

func TestDiagnosticsErrors(t *testing.T) {
t.Parallel()

type testCase struct {
diags diag.Diagnostics
expected diag.Diagnostics
}
tests := map[string]testCase{
"nil": {
diags: nil,
expected: diag.Diagnostics{},
},
"empty": {
diags: diag.Diagnostics{},
expected: diag.Diagnostics{},
},
"errors": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Might be good to check what happens when:

  • There is a nil diag.Diagnostics, it should return nil or empty diag.Diagnostics
  • There is an empty diag.Diagnostics, it should return empty diag.Diagnostics
  • There are only warning diagnostics, it should return empty diag.Diagnostics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional test coverage.

diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
expected: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
},
},
"warnings": {
diags: diag.Diagnostics{
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
expected: diag.Diagnostics{},
},
}

for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
got := test.diags.Errors()

if diff := cmp.Diff(test.expected, got); diff != "" {
t.Fatalf("expected: %q, got: %q", test.expected, got)
}
})
}
}

func TestDiagnosticsWarnings(t *testing.T) {
t.Parallel()

type testCase struct {
diags diag.Diagnostics
expected diag.Diagnostics
}
tests := map[string]testCase{
"nil": {
diags: nil,
expected: diag.Diagnostics{},
},
"empty": {
diags: diag.Diagnostics{},
expected: diag.Diagnostics{},
},
"errors": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Might be good to check what happens when:

  • There is a nil diag.Diagnostics, it should return nil or empty diag.Diagnostics
  • There is an empty diag.Diagnostics, it should return empty diag.Diagnostics
  • There are only error diagnostics, it should return empty diag.Diagnostics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional test coverage.

diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
},
expected: diag.Diagnostics{},
},
"warnings": {
diags: diag.Diagnostics{
diag.NewErrorDiagnostic("Error Summary", "Error detail."),
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
expected: diag.Diagnostics{
diag.NewWarningDiagnostic("Warning Summary", "Warning detail."),
},
},
}

for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
got := test.diags.Warnings()

if diff := cmp.Diff(test.expected, got); diff != "" {
t.Fatalf("expected: %q, got: %q", test.expected, got)
}
})
}
}