Fix race in ClientMock.MockGet()

[pracucci]

What this PR does:
Recently we started seeing some compactor unit tests failing because of a race. The race is caused by the mocked Get() function on the bucket.ClientMock. This PR fixes it.

The problem is that testify (the testing lib we're also using for mocking) doesn't allow to specify a dynamic return value (eg. a function that's called each time to get the return value for the mocked function) and so to mock the returned io.ReadCloser we use the following trick:

		// Since we return an ReadCloser and it can be consumed only once,
		// each time the mocked Get() is called we do create a new one, so
		// that getting the same mocked object twice works as expected.
		mockedGet := m.On("Get", mock.Anything, name)
		mockedGet.Run(func(args mock.Arguments) {
			mockedGet.Return(ioutil.NopCloser(bytes.NewReader([]byte(content))), err)
		})

However, there's a race condition that could get mockedGet to return the same reader twice and this cause some unit tests to fail from time to time.

To fix it, I've used a smart solution outlined by @colega (thanks!).

Which issue(s) this PR fixes:
N/A

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[pstibrany]

Nice, looks good.

[bboreham]

Should this go after the error check?
And do we still need the case where val is an io.ReadCloser ?

[pracucci]

Should this go after the error check?

I don't think so.

And do we still need the case where val is an io.ReadCloser ?

Yes. After this PR there are 2 ways to mock Get(): (1) mock the returned arguments with static values (2) mock the returned arguments with a function which is called each time Get() is called. This PR adds (2), while we should still support (1).