Cogito independent chat

Esysc commented 1 year ago

Details

A new optional field is added and its name is sinks. It takes a slice of strings as values, corresponding to all Cogito supported sinks. This parameter can be set as source.sinks and/or put.params.sinks level. The latter if specified overrides the source configuration.
In order to be backward compatible, if nothing is specified it behaviors as before defaulting to all supported sinks.

Best review commit by commit.

Tests criteria

The acceptance test, aka e2e tests, are increased by new CI jobs around the only chat feature:

send message to chat using a standard Cogito resource, but specify the sink gchat in put.params.
send message to chat using a custom only chat Cogito resource.
send message to chat using a custom only chat Cogito resource plus setting a message directory and a message file.

The test coverage has slightly decreased (from 99.5% to 99.0%), I couldn't cover a special case using unit tests. Happily, it's covered by the acceptance tests. Actually, the code below:

is executed clearly in the following CI job (see the red arrow):

Todos

[ ] Update remaining pipelines: replace Hangouts resource

Esysc commented 1 year ago

@Pix4D/integration the PR is ready for review

marco-m-pix4d commented 1 year ago

@Esysc Note that GitHub decided to hide part of the comments; be careful not to loose them :-)

Esysc commented 1 year ago

@marco-m-pix4d I should have addressed all your remarks.

Note on Sinker(): I've moved the check to a private function, so now it doesn't return an error anymore and the code test coverage is now increased of 0.1% (from 99.0 % to 99.1 %). The difference with master is still the fact that the function need to filter the []Sinker returned based on the optional sinks configuration.

Esysc commented 1 year ago

@odormond thank you for your review. It gave me the occasion to investigate further, simplify and also catch a bug processing input dirs. I've removed some useless and wrong tests that the previous code was hiding. I have added the only-msgdir directory in testdata that was missing.

Esysc commented 1 year ago

@Pix4D/integration ready for another review.

marco-m-pix4d commented 1 year ago

@Esysc I saw a lot of activity on the PR but it is not clear to me if all the points have been addressed; could you please mention the team when the PR is ready for review again? Merci :-)

Ah Ah, our comments just intersected. Good, thanks.

Esysc commented 1 year ago

@Pix4D/integration I've finally terminated, it took some time to validate the code. The last commit adds a unit test to increase the coverage from 99.3% to 99.5% as it is in the current master.

Esysc commented 1 year ago

Sorry for force-push, I rewrote the history.

Pix4D / cogito