-
Notifications
You must be signed in to change notification settings - Fork 201
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
Add integration test for Kinesis source #4967
Conversation
|
||
when(pipelineDescription.getPipelineName()).thenReturn(PIPELINE_NAME); | ||
|
||
String workerId = "worker-identifier-" + testId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chenqi0805! I will address this.
|
||
@BeforeEach | ||
void setup() { | ||
this.acknowledgementSetManager = mock(AcknowledgementSetManager.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might consider MockitoAnnotations.openMocks(this)
for initializing mocked instances
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chenqi0805! I will address this.
Signed-off-by: Souvik Bose <[email protected]>
598e4a2
to
c2ae582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Can you add a readme on how to run the integration tests? Similar to: https://github.com/opensearch-project/data-prepper/blob/main/data-prepper-plugins/s3-sink/README.md#developer-guide
} | ||
|
||
/** | ||
* CDK stack that creates a Kinesis stream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this stack supposed to create a Kinesis stream or not? I only see IAM policies being created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stream will be created as part of the test. The stack only adds the relevant permissions for accessing Kinesis streams and accessing the dynamoDb table for lease coordination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do a separate PR for adding the README.
Signed-off-by: Souvik Bose <[email protected]>
Description
This PR is to add an integration test for Kinesis source. As part of the integration test, it will perform the following actions:
putRecord
APIIssues Resolved
Resolves #[1082]
Check List
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.