Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement TestContainers Unit Tests with CaptureProxy #508

Merged

Conversation

AndreKurait
Copy link
Member

@AndreKurait AndreKurait commented Feb 12, 2024

Description

Adds CaptureProxy test container tests with a framework to test and simulate a variety of failure modes. In this version, adding the base functionality to spin up docker containers for Kafka as well as a Destination container which are reused between tests, as well as a ToxiProxy container to be used on each test as a proxy between the CaptureProxy and Kafka to enable simulation of failure modes (network disconnect, latency, etc.).

This PR includes a set of tests which simulate Kafka failure modes occurring prior to the start of the CaptureProxy. These tests are currently failing and marked as Disabled due to a bug in the CaptureProxy and will be enabled with the bug fix in MIGRATIONS-1489

Also, updates test containers version to 1.19.5 to address a mend security issue.

  • Category: Enhancement, Maintenance
  • Why these changes are required?
    • To enable high velocity testing for Capture Proxy reliability by allowing failure mode simulation environments to be spun up and used in unit tests.
  • What is the old behavior before changes and new behavior after changes?
    • No behavior changes

Issues Resolved

MIGRATIONS-1503

Is this a backport? If so, please add backport PR # and/or commits #
No

Testing

Unit Testing

Check List

  • [x ] New functionality includes testing
    • [ x] All tests pass, including unit test, integration test and doctest
  • [x ] New functionality has been documented
  • [x ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@AndreKurait AndreKurait force-pushed the ProxyStartupUnitTests branch from a9d5d4e to ba29790 Compare February 12, 2024 18:06
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d3820af) 75.48% compared to head (47e1cf3) 76.45%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #508      +/-   ##
============================================
+ Coverage     75.48%   76.45%   +0.96%     
- Complexity     1360     1375      +15     
============================================
  Files           158      158              
  Lines          6085     6085              
  Branches        530      530              
============================================
+ Hits           4593     4652      +59     
+ Misses         1139     1075      -64     
- Partials        353      358       +5     
Flag Coverage Δ
unittests 76.45% <ø> (+0.96%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndreKurait AndreKurait marked this pull request as ready for review February 12, 2024 19:09
@AndreKurait AndreKurait self-assigned this Feb 12, 2024
@AndreKurait AndreKurait added enhancement New feature or request security fix Security fix generated by Mend labels Feb 12, 2024
// Calculate average duration of impaired calls
var averageImpairedDuration = assertBasicCalls(captureProxy, numberOfTests);

long acceptableDifference = Duration.ofMillis(25).toMillis();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that this a risk to become a flaky test? How can we make it relatively stable (> 99-99.9%)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The observed latency is 5-10ms on all the runs I've done, that's where 25 was set to have the buffer.

Even when the machine is overloaded by concurrent tests, this is measuring the difference with vs without the proxy so it has some resiliency there. We could move it to a % of the total time if that seems more robust.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we see it flaky, we can add a resource lock on it to execute in isolation

@AndreKurait AndreKurait merged commit e016226 into opensearch-project:main Feb 14, 2024
7 checks passed
@AndreKurait AndreKurait deleted the ProxyStartupUnitTests branch February 14, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security fix Security fix generated by Mend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants