General improvements
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
andDemuxTopic
. - 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
- Closes #210 (closed)
- Closes #177 (closed)
- Closes #205 (closed)
- Relates to JIRA issue DM-398 and DM-399
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
andFlatbufferMessage
. - Modified or deleted (now useless unit tests) for several other classes/parts.
Other
- There is still significant overlap in functionality between
Streamer
andDemuxTopic
, 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?).