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

[FLINK-33073][Connectors/AWS] Implement end-to-end tests for KinesisS… #94

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

hlteoh37
Copy link
Contributor

…treamsSink

Purpose of the change

Implement end-to-end tests for KinesisStreamsSink.

Verifying this change

This change added tests and can be verified as follows:

  • Added end-to-end tests

Significant changes

(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for convenience.)

  • Dependencies have been added or upgraded
  • Public API has been changed (Public API is any class annotated with @Public(Evolving))
  • Serializers have been changed
  • New feature has been introduced
    • If yes, how is this documented? (not applicable / docs / JavaDocs / not documented)

@hlteoh37 hlteoh37 force-pushed the end-to-end-test branch 4 times, most recently from 3d70220 to 43c35f5 Compare September 12, 2023 07:10
@hlteoh37 hlteoh37 force-pushed the end-to-end-test branch 4 times, most recently from 75f03d9 to e7e7694 Compare September 23, 2023 14:58
@hlteoh37
Copy link
Contributor Author

Verified that running AWS end-to-end tests locally all pass

FLINK_AWS_USER=<AWS_ACCESS_KEY_ID> FLINK_AWS_PASSWORK=<AWS_SECRET_ACCESS_KEY> mvn clean verify -Prun-aws-end-to-end-tests -DdistDir=~/workplace/flink_os/flink/flink-dist/target/flink-dist_2.12-1.18-SNAPSHOT.jar -pl flink-connector-aws-kinesis-streams-e2e-tests -Dflink.version=1.18-SNAPSHOT
[INFO] --- maven-surefire-plugin:3.0.0-M5:test (end-to-end-tests) @ flink-connector-aws-kinesis-streams-e2e-tests ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.flink.connector.kinesis.sink.KinesisStreamsSinkITCase
[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 72.61 s - in org.apache.flink.connector.kinesis.sink.KinesisStreamsSinkITCase
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:27 min
[INFO] Finished at: 2023-09-23T15:55:17+01:00
[INFO] ------------------------------------------------------------------------

LOGGER.info("Successfully created Kinesis Data Stream with ARN {}.", streamARN);

LOGGER.info("Waiting until Kinesis Data Stream with ARN {} is ACTIVE.", streamARN);
kinesisWaiter.waitUntilStreamExists(
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checking this waits for stream to be ACTIVE? It is not too clear from the docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep we can see this from the API itself

@AfterEach
void teardown() {
kinesisResourceManager.close();
AWSGeneralUtil.closeResources(httpClient, kinesisClient);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we run cleanUpStaleKinesisDataStreams(); on @AfterClass as to speed up the tests? And possibly on @BeforeClass incase the tests starts failing...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats a good idea. As discussed offline - moving this to a separate "dummy" test class

Copy link
Contributor

@dannycranmer dannycranmer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hlteoh37

@dannycranmer dannycranmer merged commit 9743a29 into apache:main Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants