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

Fix unit test ValidateFeedURL and ValidateURL #1160

Merged
merged 8 commits into from Jul 24, 2020
Merged

Fix unit test ValidateFeedURL and ValidateURL #1160

merged 8 commits into from Jul 24, 2020

Conversation

MChorfa
Copy link
Contributor

@MChorfa MChorfa commented Jul 24, 2020

What does this change

Fix a failing unit test when running porter with go 1.14

Checklist

  • Unit Tests

If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Copy link
Member

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Thanks for fixing Porter on go 1.14! I often end up regretting asserting on the entire error message 😅

If you wouldn't mind tweaking which assert you are using, then this is ready to merge.

@@ -75,7 +77,8 @@ func TestInstallOptions_ValidateFeedURL(t *testing.T) {
FeedURL: "$://example.com",
}
err := opts.validateFeedURL()
require.EqualError(t, err, "invalid --feed-url $://example.com: parse $://example.com: first path segment in URL cannot contain colon")
assert.True(t, strings.Contains(err.Error(), fmt.Sprintf("invalid --feed-url %s", opts.FeedURL)))
Copy link
Member

Choose a reason for hiding this comment

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

I wanted to let you know about another testify function assert.Contains that can simplify your checks and improve the error message when the check fails:

Suggested change
assert.True(t, strings.Contains(err.Error(), fmt.Sprintf("invalid --feed-url %s", opts.FeedURL)))
assert.Contains(t, err.Error(), fmt.Sprintf("invalid --feed-url %s", opts.FeedURL))

@carolynvs carolynvs self-assigned this Jul 24, 2020
@carolynvs carolynvs added this to In Progress in Porter and Mixins [OLD] via automation Jul 24, 2020
@MChorfa MChorfa requested a review from carolynvs July 24, 2020 13:54
Copy link
Member

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Thanks for getting us go 1.14 ready! 💯

@carolynvs carolynvs merged commit c9a7d9e into getporter:main Jul 24, 2020
Porter and Mixins [OLD] automation moved this from In Progress to Done Jul 24, 2020
@vdice vdice removed this from Done in Porter and Mixins [OLD] Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants