diff --git a/.gitignore b/.gitignore index bfe07b4..f980622 100644 --- a/.gitignore +++ b/.gitignore @@ -30,5 +30,6 @@ _testmain.go # vim swap files *.swp -# test coverage report file -coverage.out +# test coverage report files +*.out +*.html diff --git a/bot.go b/bot.go index badf4c5..28cc177 100644 --- a/bot.go +++ b/bot.go @@ -60,7 +60,12 @@ func (b *Bot) startPeriodicCommands() { // MessageReceived must be called by the protocol upon receiving a message func (b *Bot) MessageReceived(channel string, text string, sender *User) { - command := parse(text, channel, sender) + command, err := parse(text, channel, sender) + if err != nil { + b.handlers.Response(channel, err.Error(), sender) + return + } + if command == nil { b.executePassiveCommands(&PassiveCmd{ Raw: text, diff --git a/cmd_test.go b/cmd_test.go index 1247da5..73f71ef 100644 --- a/cmd_test.go +++ b/cmd_test.go @@ -26,8 +26,13 @@ func resetResponses() { replies = []string{} } +func resetRegisteredPeriodicCommands() { + periodicCommands = make(map[string]PeriodicConfig) +} + func TestPeriodicCommands(t *testing.T) { resetResponses() + resetRegisteredPeriodicCommands() RegisterPeriodicCommand("morning", PeriodicConfig{ CronSpec: "0 0 08 * * mon-fri", @@ -54,6 +59,30 @@ func TestPeriodicCommands(t *testing.T) { } } +func TestErroredPeriodicCommand(t *testing.T) { + resetResponses() + resetRegisteredPeriodicCommands() + RegisterPeriodicCommand("bugged", + PeriodicConfig{ + CronSpec: "0 0 08 * * mon-fri", + Channels: []string{"#channel"}, + CmdFunc: func(channel string) (string, error) { return "bug", errors.New("error") }, + }) + b := New(&Handlers{Response: responseHandler}) + + entries := b.cron.Entries() + + if len(entries) != 1 { + t.Fatal("Should have one cron job entry") + } + + entries[0].Job.Run() + + if len(replies) != 0 { + t.Fatal("Should not have a reply in the channel") + } +} + func TestDisabledCommands(t *testing.T) { resetResponses() commands = make(map[string]*customCommand) @@ -101,6 +130,16 @@ func TestMessageReceived(t *testing.T) { }) }) + Convey("When the command arguments are invalid", func() { + Convey("It should reply with an error message", func() { + b.MessageReceived("#go-bot", "!cmd \"invalid arg", &User{Nick: "user"}) + + So(channel, ShouldEqual, "#go-bot") + So(replies, ShouldHaveLength, 1) + So(replies[0], ShouldStartWith, "Error parsing") + }) + }) + Convey("The command can return an error", func() { Convey("it sould send the message with the error to the channel", func() { cmdError := errors.New("error") @@ -172,6 +211,14 @@ func TestMessageReceived(t *testing.T) { fmt.Sprintf(availableCommands, "cmd"), }) }) + + Convey("if the help arguments are invalid, reply with an error", func() { + b.MessageReceived("#go-bot", "!help cmd \"invalid arg", &User{Nick: "user"}) + + So(channel, ShouldEqual, "#go-bot") + So(replies, ShouldHaveLength, 1) + So(replies[0], ShouldStartWith, "Error parsing") + }) }) }) diff --git a/coverage.sh b/coverage.sh new file mode 100755 index 0000000..4b04a29 --- /dev/null +++ b/coverage.sh @@ -0,0 +1,3 @@ +#!/bin/bash +go test -coverprofile cover.out +go tool cover -html=cover.out -o cover.html diff --git a/help.go b/help.go index 4350378..1d2caf0 100644 --- a/help.go +++ b/help.go @@ -14,7 +14,7 @@ const ( ) func (b *Bot) help(c *Cmd) { - cmd := parse(CmdPrefix+c.RawArgs, c.Channel, c.User) + cmd, _ := parse(CmdPrefix+c.RawArgs, c.Channel, c.User) if cmd == nil { b.showAvailabeCommands(c.Channel, c.User) return diff --git a/parser.go b/parser.go index b876bf2..97be8f1 100644 --- a/parser.go +++ b/parser.go @@ -1,20 +1,23 @@ package bot import ( + "errors" "regexp" "strings" + + "github.com/mattn/go-shellwords" ) var ( re = regexp.MustCompile("\\s+") // Matches one or more spaces ) -func parse(s string, channel string, user *User) *Cmd { +func parse(s string, channel string, user *User) (*Cmd, error) { c := &Cmd{Raw: s} s = strings.TrimSpace(s) if !strings.HasPrefix(s, CmdPrefix) { - return nil + return nil, nil } c.Channel = strings.TrimSpace(channel) @@ -26,7 +29,7 @@ func parse(s string, channel string, user *User) *Cmd { // check if we have the command and not only the prefix if c.Message == "" { - return nil + return nil, nil } // get the command @@ -36,12 +39,12 @@ func parse(s string, channel string, user *User) *Cmd { if len(pieces) > 1 { // get the arguments and remove extra spaces c.RawArgs = strings.TrimSpace(pieces[1]) - c.Args = strings.Split(removeExtraSpaces(c.RawArgs), " ") + parsedArgs, err := shellwords.Parse(c.RawArgs) + if err != nil { + return nil, errors.New("Error parsing arguments: " + err.Error()) + } + c.Args = parsedArgs } - return c -} - -func removeExtraSpaces(args string) string { - return re.ReplaceAllString(strings.TrimSpace(args), " ") + return c, nil } diff --git a/parser_test.go b/parser_test.go index 60152b1..ca7bd79 100644 --- a/parser_test.go +++ b/parser_test.go @@ -10,6 +10,7 @@ func TestPaser(t *testing.T) { user := &User{Nick: "user123"} cmdWithoutArgs := CmdPrefix + "cmd" cmdWithArgs := CmdPrefix + "cmd arg1 arg2 " + cmdWithQuotes := CmdPrefix + "cmd \"arg1 arg2\"" tests := []struct { msg string @@ -34,12 +35,31 @@ func TestPaser(t *testing.T) { RawArgs: "arg1 arg2", Args: []string{"arg1", "arg2"}, }}, + {cmdWithQuotes, &Cmd{ + Raw: cmdWithQuotes, + Command: "cmd", + Channel: channel, + User: user, + Message: "cmd \"arg1 arg2\"", + RawArgs: "\"arg1 arg2\"", + Args: []string{"arg1 arg2"}, + }}, } for _, test := range tests { - cmd := parse(test.msg, channel, user) + cmd, _ := parse(test.msg, channel, user) if !reflect.DeepEqual(test.expected, cmd) { t.Errorf("Expected:\n%#v\ngot:\n%#v", test.expected, cmd) } } } + +func TestInvalidArguments(t *testing.T) { + cmd, err := parse("!cmd Invalid \"arg", "#go-bot", &User{Nick: "user123"}) + if err == nil { + t.Error("Expected error, got nil") + } + if cmd != nil { + t.Errorf("Expected nil, got %#v", cmd) + } +}