Improve dependencies management#159
Conversation
…actions done with a response.
Codecov Report
@@ Coverage Diff @@
## master #159 +/- ##
============================================
- Coverage 61.98% 60.04% -1.94%
- Complexity 178 210 +32
============================================
Files 88 93 +5
Lines 2191 2370 +179
Branches 40 43 +3
============================================
+ Hits 1358 1423 +65
- Misses 824 936 +112
- Partials 9 11 +2 |
armiol
left a comment
armiol
left a comment
There was a problem hiding this comment.
@yuri-sergiichuk please see my comments.
| * <p>Supports having a custom write latency through {@linkplain #withSimulatedLatency(Duration) | ||
| * setting} a particular write operations duration. | ||
| */ | ||
| public final class MemoizedFirebase implements FirebaseClient { |
There was a problem hiding this comment.
This should probably be MemoizingFirebase.
| * Returns a {@code NodeValue} written to a specific {@code path}. | ||
| * | ||
| * @throws IllegalStateException | ||
| * if no value is present for the path. |
There was a problem hiding this comment.
It looks unformatted to me.
Also, we don't put periods in descriptions of parameters and exceptions.
There was a problem hiding this comment.
That's how the @throws block is formatted.
| * <p>The node has a single randomized field with the field value being a serialized | ||
| * processed message. | ||
| * | ||
| * @implNote the {@code nodeValue} holds data as a JSON primitive string (i.e. an |
There was a problem hiding this comment.
Please start this the with the capital letter.
| * Creates an {@code HttpClient} mock which returns the specified {@code content} | ||
| * on every request and uses supplied {@code observer} while building requests. | ||
| */ | ||
| static HttpClient mockHttpClient(String content, RequestObserver observer) { |
There was a problem hiding this comment.
I don't understand neither the naming nor the purpose of RequestObserver. Could you please elaborate on this so that this API piece looked a bit more clear to a naked eye?
| /** | ||
| * Observes built HTTP requests. | ||
| */ | ||
| interface RequestObserver { |
There was a problem hiding this comment.
This name confuses me as well as the API which uses it.
If I'd wake somebody up at 3 AM and ask what a RequestObserver does, they would hardly answer correctly.
| import static java.util.Collections.emptyIterator; | ||
|
|
||
| /** | ||
| * A mocked servlet request with pre-defined {@code content}, {@code type} and {@code headers}. |
There was a problem hiding this comment.
Isn't any servlet request a "fixed content" request? I mean, each of servlet requests does have a content, type and headers which never-ever are changed.
It's either about a description that I understand incorrectly, or about the name for this type which is confusing if compared to a description.
| * | ||
| * <p>In most cases this implementation should be sufficient enough for local tests. | ||
| */ | ||
| public final class FixedContentResponse implements MockedResponse { |
There was a problem hiding this comment.
Same as above.
Can't we just simplify the whole naming paradigm to KnownRequest / KnownResponse or GivenRequest / GivenResponse, or something?
| /** | ||
| * A mocked response whose {@code status} and {@code type} could be set. | ||
| */ | ||
| public final class SettableResponse implements MockedResponse { |
There was a problem hiding this comment.
Isn't it MutableResponse?
I also don't really understand why we divide the mock types into fixed first and second and fixed third. Isn't it simpler to have just one mock type?
|
@armiol PTAL again. |
2 participants
This PR improves our Gradle dependencies management a bit and applies the new scope to the dependencies that are used by the code analysis software only.
As part of the PR, I have also eliminated the usage of the
hamcrestassertion framework in favor oftruth.mockitousage is replaced with hand-crafted mocks.The change is related to the SpineEventEngine/config#156 issue.