Mattermost-server: Add unit tests to the "UploadLicenseCmd" mmctl command

Created on 19 Nov 2019  路  13Comments  路  Source: mattermost/mattermost-server

Mattermost is increasing the unit test coverage for the mmctl CLI tool. Through the use of a mocked client that will simulate the interactions with the server, the goal is to create a suite of tests that check the tool's behavior in a flexible and robust way.

This Help Wanted issue covers the addition of unit tests to the UploadLicenseCmd command, in the commands/license.go file. The expected way to add these tests is to create a test function in the commands/license_test.go file and add one s.Run block for each possible test case.

For more information on how to add a unit test to mmctl, take a look at the campaign blogpost: https://developers.mattermost.com/blog/unit-testing-mmctl-commands/

Example: mattermost/mmctl#37

If you have questions about the ticket or you need any help, feel free to post your question in the community CLI channel or contact miguel.delacruz in https://community.mattermost.com/.


If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-20291

AreCLI AreTechnical Debt Aremmctl Medium Help Wanted PR Exists TecGo

Most helpful comment

Alright @jasonblais and others i'll try to work on ASAP

All 13 comments

Can I take this one?

@mgdelacroix I can take it

Hi @jimiolaniyan, let us know if you're still open to working on this one? Let us know if you have any questions!

Else, looks like @igomonov88 may be interested working on this one :)

Hi @jasonblais I tried to work on this but i ran into this error:

missing call(s) to *mocks.MockClient.UploadLicenseFile

in mmctl_test.go

Here is the test function:

func (s *MmctlUnitTestSuite) TestUploadLicenseCmd() {
    s.Run("Successfully upload one license", func() {
        printer.Clean()
        file := "license.txt"

        s.client.
            EXPECT().
            UploadLicenseFile(file).
            Return(true, &model.Response{Error: nil}).
            Times(1)

        err := uploadLicenseCmdF(s.client, &cobra.Command{}, []string{file})
        s.Require().Nil(err)
    })
}

@M-ZubairAhmed or @larkox wondering if you'd know why that error occurs for @jimiolaniyan?

(cc @reflog and @mgdelacroix as well, but I think you guys are off this week)

I would say that the problem is that, since it cannot open the file to read, it never calls uploadlicensefile. Therefore the error. On top of that, uploadlicensefile is called with a stream of bytes, not the filename. That might be another source of the error (it is never called with those parameters).

In order to solve this, I would mock also the ioutil function in order to receive the values we can control. Another solution would be to create a file for this test, but it does not feel as the optimal option.

thanks! @jimiolaniyan let us know if that's helpful or if we can help clarify?

@jimiolaniyan @larkox is right on 100% thae answer, just a general rule is that when ever you ser errors starting with missing call(s) toerrors it means you have missing a function to mock.

Alright @jasonblais and others i'll try to work on ASAP

I can finish this one if you want @jasonblais @hanzei

@jimiolaniyan, heads up that @fedealconada has expressed an interest to help with this one, so I've assigned the ticket his way. If you're still making progress on this one, feel free to ping @fedealconada and coordinate on that front. No worries if you'd prefer to pass :)

@lieut-data I've just submitted a PR. The linter is complaining and I'm not sure how to solve that... I know sometimes golangci-cli is not so clear with messaged xD but I'm still not sure how to fix that... Any ideas?

So sorry I couldn't didn't get the chance to complete this, thanks for taking it up @fedealconada

Was this page helpful?
0 / 5 - 0 ratings