From 8244351bb58c431f3834592e8915b7451fd4e8ea Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 19 Apr 2019 01:41:52 +0300 Subject: [PATCH] #1: Fix race conditions errors and rewrite tests --- Gopkg.lock | 17 +- api/mojang/queue/jobs_structure.go | 9 +- api/mojang/queue/queue.go | 11 +- api/mojang/queue/queue_test.go | 344 ++++++++++++++--------------- 4 files changed, 191 insertions(+), 190 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 2acaa88..72ac25f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -226,10 +226,23 @@ revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" version = "v1.0.0" +[[projects]] + digest = "1:711eebe744c0151a9d09af2315f0bb729b2ec7637ef4c410fa90a18ef74b65b6" + name = "github.com/stretchr/objx" + packages = ["."] + pruneopts = "" + revision = "477a77ecc69700c7cdeb1fa9e129548e1c1c393c" + version = "v0.1.1" + [[projects]] digest = "1:381bcbeb112a51493d9d998bbba207a529c73dbb49b3fd789e48c63fac1f192c" name = "github.com/stretchr/testify" - packages = ["assert"] + packages = [ + "assert", + "mock", + "require", + "suite", + ] pruneopts = "" revision = "ffdc059bfe9ce6a4e144ba849dbedead332c6053" version = "v1.3.0" @@ -303,6 +316,8 @@ "github.com/spf13/cobra", "github.com/spf13/viper", "github.com/stretchr/testify/assert", + "github.com/stretchr/testify/mock", + "github.com/stretchr/testify/suite", "github.com/thedevsaddam/govalidator", "gopkg.in/h2non/gock.v1", ] diff --git a/api/mojang/queue/jobs_structure.go b/api/mojang/queue/jobs_structure.go index 464e330..02d624d 100644 --- a/api/mojang/queue/jobs_structure.go +++ b/api/mojang/queue/jobs_structure.go @@ -25,24 +25,29 @@ func (s *jobsQueue) New() *jobsQueue { func (s *jobsQueue) Enqueue(t *jobItem) { s.lock.Lock() + defer s.lock.Unlock() + s.items = append(s.items, t) - s.lock.Unlock() } func (s *jobsQueue) Dequeue(n int) []*jobItem { s.lock.Lock() + defer s.lock.Unlock() + if n > s.Size() { n = s.Size() } items := s.items[0:n] s.items = s.items[n:len(s.items)] - s.lock.Unlock() return items } func (s *jobsQueue) IsEmpty() bool { + s.lock.Lock() + defer s.lock.Unlock() + return len(s.items) == 0 } diff --git a/api/mojang/queue/queue.go b/api/mojang/queue/queue.go index ad517cc..2a330cf 100644 --- a/api/mojang/queue/queue.go +++ b/api/mojang/queue/queue.go @@ -11,6 +11,9 @@ import ( var usernamesToUuids = mojang.UsernamesToUuids var uuidToTextures = mojang.UuidToTextures var delay = time.Second +var forever = func() bool { + return true +} type JobsQueue struct { Storage Storage @@ -19,7 +22,7 @@ type JobsQueue struct { queue jobsQueue } -func (ctx *JobsQueue) GetTexturesForUsername(username string) *mojang.SignedTexturesResponse { +func (ctx *JobsQueue) GetTexturesForUsername(username string) chan *mojang.SignedTexturesResponse { ctx.onFirstCall.Do(func() { ctx.queue.New() ctx.startQueue() @@ -28,14 +31,15 @@ func (ctx *JobsQueue) GetTexturesForUsername(username string) *mojang.SignedText resultChan := make(chan *mojang.SignedTexturesResponse) // TODO: prevent of adding the same username more than once ctx.queue.Enqueue(&jobItem{username, resultChan}) + // TODO: return nil if processing takes more than 5 seconds - return <-resultChan + return resultChan } func (ctx *JobsQueue) startQueue() { go func() { time.Sleep(delay) - for true { + for forever() { start := time.Now() ctx.queueRound() time.Sleep(delay - time.Since(start)) @@ -81,6 +85,7 @@ func (ctx *JobsQueue) queueRound() { } if uuid != "" { + var err error result, err = uuidToTextures(uuid, true) if err != nil { if _, ok := err.(*mojang.TooManyRequestsError); !ok { diff --git a/api/mojang/queue/queue_test.go b/api/mojang/queue/queue_test.go index 969c98f..d53b800 100644 --- a/api/mojang/queue/queue_test.go +++ b/api/mojang/queue/queue_test.go @@ -3,211 +3,187 @@ package queue import ( "crypto/rand" "encoding/base64" - "errors" - "log" - "testing" - "time" "github.com/elyby/chrly/api/mojang" - testify "github.com/stretchr/testify/assert" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "testing" ) -func TestJobsQueue_GetTexturesForUsername(t *testing.T) { - delay = 50 * time.Millisecond - - t.Run("receive textures for one username", func(t *testing.T) { - assert := testify.New(t) - - usernamesToUuids = createUsernameToUuidsMock( - assert, - []string{"maksimkurb"}, - []*mojang.ProfileInfo{ - {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, - }, - nil, - ) - uuidToTextures = createUuidToTextures([]*createUuidToTexturesResult{ - createTexturesResult("0d252b7218b648bfb86c2ae476954d32", "maksimkurb"), - }) - - queue := &JobsQueue{Storage: &NilStorage{}} - result := queue.GetTexturesForUsername("maksimkurb") - - if assert.NotNil(result) { - assert.Equal("0d252b7218b648bfb86c2ae476954d32", result.Id) - assert.Equal("maksimkurb", result.Name) - } - }) - - t.Run("receive textures for few usernames", func(t *testing.T) { - assert := testify.New(t) - - usernamesToUuids = createUsernameToUuidsMock( - assert, - []string{"maksimkurb", "Thinkofdeath"}, - []*mojang.ProfileInfo{ - {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, - {Id: "4566e69fc90748ee8d71d7ba5aa00d20", Name: "Thinkofdeath"}, - }, - nil, - ) - uuidToTextures = createUuidToTextures([]*createUuidToTexturesResult{ - createTexturesResult("0d252b7218b648bfb86c2ae476954d32", "maksimkurb"), - createTexturesResult("4566e69fc90748ee8d71d7ba5aa00d20", "Thinkofdeath"), - }) - - queue := &JobsQueue{Storage: &NilStorage{}} - resultChan1 := make(chan *mojang.SignedTexturesResponse) - resultChan2 := make(chan *mojang.SignedTexturesResponse) - go func() { - resultChan1 <- queue.GetTexturesForUsername("maksimkurb") - }() - go func() { - resultChan2 <- queue.GetTexturesForUsername("Thinkofdeath") - }() - - assert.NotNil(<-resultChan1) - assert.NotNil(<-resultChan2) - }) - - t.Run("query no more than 100 usernames and all left on the next iteration", func(t *testing.T) { - assert := testify.New(t) - - usernames := make([]string, 120, 120) - for i := 0; i < 120; i++ { - usernames[i] = randStr(8) - } - - usernamesToUuids = createUsernameToUuidsMock(assert, usernames[0:100], []*mojang.ProfileInfo{}, nil) - - queue := &JobsQueue{Storage: &NilStorage{}} - - scheduleUsername := func(username string) { - queue.GetTexturesForUsername(username) - } - - for _, username := range usernames { - go scheduleUsername(username) - time.Sleep(50 * time.Microsecond) // Add delay to have consistent order - } - - // Let it begin first iteration - time.Sleep(delay + delay/2) - - usernamesToUuids = createUsernameToUuidsMock( - assert, - usernames[100:120], - []*mojang.ProfileInfo{}, - nil, - ) - - time.Sleep(delay) - }) - - t.Run("should do nothing if queue is empty", func(t *testing.T) { - assert := testify.New(t) - - usernamesToUuids = createUsernameToUuidsMock(assert, []string{"maksimkurb"}, []*mojang.ProfileInfo{}, nil) - uuidToTextures = func(uuid string, signed bool) (*mojang.SignedTexturesResponse, error) { - t.Error("this method shouldn't be called") - return nil, nil - } - - // Perform first iteration and await it finish - queue := &JobsQueue{Storage: &NilStorage{}} - result := queue.GetTexturesForUsername("maksimkurb") - assert.Nil(result) - - // Override external API call that indicates, that queue is still trying to obtain somethid - usernamesToUuids = func(usernames []string) ([]*mojang.ProfileInfo, error) { - t.Error("this method shouldn't be called") - return nil, nil - } - - // Let it to iterate few times - time.Sleep(delay * 2) - }) - - t.Run("handle 429 error when exchanging usernames to uuids", func(t *testing.T) { - assert := testify.New(t) - - usernamesToUuids = createUsernameToUuidsMock(assert, []string{"maksimkurb"}, nil, &mojang.TooManyRequestsError{}) - - queue := &JobsQueue{Storage: &NilStorage{}} - result := queue.GetTexturesForUsername("maksimkurb") - assert.Nil(result) - }) - - t.Run("handle 429 error when requesting user's textures", func(t *testing.T) { - assert := testify.New(t) - - usernamesToUuids = createUsernameToUuidsMock( - assert, - []string{"maksimkurb"}, - []*mojang.ProfileInfo{ - {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, - }, - nil, - ) - uuidToTextures = createUuidToTextures([]*createUuidToTexturesResult{ - createTexturesResult("0d252b7218b648bfb86c2ae476954d32", &mojang.TooManyRequestsError{}), - }) - - queue := &JobsQueue{Storage: &NilStorage{}} - result := queue.GetTexturesForUsername("maksimkurb") - assert.Nil(result) - }) +type MojangApiMocks struct { + mock.Mock } -func createUsernameToUuidsMock( - assert *testify.Assertions, - expectedUsernames []string, - result []*mojang.ProfileInfo, - err error, -) func(usernames []string) ([]*mojang.ProfileInfo, error) { - return func(usernames []string) ([]*mojang.ProfileInfo, error) { - assert.ElementsMatch(expectedUsernames, usernames) - return result, err +func (o *MojangApiMocks) UsernameToUuids(usernames []string) ([]*mojang.ProfileInfo, error) { + args := o.Called(usernames) + var result []*mojang.ProfileInfo + if casted, ok := args.Get(0).([]*mojang.ProfileInfo); ok { + result = casted + } + + return result, args.Error(1) +} + +func (o *MojangApiMocks) UuidToTextures(uuid string, signed bool) (*mojang.SignedTexturesResponse, error) { + args := o.Called(uuid, signed) + var result *mojang.SignedTexturesResponse + if casted, ok := args.Get(0).(*mojang.SignedTexturesResponse); ok { + result = casted + } + + return result, args.Error(1) +} + +type QueueTestSuite struct { + suite.Suite + Queue *JobsQueue + MojangApi *MojangApiMocks + Iterate func() + + iterateChan chan bool + done func() +} + +func (suite *QueueTestSuite) SetupSuite() { + delay = 0 +} + +func (suite *QueueTestSuite) SetupTest() { + suite.Queue = &JobsQueue{} + + suite.iterateChan = make(chan bool) + forever = func() bool { + return <-suite.iterateChan + } + + suite.Iterate = func() { + suite.iterateChan <- true + } + + suite.done = func() { + suite.iterateChan <- false + } + + suite.MojangApi = new(MojangApiMocks) + usernamesToUuids = suite.MojangApi.UsernameToUuids + uuidToTextures = suite.MojangApi.UuidToTextures +} + +func (suite *QueueTestSuite) TearDownTest() { + suite.done() + suite.MojangApi.AssertExpectations(suite.T()) +} + +func (suite *QueueTestSuite) TestReceiveTexturesForOneUsername() { + suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ + {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, + }, nil) + suite.MojangApi.On("UuidToTextures", "0d252b7218b648bfb86c2ae476954d32", true).Once().Return( + &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, + nil, + ) + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + suite.Iterate() + + result := <-resultChan + if suite.Assert().NotNil(result) { + suite.Assert().Equal("0d252b7218b648bfb86c2ae476954d32", result.Id) + suite.Assert().Equal("maksimkurb", result.Name) } } -type createUuidToTexturesResult struct { - uuid string - result *mojang.SignedTexturesResponse - err error +func (suite *QueueTestSuite) TestReceiveTexturesForFewUsernames() { + suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb", "Thinkofdeath"}).Once().Return([]*mojang.ProfileInfo{ + {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, + {Id: "4566e69fc90748ee8d71d7ba5aa00d20", Name: "Thinkofdeath"}, + }, nil) + suite.MojangApi.On("UuidToTextures", "0d252b7218b648bfb86c2ae476954d32", true).Once().Return( + &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, + nil, + ) + suite.MojangApi.On("UuidToTextures", "4566e69fc90748ee8d71d7ba5aa00d20", true).Once().Return( + &mojang.SignedTexturesResponse{Id: "4566e69fc90748ee8d71d7ba5aa00d20", Name: "Thinkofdeath"}, + nil, + ) + + resultChan1 := suite.Queue.GetTexturesForUsername("maksimkurb") + resultChan2 := suite.Queue.GetTexturesForUsername("Thinkofdeath") + + suite.Iterate() + + suite.Assert().NotNil(<-resultChan1) + suite.Assert().NotNil(<-resultChan2) } -func createTexturesResult(uuid string, result interface{}) *createUuidToTexturesResult { - output := &createUuidToTexturesResult{uuid: uuid} - if username, ok := result.(string); ok { - output.result = &mojang.SignedTexturesResponse{Id: uuid, Name: username} - } else if err, ok := result.(error); ok { - output.err = err - } else { - log.Fatal("invalid result type passed") +func (suite *QueueTestSuite) TestReceiveTexturesForMoreThan100Usernames() { + usernames := make([]string, 120, 120) + for i := 0; i < 120; i++ { + usernames[i] = randStr(8) } - return output + suite.MojangApi.On("UsernameToUuids", usernames[0:100]).Once().Return([]*mojang.ProfileInfo{}, nil) + suite.MojangApi.On("UsernameToUuids", usernames[100:120]).Once().Return([]*mojang.ProfileInfo{}, nil) + + for _, username := range usernames { + suite.Queue.GetTexturesForUsername(username) + } + + suite.Iterate() + suite.Iterate() } -func createUuidToTextures( - results []*createUuidToTexturesResult, -) func(uuid string, signed bool) (*mojang.SignedTexturesResponse, error) { - return func(uuid string, signed bool) (*mojang.SignedTexturesResponse, error) { - for _, result := range results { - if result.uuid == uuid { - return result.result, result.err - } - } +func (suite *QueueTestSuite) TestDoNothingWhenNoTasks() { + suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{}, nil) - return nil, errors.New("cannot find corresponding result") - } + // Perform first iteration and await it finish + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + suite.Iterate() + + suite.Assert().Nil(<-resultChan) + + // Let it to perform a few more iterations to ensure, that there is no calls to external APIs + suite.Iterate() + suite.Iterate() +} + +func (suite *QueueTestSuite) TestHandle429ResponseWhenExchangingUsernamesToUuids() { + suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return(nil, &mojang.TooManyRequestsError{}) + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + suite.Iterate() + + suite.Assert().Nil(<-resultChan) +} + +func (suite *QueueTestSuite) TestHandle429ResponseWhenRequestingUsersTextures() { + suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ + {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, + }, nil) + suite.MojangApi.On("UuidToTextures", "0d252b7218b648bfb86c2ae476954d32", true).Once().Return( + nil, + &mojang.TooManyRequestsError{}, + ) + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + suite.Iterate() + + suite.Assert().Nil(<-resultChan) +} + +func TestJobsQueueSuite(t *testing.T) { + suite.Run(t, new(QueueTestSuite)) } // https://stackoverflow.com/a/50581165 func randStr(len int) string { buff := make([]byte, len) - rand.Read(buff) + _, _ = rand.Read(buff) str := base64.StdEncoding.EncodeToString(buff) // Base 64 can be longer than len