You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Before this changeset: 20 files, 807 LOC (sans tests), UT coverage 65.5%
With this changeset: 10 files, 659 LOC (sans tests), UT coverage 97.2%
This bump in coverage percentage comes with some significant
refactoring. Its timing so close to a release cut makes this admittedly
unfortunate, but I would argue that everything else about it is a
substantial improvement.
Background: Back when this package was originally written, in order to
have integration tests, I had to write my own full-fledged mock objects,
extending the ones given to me by the underlying Kafka library. This
forced me to come up with additional interfaces in the non-test path so
that I can do test injection (see: `TestableConsenter`), sacrificing the
code's readability, and bumping its complexity both in the non-test and
the test paths.
Now we're at a stage where I can use `bootstrap.feature` to run
Kafka-related integration tests (see: FAB-3387), so all of this
complexity in the `kafka` package can go away. The result is a package
that is (a) much easier to read, (b) no longer requires me to maintain
thin wrappers around the sarama library objects, and (c) makes unit
tests <strike>very<strike> easy to write, as evidenced by the
significant bump in coverage.
Also worth noting: no new logic/features have been added with this
changeset. This is simply a refactoring of the existing logic.
Review entrypoint: consenter.go
Change-Id: I3e0e1c1d74d2dfdc70483e15a6823281ba2ddd19
Signed-off-by: Kostas Christidis <[email protected]>
0 commit comments