-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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-15427][Statebackend][test] Check TTL test in test_stream_statettl.sh and skip the exception check #10726
Conversation
…_ttl.sh and skip the exception check
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 461a277 (Tue Dec 31 02:05:59 UTC 2019) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
CI report:
Bot commandsThe @flinkbot bot supports the following commands:
|
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.
+1, the solution follows similar pattern as #10588 and could verify the result correctly according to the source code of TtlVerifyUpdateFunction#flatMap
Minor: changing |
@carp84 thanks for the review. Add specific error message and check against it is better(I also add a comment to remind others do not change this specific error message). After adding the specific error message we would not get a false pass even if somebody changes the |
… to verify the test
ran the end-to-end test locally
2 comment the if-check in
|
The Travis test failed because of another irrelevant issue(has been tracked by FLINK-15247) |
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 for hardening this end-to-end test case @klion26. I think we are still missing some call sites of test_stream_state_ttl.sh
. Once this is updated, we can move forward with merging this PR.
@tillrohrmann thanks for the review. addressed the comments, PTAL, thanks~ |
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 for addressing my comments @klion26. LGTM. Merging this PR now.
…_ttl.sh and skip the exception check Add some specific error message in TtlVerifyUpdateFunction and use it to verify the test This closes #10726.
What is the purpose of the change
Add test check logic in
test_stream_state_ttl.sh
for TTL end-to-end test, and skip the exception check for those tests.Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation