exonum-java-binding
exonum-java-binding copied to clipboard
exonum
Evaluate Truth assertion framework
Overview
The PR is better viewed commit-by-commit. It starts with a singe test migrated to Truth, adds a minimal Subject (~ Hamcrest Matcher), then extends that subject with a new operation, and finally adds some assertions on common library types like Strings and Throwables and various collections.
See:
- https://jira.bf.local/browse/ECR-3339
- https://github.com/google/truth/issues/612
Definition of Done
- [ ] There are no TODOs left in the code
- [ ] Change is covered by automated tests
- [ ] The coding guidelines are followed
- [ ] Public API has Javadoc
- [ ] Method preconditions are checked and documented in the Javadoc of the method
- [ ] Changelog is updated if needed (in case of notable or breaking changes)
- [ ] The continuous integration build passes
Looks nice. Will it bring benefits for testing our collections? For example, is it hard to make such assertions (or similar):
assertThat(listIndexProxy).isEmpty();
assertThat(listIndexProxy).hasSize(1);
assertThat(listIndexProxy).containsExactly("one", "two");
Thanks for the review,
Will it bring benefits for testing our collections?
The good thing is, IterableSubject is already applicable to our List and Set indexes, and it is non-final, therefore, can be extended to add some specific assertions (proofs, hashes, etc.). The assertions you've seen in the PR for Lists use IterableSubject (= there is no ListSubject in Truth).
Regarding Maps, I currently see several ways how to do that:
- A full-blown implementation of a custom Subject for our Map indexes (which are not Maps from the Java Collection Framework).
- For a proxy of the efforts, required to implement them, one can see the appropriate Truth subjects: for example, MapSubject.
- Use a hack and rely on the MapSubject. It can be done in several ways:
- either by having an explicit method converting MapIndex to a Map:
assertThat(exonumMapIndex).asMap().containsEntry(k, v); - or by extending the MapSubject and performing this conversion in the constructor:
assertThat(exonumMapIndex).containsEntry(k, v).
- either by having an explicit method converting MapIndex to a Map:
The big advantage of the second approach is that there is way less operations to implement (test, document) and maintain. There are a couple of disadvantages too: (a) unless we create an MapIndexToMapAdapter, they will always convert the MapIndex into Map in O(N) even for assertions using O(1) operations; (b) MapIndex is not a Map, therefore, some operations have different signatures (e.g., size -> long instead of int). I think (a) is negligible in usual tests; (b), though a valid concern and must be evaluated more carefully by inspecting operations in the MapSubject and Map indexes, does not seem to be a big issue with the present interface of our map indexes being very similar to Maps.
Looks nice.
they will always convert the MapIndex into Map in O(N) even for assertions using O(1) operations
I believe it's not a problem because usually there're not so many elements in test collections.