Skip to content

Commit

Permalink
Fix ApiUpdated test to properly run its reload
Browse files Browse the repository at this point in the history
On one hand it was running a reload manually, which was missing the
point of the test. We want the redis event to fire it on its own.

On the other, not sending a reload tick would mean that if the event
indeed fires a reload, it would be queued but not executed, breaking
other tests that depend on an empty reload queue.
  • Loading branch information
mvdan committed Mar 22, 2017
1 parent d30e1d4 commit 9b74711
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
10 changes: 8 additions & 2 deletions api_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"sync"
"testing"
"time"

"github.com/garyburd/redigo/redis"
"github.com/lonelycode/gorpc"
Expand Down Expand Up @@ -491,16 +493,20 @@ func TestGetAPISpecsDashboardSuccess(t *testing.T) {
config.DBAppConfOptions.ConnectionString = ""
}()

var wg sync.WaitGroup
wg.Add(1)
msg := redis.Message{Data: []byte(`{"Command": "ApiUpdated"}`)}
handleRedisEvent(msg)
handleRedisEvent(msg, wg.Done)

if len(reloadChan) != 1 {
t.Fatal("Should trigger reload")
}

// Since we already know that reload is queued
doReload()
reloadTick <- time.Time{}

// Wait for the reload to finish, then check it worked
wg.Wait()
if len(ApiSpecRegister) != 1 {
t.Error("Should return array with one spec", ApiSpecRegister)
}
Expand Down
18 changes: 8 additions & 10 deletions redis_signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ func startPubSubLoop() {
cacheStore.Connect()
// On message, synchronise
for {
err := cacheStore.StartPubSubHandler(RedisPubSubChannel, handleRedisEvent)
err := cacheStore.StartPubSubHandler(RedisPubSubChannel, func(v interface{}) {
handleRedisEvent(v, nil)
})
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "pub-sub",
Expand All @@ -36,7 +38,7 @@ func startPubSubLoop() {
}
}

func handleRedisEvent(v interface{}) {
func handleRedisEvent(v interface{}, reloadFn func()) {
message, ok := v.(redis.Message)
if !ok {
return
Expand Down Expand Up @@ -73,7 +75,10 @@ func handleRedisEvent(v interface{}) {
case NoticeGatewayLENotification:
onLESSLStatusReceivedHandler(notif.Payload)
case NoticeApiUpdated, NoticeApiRemoved, NoticeApiAdded, NoticePolicyChanged, NoticeGroupReload:
handleReloadMsg()
log.WithFields(logrus.Fields{
"prefix": "pub-sub",
}).Info("Reloading endpoints")
reloadURLStructure(reloadFn)
default:
log.WithFields(logrus.Fields{
"prefix": "pub-sub",
Expand All @@ -82,13 +87,6 @@ func handleRedisEvent(v interface{}) {

}

func handleReloadMsg() {
log.WithFields(logrus.Fields{
"prefix": "pub-sub",
}).Info("Reloading endpoints")
reloadURLStructure(nil)
}

var warnedOnce bool
var notificationVerifier goverify.Verifier

Expand Down

0 comments on commit 9b74711

Please sign in to comment.