-
Notifications
You must be signed in to change notification settings - Fork 303
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 blog article: Howto test a batch source with the new Source framework #643
Conversation
@MartijnVisser @zentol did almost all my last reviews. Would you have time to review this small article as you know the source framework ? |
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.
I took a quick look over it. I feel especially for the IT case it would be nice to first provide an overview over the differnet parts that make up a test "There's a test framework; We need a backend the source can read from, a test context for Flinks testing framework to interact with the backend, ..." and then drill down into the details as you already did.
Right now it's very "I have to do this and this and this" but you don't really know where you're heading.
|
||
### Testing the serializers | ||
|
||
[example Cassandra SplitSerializer](https://github.com/apache/flink-connector-cassandra/blob/d92dc8d891098a9ca6a7de6062b4630079beaaef/flink-connector-cassandra/src/main/java/org/apache/flink/connector/cassandra/source/split/CassandraSplitSerializer.java) |
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.
[example Cassandra SplitSerializer](https://github.com/apache/flink-connector-cassandra/blob/d92dc8d891098a9ca6a7de6062b4630079beaaef/flink-connector-cassandra/src/main/java/org/apache/flink/connector/cassandra/source/split/CassandraSplitSerializer.java) | |
[Example Cassandra SplitSerializer](https://github.com/apache/flink-connector-cassandra/blob/d92dc8d891098a9ca6a7de6062b4630079beaaef/flink-connector-cassandra/src/main/java/org/apache/flink/connector/cassandra/source/split/CassandraSplitSerializer.java) |
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.
done
](https://github.com/apache/flink-connector-cassandra/blob/d92dc8d891098a9ca6a7de6062b4630079beaaef/flink-connector-cassandra/src/test/java/org/apache/flink/connector/cassandra/source/CassandraSourceITCase.java) | ||
|
||
For tests that require a running backend, Flink provides a JUnit5 source test framework. To use it | ||
we create an *ITCase named class that |
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.
we create an *ITCase named class that | |
we create an *ITCase named class that |
The name only really matters if the test is added to Flink. I think we shouldn't assume that because users might have their own sources they want to test that won't be contributed to Flink.
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.
👍
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.
done
MiniClusterTestEnvironment flinkTestEnvironment = new MiniClusterTestEnvironment(); | ||
` | ||
|
||
### Backend runtime environment |
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.
Maybe some high-level intro line what this is about could be helpful.
Something like "To test the connector we need a backend to run the connector against. Something something need to implement something to integrate this into junit5; maybe provided by library."
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.
done
The test context is scoped to the test case. So it is where we do things like creating test table, | ||
creating the source or writing test data. |
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 test context is scoped to the test case. So it is where we do things like creating test table, | |
creating the source or writing test data. | |
The test context provides Flink with means to interact with the backend, like inserting test data/tables or constructing sources/sink. |
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.
Agree with the rephrasing but I'd like also to keep the important notion of test case scope of the TestContext (compared to test suite scope of the backend environment)
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.
done
MiniClusterTestEnvironment flinkTestEnvironment = new MiniClusterTestEnvironment(); | ||
` | ||
|
||
### Backend runtime environment |
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.
### Backend runtime environment | |
### Backend environment |
It is never referenced as "backend runtime environment" in subsequent sections
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.
ok fair enough, in that case I'll also remove "runtime" from the "Flink runtime environment" title for coherence.
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.
done
Totally agree, I'll do something similar to what I did on the source creation blog: an architecture paragraph |
3d516fb
to
a11295e
Compare
done |
@zentol Thanks for your review (once again). I addressed all your comments PTAL |
Last article of the series about testing the created batch source in the previous article.
R:@zentol. Feel free to delegate as you have already the previous 2 articles to review.