Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-write the RetryPolicy unit tests to remove strong coupling with implementation detail which are marked as virtual for mocking #5585

Open
ahsonkhan opened this issue Apr 29, 2024 · 0 comments

Comments

@ahsonkhan
Copy link
Member

ahsonkhan commented Apr 29, 2024

Follow-up from #5584
Currently the RetryPolicy has these two methods which are part of its implementation detail and being used in unit tests:

  • ShouldRetryOnResponse
  • ShouldRetryOnTransportFailure

protected:
virtual bool ShouldRetryOnTransportFailure(
RetryOptions const& retryOptions,
int32_t attempt,
std::chrono::milliseconds& retryAfter,
double jitterFactor = -1) const;
virtual bool ShouldRetryOnResponse(
RawResponse const& response,
RetryOptions const& retryOptions,
int32_t attempt,
std::chrono::milliseconds& retryAfter,
double jitterFactor = -1) const;

These methods don't need to be virtual nor are they part of the public surface area. However, the way the tests are written, they take a dependency on this implementation detail. Additionally, because the unit tests have them mocked, they have to be marked as virtual, which they shouldn't be.

protected:
bool ShouldRetryOnTransportFailure(
RetryOptions const& retryOptions,
int32_t attempt,
std::chrono::milliseconds& retryAfter,
double jitterFactor) const override
{
return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor);
}
bool ShouldRetryOnResponse(
RawResponse const& response,
RetryOptions const& retryOptions,
int32_t attempt,
std::chrono::milliseconds& retryAfter,
double jitterFactor) const override
{
return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor);
}
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant