Producer Improvements & Unit Tests

esrlabs / chipmunk

log analysis tool

Apache License 2.0

553 stars 39 forks source link

Producer Improvements & Unit Tests #2045

Open AmmarAbouZor opened 2 months ago

AmmarAbouZor commented 2 months ago

This PR provide unit-tests for producer main loop in the function read_next_segment().

[x] Tests should cover each case in the parse returns
[x] Code of producer is checked and a couple of notes are written in the code itself to be discussed with the reviewers
[x] Add Unit-Test for byte sources to uncover the bugs found and solve them after that.

This PR will stay as Draft until the comments in code are resolved.

AmmarAbouZor commented 2 months ago

I've found some points in the code which could be potential bugs or to be improved. I've written all of them directly in source code so their are easy to find by reviewing the code changes in this pr ignoring all the changes inside "tests" directory.

@marcmo @DmitryAstafyev Could you please take a look on the notes so we can discuss them in more details and resolve them.

Most of the potential bugs are because of the assumption that the parser will consume all the available data on each parse call and that the reload method on byte-source will be called with an empty buffer only

AmmarAbouZor commented 1 month ago

Unit Tests for all byte source has been added to insure reload result matches the ReloadResult signature. These tests uncovered that some byte sources weren't returning the correct available bytes, which has been fixed and the tests are green for all sources expect PCAP legacy source because I couldn't provide an input that allows the reload method to be called twice.

@marcmo Is it possible to have input for the legacy PCAP source that allows calling reload method twice? Besides that, there are other potential bugs that we need to clarify in the PR

marcmo commented 1 month ago

is this ready for review? if so please mark it non-draft

AmmarAbouZor commented 1 month ago

@marcmo We still have some points that we need to clarify here than I can write the rest of the unit tests then it can be ready for review. The remaining points are still marked in the code with BUG comments

AmmarAbouZor commented 1 day ago

@marcmo I've resolve the issue with creating valid input for all the tests to pass. Now we still have three points marked is BUG in file /indexer/sources/src/producer.rs that we still need to clarify, then this PR would be ready for final review