Skip to content

General improvements

Afonso Mukai requested to merge issue_210 into master

Created by: SkyToGround

Description of work

So this turned into a bit of a beast. It started out as a fix of issue #210 (closed) but it turned into something ... more.

The following should be a (mostly) complete list of what has been done.

  • Fixes issue #210 (closed) by moving responsibility of getting flatbuffer metadata access into a new class: FlatbufferMessage.
  • Removed duplicated functionality from a bunch of places, mostly Streamer and DemuxTopic.
  • Added support for the trompeloeil mocking library in order to more easily implement unit tests.
  • Changed over to using C++14 in order to be able to use trompeloeil.
  • Removed a whole lot of dead code.
  • Made some names more descriptive.
  • Modified KafkaW::Msg to reduce coupling and thus simplify unit testing.
  • Modified several classes to simplify future and current unit testing.
  • Fixed several problems (i.e. bugs) found mostly in Streamer.

Because of the changes I made I had to touch a lot of files. I am sorry about and I will try to keep pull requests smaller in the future.

Integration tests

I have so far done the following integration tests:

  • Faulty kafka (flatbuffer) messages are ignored (manual test)
  • Valid kafka (flatbuffer) messages with an unknown source name are ignored (manual test)
  • Valid and known kafka (flatbuffer) messages are written to file (manual test).
  • Jenkins automatic integration test was completed successfully.

There is one or two more integration tests will be done before I think we are ready to merge.

Issue

Acceptance Criteria

  • Are there design problems with this implementation?
  • Do we need more unit tests?
  • How much documentation is the minimum required (currently I have added nothing).

Unit Tests

  • Added unit tests for Streamer, DemuxTopic and FlatbufferMessage.
  • Modified or deleted (now useless unit tests) for several other classes/parts.

Other

  • There is still significant overlap in functionality between Streamer and DemuxTopic, I have deferred fixing that for now but maybe I should not.

Code Review (To be filled in by the reviewer only)

  • Is the code of an acceptable quality?
  • Do the changes function as described and is it robust?

Nominate for Group Code Review (Anyone can nominate it)

Indicate if you think the code should be reviewed in a Thursday code review session.

  • Recommend for group code review

Also, nominate it on the code_review Slack channel (does someone want to automate this?).

Merge request reports