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

Conversation

vieiralucas
Copy link
Member

Why this matters

Addresses #74 (comment)

@rafiramadhana check this out.

Comment on lines +130 to 134
if tt.err == nil {
assert.NoError(t, err)
} else {
assert.NoError(t, tt.err)
assert.ErrorIs(t, err, tt.err)
}
Copy link
Member Author

@vieiralucas vieiralucas Sep 13, 2023

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! 🤘

} 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

@rafiramadhana
Copy link
Contributor

lgtm

@vieiralucas vieiralucas merged commit 9b1475e into main Sep 13, 2023
3 checks passed
@vieiralucas vieiralucas deleted the definition-name-validation-test-fixes branch September 13, 2023 02:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants