From b7e830f4a13f3583d0ab8e79fc8eae351c460af4 Mon Sep 17 00:00:00 2001 From: Ezekiel Date: Wed, 1 May 2024 04:16:05 +0800 Subject: [PATCH] Print & Return errors using multierror if an error happens (#26625) * Print & Return errors using multierror if an error happens * Group up errors assertion in test * Remove trailing newspace * Remove WrappedErrors loop --------- Co-authored-by: Mattermost Build --- server/cmd/mmctl/commands/channel.go | 12 +++-- server/cmd/mmctl/commands/channel_e2e_test.go | 15 +++--- server/cmd/mmctl/commands/channel_test.go | 50 +++++++++---------- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/server/cmd/mmctl/commands/channel.go b/server/cmd/mmctl/commands/channel.go index f43d2ea0ee..fe02a499cf 100644 --- a/server/cmd/mmctl/commands/channel.go +++ b/server/cmd/mmctl/commands/channel.go @@ -368,18 +368,24 @@ func unarchiveChannelsCmdF(c client.Client, cmd *cobra.Command, args []string) e return errors.New("enter at least one channel") } + var errs *multierror.Error + channels := getChannelsFromChannelArgs(c, args) for i, channel := range channels { if channel == nil { - printer.PrintError("Unable to find channel '" + args[i] + "'") + msg := "Unable to find channel '" + args[i] + "'" + printer.PrintError(msg) + errs = multierror.Append(errs, errors.New(msg)) continue } if _, _, err := c.RestoreChannel(context.TODO(), channel.Id); err != nil { - printer.PrintError("Unable to unarchive channel '" + args[i] + "'. Error: " + err.Error()) + msg := "Unable to unarchive channel '" + args[i] + "'. Error: " + err.Error() + printer.PrintError(msg) + errs = multierror.Append(errs, errors.New(msg)) } } - return nil + return errs.ErrorOrNil() } func makeChannelPrivateCmdF(c client.Client, cmd *cobra.Command, args []string) error { diff --git a/server/cmd/mmctl/commands/channel_e2e_test.go b/server/cmd/mmctl/commands/channel_e2e_test.go index 21ac67c134..f047ca7d2c 100644 --- a/server/cmd/mmctl/commands/channel_e2e_test.go +++ b/server/cmd/mmctl/commands/channel_e2e_test.go @@ -291,8 +291,9 @@ func (s *MmctlE2ETestSuite) TestUnarchiveChannelsCmdF() { printer.Clean() err := unarchiveChannelsCmdF(s.th.Client, &cobra.Command{}, []string{fmt.Sprintf("%s:%s", s.th.BasicTeam.Id, s.th.BasicDeletedChannel.Name)}) - s.Require().Nil(err) - s.Require().Contains(printer.GetErrorLines()[0], fmt.Sprintf("Unable to unarchive channel '%s:%s'", s.th.BasicTeam.Id, s.th.BasicDeletedChannel.Name)) + expectedError := fmt.Sprintf("Unable to unarchive channel '%s:%s'", s.th.BasicTeam.Id, s.th.BasicDeletedChannel.Name) + s.Require().ErrorContains(err, expectedError) + s.Require().Contains(printer.GetErrorLines()[0], expectedError) s.Require().Contains(printer.GetErrorLines()[0], "You do not have the appropriate permissions.") }) @@ -300,16 +301,18 @@ func (s *MmctlE2ETestSuite) TestUnarchiveChannelsCmdF() { printer.Clean() err := unarchiveChannelsCmdF(c, &cobra.Command{}, []string{fmt.Sprintf("%s:%s", s.th.BasicTeam.Id, "nonexistent-channel")}) - s.Require().Nil(err) - s.Require().Contains(printer.GetErrorLines()[0], fmt.Sprintf("Unable to find channel '%s:%s'", s.th.BasicTeam.Id, "nonexistent-channel")) + expectedError := fmt.Sprintf("Unable to find channel '%s:%s'", s.th.BasicTeam.Id, "nonexistent-channel") + s.Require().ErrorContains(err, expectedError) + s.Require().Contains(printer.GetErrorLines()[0], expectedError) }) s.Run("Unarchive open channel", func() { printer.Clean() err := unarchiveChannelsCmdF(s.th.SystemAdminClient, &cobra.Command{}, []string{fmt.Sprintf("%s:%s", s.th.BasicTeam.Id, s.th.BasicChannel.Name)}) - s.Require().Nil(err) - s.Require().Contains(printer.GetErrorLines()[0], fmt.Sprintf("Unable to unarchive channel '%s:%s'", s.th.BasicTeam.Id, s.th.BasicChannel.Name)) + expectedError := fmt.Sprintf("Unable to unarchive channel '%s:%s'", s.th.BasicTeam.Id, s.th.BasicChannel.Name) + s.Require().ErrorContains(err, expectedError) + s.Require().Contains(printer.GetErrorLines()[0], expectedError) s.Require().Contains(printer.GetErrorLines()[0], "Unable to unarchive channel. The channel is not archived.") }) } diff --git a/server/cmd/mmctl/commands/channel_test.go b/server/cmd/mmctl/commands/channel_test.go index deee9da73f..0f3225fea5 100644 --- a/server/cmd/mmctl/commands/channel_test.go +++ b/server/cmd/mmctl/commands/channel_test.go @@ -1783,13 +1783,13 @@ func (s *MmctlUnitTestSuite) TestUnarchiveChannelCmdF() { Times(1) err := unarchiveChannelsCmdF(s.client, cmd, args) - s.Require().Nil(err) + expectedError := fmt.Sprintf("Unable to find channel '%s'", args[0]) + + s.Require().ErrorContains(err, expectedError) + s.Require().Equal(expectedError, printer.GetErrorLines()[0]) + s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - - actual := printer.GetErrorLines()[0] - expected := fmt.Sprintf("Unable to find channel '%s'", args[0]) - s.Require().Equal(expected, actual) }) s.Run("Fail to unarchive a non-existing channel on an existent team", func() { @@ -1821,13 +1821,13 @@ func (s *MmctlUnitTestSuite) TestUnarchiveChannelCmdF() { Times(1) err := unarchiveChannelsCmdF(s.client, cmd, args) - s.Require().Nil(err) + expectedError := fmt.Sprintf("Unable to find channel '%s'", args[0]) + + s.Require().ErrorContains(err, expectedError) + s.Require().Equal(expectedError, printer.GetErrorLines()[0]) + s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - - actual := printer.GetErrorLines()[0] - expected := fmt.Sprintf("Unable to find channel '%s'", args[0]) - s.Require().Equal(expected, actual) }) s.Run("Fail to unarchive a non-existing channel", func() { @@ -1844,13 +1844,13 @@ func (s *MmctlUnitTestSuite) TestUnarchiveChannelCmdF() { Times(1) err := unarchiveChannelsCmdF(s.client, cmd, args) - s.Require().Nil(err) + expectedError := fmt.Sprintf("Unable to find channel '%s'", args[0]) + + s.Require().ErrorContains(err, expectedError) + s.Require().Equal(expectedError, printer.GetErrorLines()[0]) + s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - - actual := printer.GetErrorLines()[0] - expected := fmt.Sprintf("Unable to find channel '%s'", args[0]) - s.Require().Equal(expected, actual) }) s.Run("Fail to unarchive an existing channel when client throws error", func() { @@ -1875,13 +1875,13 @@ func (s *MmctlUnitTestSuite) TestUnarchiveChannelCmdF() { Times(1) err := unarchiveChannelsCmdF(s.client, cmd, args) - s.Require().Nil(err) + expectedError := fmt.Sprintf("Unable to unarchive channel '%s'. Error: %s", channelName, mockErr.Error()) + + s.Require().ErrorContains(err, expectedError) + s.Require().Equal(expectedError, printer.GetErrorLines()[0]) + s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - - actual := printer.GetErrorLines()[0] - expected := fmt.Sprintf("Unable to unarchive channel '%s'. Error: %s", channelName, mockErr.Error()) - s.Require().Equal(expected, actual) }) s.Run("Fail to unarchive when team and channel not provided", func() { @@ -1891,13 +1891,13 @@ func (s *MmctlUnitTestSuite) TestUnarchiveChannelCmdF() { args := []string{":"} err := unarchiveChannelsCmdF(s.client, cmd, args) - s.Require().Nil(err) + expectedError := fmt.Sprintf("Unable to find channel '%s'", args[0]) + + s.Require().ErrorContains(err, expectedError) + s.Require().Equal(expectedError, printer.GetErrorLines()[0]) + s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - - actual := printer.GetErrorLines()[0] - expected := fmt.Sprintf("Unable to find channel '%s'", args[0]) - s.Require().Equal(expected, actual) }) }