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

license, openGraph tests: convert to testify #12919

Merged
merged 4 commits into from
Oct 31, 2019
Merged

Conversation

drekar
Copy link
Contributor

@drekar drekar commented Oct 25, 2019

Summary

Update api4/license_test.go and api4/openGraph_test.go to use testify.

Ticket Link

Fixes #12860
MM-19625

@hanzei
Copy link
Contributor

hanzei commented Oct 25, 2019

/check-cla

@hanzei hanzei self-requested a review October 25, 2019 21:30
@hanzei hanzei added 2: Dev Review Requires review by a developer Tests/Not Needed New release tests not required labels Oct 25, 2019
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

The code looks fine. I think we can make it a bit cleaner though.

api4/license_test.go Outdated Show resolved Hide resolved
api4/license_test.go Outdated Show resolved Hide resolved
api4/openGraph_test.go Outdated Show resolved Hide resolved
@hanzei hanzei marked this pull request as ready for review October 26, 2019 19:05
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@@ -4,6 +4,8 @@ import (
"net/http"
"testing"

"github.com/stretchr/testify/require"

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line

}
_, err := Client.DoApiGet("/license/client", "")
require.Error(t, err, "get /license/client did not return an error")
require.Equal(t, err.StatusCode, http.StatusNotImplemented,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you can combine these lines.

}
_, err = Client.DoApiGet("/license/client?format=junk", "")
require.Error(t, err, "get /license/client?format=junk did not return an error")
require.Equal(t, err.StatusCode, http.StatusBadRequest,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you can combine these lines.

@hanzei hanzei requested a review from a team October 26, 2019 19:07
@hanzei hanzei added this to the v5.18.0 milestone Oct 26, 2019
@ghost ghost requested review from devinbinnie and imisshtml and removed request for a team October 26, 2019 19:07
@hanzei hanzei removed the request for review from devinbinnie October 26, 2019 19:07
@drekar
Copy link
Contributor Author

drekar commented Oct 29, 2019

Rebasing branch on latest upstream master (caused 4 removed commits, so force pushed to keep the merge log clean)

Copy link

@imisshtml imisshtml left a comment

Choose a reason for hiding this comment

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

These changes look good!

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Oct 31, 2019
@hanzei hanzei merged commit 6f6eb13 into mattermost:master Oct 31, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Oct 31, 2019
jozuenoon pushed a commit to jozuenoon/mattermost-server that referenced this pull request Nov 2, 2019
* master: (70 commits)
  Run unused against codebase (mattermost#12968)
  [MM-12623] Create CLI command "config reset" (mattermost#10296)
  Migrate tests from store/storetest/status_store.go to use test… (mattermost#12873)
  Fix golangci-lint target (mattermost#12985)
  [MM-16437] Plugin crashes the server when calling WriteHeader with an invalid http code (mattermost#11276)
  MM-18060: Include deleted posts in compliance export. (mattermost#12957)
  [MM-18331] When patching a bot send websocket notification (mattermost#12373)
  [MM-19473] Fix data race on user login (mattermost#12870)
  license, openGraph tests: convert to testify (mattermost#12919)
  oauth_test: use testify (mattermost#12949)
  [MM-18830] Unhelpful error message when adding bot to a channel before adding to team (mattermost#12844)
  emoji_test: update to use testify (mattermost#12932)
  MM-19310 - Wrong validation message when Bot name ends in a . (mattermost#12905)
  Migrate tests from store/storetest/oauth_store.go to use testify (mattermost#12875)
  Include extra metadata when clicking an interactive button (mattermost#12697)
  MM-19553: Generate valid passwords on bulk import. (mattermost#12871)
  Convert api4/webhook_test.go t.Fatal calls into require/assert calls (mattermost#12904)
  Fix golangci-lint target for non GOPATH installations (mattermost#12934)
  MM-17888 Check plugin Helpers minimum server version comments (mattermost#12663)
  [MM-18898] Stringify plugin.Log* parameters (mattermost#12700)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Hacktoberfest Tests/Not Needed New release tests not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate tests from "api4/license_test.go" and "api4/openGraph_test.go" to use testify
5 participants