Improve event and state assertions #1261
Conversation
This order is more natural because we an ID is a "pointer" to an entity. Also, we're going to have more assertion methods that would accept an instance of an entity state. Creation of an entity state (which is a Protobuf message) is a big block of code. It would be more readable if the ID parameter comes before such a block. So, to make the order consistent for all assertion methods, we're making ID coming first for all.
Also: * Add nullity checks.
Codecov Report
@@ Coverage Diff @@
## master #1261 +/- ##
============================================
+ Coverage 91.06% 91.08% +0.01%
- Complexity 4655 4658 +3
============================================
Files 598 598
Lines 14734 14767 +33
Branches 841 841
============================================
+ Hits 13417 13450 +33
Misses 1057 1057
Partials 260 260 |
| * | ||
| * <p>The method compares only fields in the passed state. | ||
| */ | ||
| public final <I, S extends EntityState> void assertState(I id, S entityState) { |
There was a problem hiding this comment.
| public final <I, S extends EntityState> void assertState(I id, S entityState) { | |
| public final void assertStateEquals(Object id, EntityState entityState) { |
| * @return assertion that compares only expected fields | ||
| */ | ||
| public final <I, S extends EntityState> ProtoFluentAssertion | ||
| assertState(I id, Class<S> stateClass) { |
There was a problem hiding this comment.
I suggest we get rid of unnecessary type parameters. In this case, since neither <I> nor <S> is used in the return type, they both are unnecessary.
Also, there used to be a style check for that. Is it off now?
There was a problem hiding this comment.
Indeed, they are not used. But they are consistent with the rest of assertion methods. Here having <I> as a generic parameter we hint on the fact that an object which can be passed to it can be of one of the identifier types, not any Object.
| * Obtains a Subject for an entity of the passed class with the given ID. | ||
| */ | ||
| public final <I, E extends Entity<I, ? extends EntityState>> | ||
| EntitySubject assertEntity(I id, Class<E> entityClass) { |
There was a problem hiding this comment.
Why do we change the order of parameters? One could argue that specifying an entity type first and then its ID is more natural from the user's perspective.
There was a problem hiding this comment.
I mentioned this in one of the commit comments. Here's why. 1) When we assert against an entity state, it's a big block of code. Having the ID parameter below this block makes it poorly visible.
- A reference to an object usually comes first in a procedure that deals with an object. We use IDs as reference to entities.
This PR adds assertion methods for events and entity states to
BlackBoxContext.Entity and entity state assertion methods now accept an ID as the first parameter. Methods that used to have the ID as the second parameter were deprecated in favour of ones with the new order.