Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

make layerfile name definition validation tests more reliable #86

Merged
merged 1 commit into from
Sep 13, 2023
Merged

make layerfile name definition validation tests more reliable #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 5 additions & 6 deletions internal/layerfile/layerfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"path"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -100,7 +99,7 @@ func TestToLayers_ValidateNameOfLayerDefinitions(t *testing.T) {
},
},
},
err: errors.Wrap(ErrInvalidDefinitionName, "invalid name for a layer definition"),
err: ErrInvalidDefinitionName,
},
{
name: "Name has special character",
Expand All @@ -111,7 +110,7 @@ func TestToLayers_ValidateNameOfLayerDefinitions(t *testing.T) {
},
},
},
err: errors.Wrap(ErrInvalidDefinitionName, "invalid!"),
err: ErrInvalidDefinitionName,
},
{
name: "Valid name",
Expand All @@ -128,10 +127,10 @@ func TestToLayers_ValidateNameOfLayerDefinitions(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := tt.lf.ToLayers()
if err != nil {
assert.EqualError(t, tt.err, err.Error())
if tt.err == nil {
assert.NoError(t, err)
} else {
assert.NoError(t, tt.err)
assert.ErrorIs(t, err, tt.err)
Copy link
Member Author

Choose a reason for hiding this comment

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

Also, it's fine to just use ErrorIs, I don't want this test to fail if we change error messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, got it

}
Comment on lines +130 to 134
Copy link
Member Author

@vieiralucas vieiralucas Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Changed this because we should assert that there is an error or not based on tt.err being defined or not. That's because what is defined in tt is what we expect to happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

i see, thanks for the pointer

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for the contribution! 🤘

})
}
Expand Down
Loading