Skip to content

Commit

Permalink
[FAB-4184] Improve UT coverage for kafka package
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
kchristidis committed Jun 6, 2017
1 parent 35b7717 commit 79660c3
Show file tree
Hide file tree
Showing 26 changed files with 2,077 additions and 2,247 deletions.
113 changes: 0 additions & 113 deletions orderer/kafka/broker.go

This file was deleted.

65 changes: 0 additions & 65 deletions orderer/kafka/broker_mock_test.go

This file was deleted.

83 changes: 0 additions & 83 deletions orderer/kafka/broker_test.go

This file was deleted.

Loading

0 comments on commit 79660c3

Please sign in to comment.