-
Notifications
You must be signed in to change notification settings - Fork 13k
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-4110] Add testing skeleton to quickstart #2153
Conversation
|
||
|
||
/** | ||
* This test uses a finite stream. The TestingSource will stop after 1000 elements have been send. |
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.
send --> sent
This will be very helpful! LGTM. |
I think these tests are a bit too specific for the quickstarts. Why starting your own Flink cluster when the local execution environment would do the same? People using the quickstarts should not have immediately a problem with long test run times so it shouldn't hurt them if a cluster is started for each test case. I think it would be better to properly document these testing techniques somewhere since they are only relevant for advanced users. We might include them in the online documentation or in the planned Flink streaming cook book. |
What I would like to see here is that the tests in the quickstart actually test the code from the job in the quickstart. For example if the job in the quickstart is WordCount it would be nice to show tests that are actually testing that it counts words correctly. |
+1 to what Till and Jamie said. Regarding the removed joda-time exclusion: that's independent of the testing skeleton, right? |
Yes, the joda-time exclusion is independent. I just experienced that this has been an issue for our users multiple times. I'll update the tests to test the |
Now that some time has passed, I feel like we should really cover this in the docs first. What do you think? |
I agree. There's a recent example of a user who suggested the same thing: http://apache-flink-user-mailing-list-archive.2336050.n4.nabble.com/TestStreamEnvironment-await-last-flush-of-processing-time-based-windows-td11232.html |
OK, let's close this then? |
Yup |
With this change, our quickstart archetypes will also contain some sample code for bringing up an embedded server for testing purposes.