在Golang中测试一次条件

我有一个依赖于通过调用外部服务来填充缓存的模块,如下所示:

func (provider *Cache) GetItem(productId string, skuId string, itemType string) (*Item, error) {

    // First, create the key we'll use to uniquely identify the item
    key := fmt.Sprintf("%s:%s", productId, skuId)

    // Now, attempt to get the concurrency control associated with the item key
    // If we couldn't find it then create one and add it to the map
    var once *sync.Once
    if entry, ok := provider.lockMap.Load(key); ok {
        once = entry.(*sync.Once)
    } else {
        once = &sync.Once{}
        provider.lockMap.Store(key, once)
    }

    // Now, use the concurrency control to attempt to request the item
    // but only once. Channel any errors that occur
    cErr := make(chan error, 1)
    once.Do(func() {

        // We didn't find the item in the cache so we'll have to get it from the partner-center
        item, err := provider.client.GetItem(productId, skuId)
        if err != nil {
            cErr <- err
            return
        }

        // Add the item to the cache
        provider.cache.Store(key, &item)
    })

    // Attempt to read an error from the channel; if we get one then return it
    // Otherwise, pull the item out of the cache. We have to use the select here because this is
    // the only way to attempt to read from a channel without it blocking
    var sku interface{}
    select {
    case err, ok := <-cErr:
        if ok {
            return nil, err
        }
    default:
        item, _ = provider.cache.Load(key)
    }

    // Now, pull out a reference to the item and return it
    return item.(*Item), nil
}

This method works as I expect it to. My problem is testing; specifically testing to ensure that the GetItem method is called only once for a given value of key. My test code is below:

var _ = Describe("Item Tests", func() {

    It("GetItem - Not cached, two concurrent requests - Client called once", func() {

        // setup cache

        // Setup a wait group so we can ensure both processes finish
        var wg sync.WaitGroup
        wg.Add(2)

        // Fire off two concurrent requests for the same SKU
        go runRequest(&wg, cache)
        go runRequest(&wg, cache)
        wg.Wait()

        // Check the cache; it should have one value
        _, ok := cache.cache.Load("PID:SKUID")
        Expect(ok).Should(BeTrue())

        // The client should have only been requested once
        Expect(client.RequestCount).Should(Equal(1)) // FAILS HERE
    })
})

// Used for testing concurrency
func runRequest(wg *sync.WaitGroup, cache *SkuCache) {
    defer wg.Done()
    sku, err := cache.GetItem("PID", "SKUID", "fakeitem")
    Expect(err).ShouldNot(HaveOccurred())
}

type mockClient struct {
    RequestFails    bool
    RequestCount    int
    lock            sync.Mutex
}

func NewMockClient(requestFails bool) *mockClient {
    return &mockClient{
        RequestFails:    requestFails,
        RequestCount:    0,
        lock:            sync.Mutex{},
    }
}

func (client *mockClient) GetItem(productId string, skuId string) (item Item, err error) {
    defer GinkgoRecover()

    // If we want to simulate client failure then return an error here
    if client.RequestFails {
        err = fmt.Errorf("GetItem failed")
        return
    }

    // Sleep for 100ms so we can more accurately simulate the request latency
    time.Sleep(100 * time.Millisecond)

    // Update the request count
    client.lock.Lock()
    client.RequestCount++
    client.lock.Unlock()

    item = Item{
        Id:              skuId,
        ProductId:       productId,
    }

    return
}

我一直遇到的问题是该测试有时会失败,因为在注释行中,当请求计数为1时,请求计数为2。此故障并不一致,因此我不确定如何调试此问题。任何帮助将不胜感激。

评论
  • 22相亲
    22相亲 回复

    我认为您的测试有时会失败,因为您的缓存无法提供保证它只能提取项目一次的方法,因此您很幸运测试抓住了这一点。

    If an item is not in it, and 2 concurrent goroutines call Cache.GetItem() at the same time, it may happen that lockMap.Load() will report in both that the key is not in the map, both goroutines create a sync.Once, and both will store their own instance in the map (obviously only one–the latter–will remain in the map, but your cache does not check this).

    Then the 2 goroutines both will call client.GetItem() because 2 separate sync.Once provides no synchronization. Only if the same sync.Once instance is used, only then there is guarantee that the function passed to Once.Do() is executed only once.

    I think a sync.Mutex would be easier and more appropriate to avoid creating and using 2 sync.Once here.

    Or since you're already using sync.Map, you may use the Map.LoadOrStore() method: create a sync.Once, and pass that to Map.LoadOrStore(). If the key is already in the map, use the returned sync.Once. If the key is not in the map, your sync.Once will be stored in it and so you can use that. This will ensure no multiple concurrent goroutines can store multiple sync.once instances in it.

    像这样:

    var once *sync.Once
    if entry, loaded := provider.lockMap.LoadOrStore(key, once); loaded {
        // Was already in the map, use the loaded Once
        once = entry.(*sync.Once)
    }