feat(Polly): simplify ternary unit tests #223
Conversation
|
Hi @av-linasuzuki could you review your commit messages and follow the current standard? It's not documented in the contribution guide (that's my failure) but nonetheless there's a standard. For practical examples, see the commit history of the project. For the syntax itself, check here. If you need help changing previous commit messages, take a look here. If you need help, get in touch with me directly. Thanks! |
av-linasuzuki
force-pushed
the
feature/Simplify-Ternary
branch
from
6767f8f to
0e8a427
Compare
ying-feifan-avanade
force-pushed
the
feature/Simplify-Ternary
branch
from
0e8a427 to
432c761
Compare
av-linasuzuki
force-pushed
the
feature/Simplify-Ternary
branch
from
432c761 to
e1cc147
Compare
|
@av-linasuzuki @ying-feifan-avanade please include in the PR description a command to close the issue, as described here: https://help.github.com/en/enterprise/2.16/user/github/managing-your-work-on-github/closing-issues-using-keywords (You can edit the description of the issue by clicking the ellipsis "..." on the top-right side of the textbox) |
There was a problem hiding this comment.
I'm pointing out a few minor changes, but there are some important things to note:
-
Tests depend on external conditions, for instance, that URL https://www.aipdhapsidh.com be invalid. If, for instance, I were to register said URL now, our test would fail.
-
The tests don't actually tests the declared conditions. For instance,
ResilientRequestWhenInvalidUrlAfterRetriedConfiguredAttempsThrowHttpRequestexplicitly states that the throw is made after configured attempts. However, we don't check how many times the request was made and, therefore, can't know for sure if polly is doing what it is expected. Same can be said of the other tests.
One way around those two issues is to start a mock server which you will call using polly, as I did while testing LiquidApi. You can check on the mock side if n calls where made, and that would be enough.
Testing the backOffTime is a bit more hard, since the time informed and the time measured will conflict, but you could try measuring with a very small value and then with a large value, which would prove that a delay was applied.
| [Fact] | ||
| public async Task ResilientRequestShouldUseDefaultPollyConfigAsRetryAttemptWhenIsBackOffFalse() | ||
| { | ||
| //Arrange | ||
| PollyConfiguration _pollyConfiguration = new PollyConfiguration { IsBackOff = false, Retry = 3, Wait = 5 }; | ||
| var url = "https://www.google.com"; | ||
| //Action | ||
| using (HttpResponseMessage result = await _sut.ResilientRequest(url, _httpClient, _http, _pollyConfiguration).ConfigureAwait(false)) | ||
| { | ||
| //Assert | ||
| Assert.True(result.IsSuccessStatusCode); | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you please tell me the difference between this test and the previous one? They seem to be exactly the same.
There was a problem hiding this comment.
We tried to test for a false return but failed.
Can we leave without this test?
issue #113
simplify ternary in
Pollyand created the unit test about this class