Skip to content

Commit

Permalink
Merge pull request #487 from innogames/cleanup
Browse files Browse the repository at this point in the history
Some cleanups + more tests etc
  • Loading branch information
brainexe authored Oct 22, 2023
2 parents c8b35ab + 7f6fe46 commit f29110e
Show file tree
Hide file tree
Showing 19 changed files with 107 additions and 87 deletions.
2 changes: 2 additions & 0 deletions bot/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ func (b *Bot) Run(ctx *util.ServerContext) {
// wait until other services are properly shut down
ctx.StopTheWorld()
return
case <-ctx.Done():
return
}
}
}
Expand Down
24 changes: 0 additions & 24 deletions bot/matcher/result.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package matcher

import (
"regexp"
"strconv"
)

Expand All @@ -18,26 +17,3 @@ func (m Result) GetInt(key string) int {
number, _ := strconv.Atoi(m[key])
return number
}

// ReResult is a regexp based result which is returned by RegexpMatcher
type ReResult struct {
match []string
re *regexp.Regexp
}

// GetString returns a parameter, casted as string
func (m ReResult) GetString(key string) string {
for idx, name := range m.re.SubexpNames() {
if name == key {
return m.match[idx]
}
}

return ""
}

// GetInt returns a parameter, casted as int
func (m ReResult) GetInt(key string) int {
number, _ := strconv.Atoi(m.GetString(key))
return number
}
1 change: 1 addition & 0 deletions bot/util/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var parserTestCases = []struct {
{"10h12s", "10h12s"},
{"10h12sec", "10h12s"},
{"10h12.5sec", "10h12.5s"},
{"10h12.0sec", "10h12s"},
{"10h1min12sec", "10h1m12s"},
{"10h", "10h0m0s"},
{"0d", "0s"},
Expand Down
1 change: 1 addition & 0 deletions cmd/cli/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func TestAll(t *testing.T) {
defer lock.Unlock()

ctx := util.NewServerContext()
defer ctx.StopTheWorld()

expectedOutput := &util.MutexBuffer{}
expectedOutput.Write([]byte("Type in your command:\n"))
Expand Down
4 changes: 4 additions & 0 deletions command/clouds/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func TestGetCommands(t *testing.T) {
commands := GetCommands(cfg, base)
assert.Equal(t, 2, commands.Count())

// test help
help := commands.GetHelp()
assert.Equal(t, 2, len(help))

// list the CF
message := msg.Message{}
message.Text = "aws cf list"
Expand Down
18 changes: 12 additions & 6 deletions command/cron/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ func NewCronCommand(base bot.BaseCommand, crons []config.Cron) bot.Command {
return nil
}

cron := cronLib.New()
cron := cronLib.New(
cronLib.WithLogger(newCronLogger()),
)
cmd := &command{base, crons, cron}

for _, cronCommand := range crons {
Expand All @@ -28,9 +30,6 @@ func NewCronCommand(base bot.BaseCommand, crons []config.Cron) bot.Command {
}
}

cron.Start()
log.Infof("Initialized %d crons", len(crons))

return cmd
}

Expand All @@ -40,8 +39,15 @@ type command struct {
cron *cronLib.Cron
}

func (c *command) IsEnabled() bool {
return len(c.cfg) > 0
// RunAsync provide proper Cron start/stop in a async context
func (c *command) RunAsync(ctx *util.ServerContext) {
ctx.RegisterChild()
defer ctx.ChildDone()

c.cron.Start()
log.Infof("Initialized %d crons", len(c.cfg))

<-ctx.Done()
}

func (c *command) getCallback(cron config.Cron) func() {
Expand Down
7 changes: 7 additions & 0 deletions command/cron/cron_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/innogames/slack-bot/v2/bot"
"github.com/innogames/slack-bot/v2/bot/config"
"github.com/innogames/slack-bot/v2/bot/msg"
"github.com/innogames/slack-bot/v2/bot/util"
"github.com/innogames/slack-bot/v2/mocks"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -49,6 +50,12 @@ func TestCron(t *testing.T) {
commands := bot.Commands{}
commands.AddCommand(command)

t.Run("Run in background", func(t *testing.T) {
ctx := util.NewServerContext()
go command.RunAsync(ctx)
ctx.StopTheWorld()
})

t.Run("List crons", func(t *testing.T) {
message := msg.Message{}
message.Text = "list crons"
Expand Down
9 changes: 7 additions & 2 deletions command/cron/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,19 @@ func (c *command) GetMatcher() matcher.Matcher {
return matcher.NewTextMatcher("list crons", c.listCrons)
}

func (c *command) listCrons(match matcher.Result, message msg.Message) {
func (c *command) listCrons(_ matcher.Result, message msg.Message) {
text := fmt.Sprintf("*%d crons:*\n", len(c.cfg))

now := time.Now()
for i, entry := range c.cron.Entries() {
last := ""
if !entry.Prev.IsZero() {
last = fmt.Sprintf("last %s, ", entry.Prev)
}
text += fmt.Sprintf(
" - `%s`, next in %s (`%s`)\n",
" - `%s`, %snext in %s (`%s`)\n",
c.cfg[i].Schedule,
last,
util.FormatDuration(entry.Next.Sub(now)),
strings.Join(c.cfg[i].Commands, "; "),
)
Expand Down
22 changes: 22 additions & 0 deletions command/cron/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package cron

import log "github.com/sirupsen/logrus"

// special logger is needed for the cron lib...
type cronLogger struct {
logger *log.Logger
}

func newCronLogger() cronLogger {
return cronLogger{log.StandardLogger()}
}

func (l cronLogger) Info(msg string, keysAndValues ...any) {
data := append([]any{"Cron", msg}, keysAndValues...)
l.logger.Debug(data...)
}

func (l cronLogger) Error(err error, msg string, keysAndValues ...any) {
data := append([]any{"Cron", err, msg}, keysAndValues...)
l.logger.Error(data...)
}
4 changes: 2 additions & 2 deletions command/jenkins/build_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ func newBuildWatcherCommand(base jenkinsCommand) bot.Command {
}

func (c *buildWatcherCommand) GetMatcher() matcher.Matcher {
return matcher.NewRegexpMatcher(`(notify|inform)( me about)? (job|build) ?(?P<job>[\w\-_\\/]*)( #?(?P<build>\d+))?`, c.run)
return matcher.NewRegexpMatcher(`(notify|inform)( me about)? (job|build) ?(?P<job>[\w\-_\\/]*)( #?(?P<build>\d+))?`, c.watch)
}

func (c *buildWatcherCommand) run(match matcher.Result, message msg.Message) {
func (c *buildWatcherCommand) watch(match matcher.Result, message msg.Message) {
jobName := match.GetString("job")
buildNumber := match.GetInt("build")

Expand Down
4 changes: 2 additions & 2 deletions command/jenkins/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ func newNodesCommand(base jenkinsCommand, cfg config.Jenkins) bot.Command {
}

func (c *nodesCommand) GetMatcher() matcher.Matcher {
return matcher.NewTextMatcher("list jenkins nodes", c.run)
return matcher.NewTextMatcher("list jenkins nodes", c.listNodes)
}

func (c *nodesCommand) run(match matcher.Result, message msg.Message) {
func (c *nodesCommand) listNodes(_ matcher.Result, message msg.Message) {
ctx := context.Background()
nodes, err := c.jenkins.GetAllNodes(ctx)
if err != nil {
Expand Down
13 changes: 5 additions & 8 deletions command/openai/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package openai

import (
"regexp"

"github.com/tiktoken-go/tokenizer"
)

// https://platform.openai.com/docs/models/gpt-3-5
Expand All @@ -25,7 +23,7 @@ func truncateMessages(model string, inputMessages []ChatMessage) ([]ChatMessage,
truncatedMessages := 0
maxTokens := getMaxTokensForModel(model)
for _, message := range inputMessages {
tokens := countTokensForMessage(message)
tokens := estimateTokensForMessage(message)

if currentTokens+tokens >= maxTokens {
truncatedMessages++
Expand All @@ -51,9 +49,8 @@ func getMaxTokensForModel(model string) int {
return 4000
}

func countTokensForMessage(message ChatMessage) int {
enc, _ := tokenizer.ForModel(tokenizer.GPT4)
tokens, _, _ := enc.Encode(message.Content)

return len(tokens)
func estimateTokensForMessage(message ChatMessage) int {
// to lower the dependency to heavy external libs we use the rule of thumbs which is totally fine here
// https://platform.openai.com/tokenizer
return len(message.Content) / 4
}
10 changes: 5 additions & 5 deletions command/openai/tokens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ func TestTruncate(t *testing.T) {

outputMessages, inputTokens, truncatedMessages := truncateMessages("dummy-test", messages)

assert.Equal(t, 6, len(outputMessages))
assert.Equal(t, 99, inputTokens)
assert.Equal(t, 1, truncatedMessages)
assert.Equal(t, 5, len(outputMessages))
assert.Equal(t, 85, inputTokens)
assert.Equal(t, 2, truncatedMessages)
}

func TestCountTokens(t *testing.T) {
t.Run("Count", func(t *testing.T) {
actual := countTokensForMessage(ChatMessage{Content: "hello you!"})
assert.Equal(t, 3, actual)
actual := estimateTokensForMessage(ChatMessage{Content: "hello you!"})
assert.Equal(t, 2, actual)
})
}
3 changes: 3 additions & 0 deletions command/pool/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/stretchr/testify/assert"
)

// compile time check that the interface matches
var _ bot.Runnable = &poolCommands{}

func TestPools(t *testing.T) {
slackClient := &mocks.SlackClient{}
base := bot.BaseCommand{SlackClient: slackClient}
Expand Down
5 changes: 2 additions & 3 deletions command/pool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ import (
"sync"
"time"

"github.com/innogames/slack-bot/v2/bot/util"

"github.com/innogames/slack-bot/v2/bot/config"
"github.com/innogames/slack-bot/v2/bot/storage"
"github.com/innogames/slack-bot/v2/bot/util"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
)
Expand All @@ -36,7 +35,7 @@ type ResourceLock struct {
type pool struct {
locks map[*config.Resource]*ResourceLock
lockDuration time.Duration
mu sync.RWMutex
mu sync.Mutex
}

// getNewPool create a new pool and initialize it by the local storage
Expand Down
6 changes: 3 additions & 3 deletions command/vcs/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ func GetCommands(base bot.BaseCommand, config *config.Config) bot.Commands {
return commands
}

// RunAsync registers regular branch updates in the backjground with proper stopping on exit
// RunAsync registers regular branch updates in the background with proper stopping on exit
func (c *vcsCommand) RunAsync(ctx *util.ServerContext) {
go vcs.InitBranchWatcher(c.cfg, ctx)
vcs.InitBranchWatcher(c.cfg, ctx)
}

type vcsCommand struct {
Expand All @@ -40,7 +40,7 @@ func (c *vcsCommand) GetMatcher() matcher.Matcher {
return matcher.NewTextMatcher("list branches", c.listBranches)
}

func (c *vcsCommand) listBranches(match matcher.Result, message msg.Message) {
func (c *vcsCommand) listBranches(_ matcher.Result, message msg.Message) {
branches := vcs.GetBranches()
slices.Sort(branches)

Expand Down
3 changes: 3 additions & 0 deletions command/vcs/vcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/stretchr/testify/assert"
)

// compile time check that the interface matches
var _ bot.Runnable = &vcsCommand{}

func TestVCS(t *testing.T) {
slackClient := &mocks.SlackClient{}
base := bot.BaseCommand{SlackClient: slackClient}
Expand Down
18 changes: 8 additions & 10 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,25 @@ go 1.20
require (
github.com/alicebob/miniredis/v2 v2.30.5
github.com/andygrunwald/go-jira v1.16.0
github.com/aws/aws-sdk-go v1.45.5
github.com/aws/aws-sdk-go v1.45.26
github.com/bndr/gojenkins v1.1.0
github.com/brainexe/viper v1.8.2
github.com/gfleury/go-bitbucket-v1 v0.0.0-20230830121038-6e30c5760c87
github.com/google/go-github v17.0.0+incompatible
github.com/gookit/color v1.5.4
github.com/hackebrot/turtle v0.2.0
github.com/pkg/errors v0.9.1
github.com/redis/go-redis/v9 v9.1.0
github.com/redis/go-redis/v9 v9.2.1
github.com/rifflock/lfshook v0.0.0-20180920164130-b9218ef580f5
github.com/robfig/cron/v3 v3.0.1
github.com/sdomino/golang-scribble v0.0.0-20230717151034-b95d4df19aa8
github.com/sirupsen/logrus v1.9.3
github.com/slack-go/slack v0.12.3
github.com/stretchr/testify v1.8.4
github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c
github.com/tiktoken-go/tokenizer v0.1.0
github.com/xanzy/go-gitlab v0.93.0
golang.org/x/exp v0.0.0-20230905200255-921286631fa9
golang.org/x/oauth2 v0.12.0
github.com/xanzy/go-gitlab v0.93.1
golang.org/x/exp v0.0.0-20231006140011-7918f672742d
golang.org/x/oauth2 v0.13.0
golang.org/x/text v0.13.0
gopkg.in/yaml.v3 v3.0.1
)
Expand All @@ -34,7 +33,6 @@ require (
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/dlclark/regexp2 v1.9.0 // indirect
github.com/fatih/structs v1.1.0 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
Expand All @@ -51,7 +49,7 @@ require (
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/spf13/afero v1.9.5 // indirect
github.com/spf13/afero v1.10.0 // indirect
github.com/spf13/cast v1.5.1 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
Expand All @@ -61,8 +59,8 @@ require (
github.com/trivago/tgo v1.0.7 // indirect
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
github.com/yuin/gopher-lua v1.1.0 // indirect
golang.org/x/net v0.15.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/net v0.17.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/time v0.3.0 // indirect
google.golang.org/appengine v1.6.8 // indirect
google.golang.org/protobuf v1.31.0 // indirect
Expand Down
Loading

0 comments on commit f29110e

Please sign in to comment.