-
Notifications
You must be signed in to change notification settings - Fork 570
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
test: add create and drop sink test for nexmark #8686
Conversation
There seem too many logs. Can we disable these?
|
timeout? |
Codecov Report
@@ Coverage Diff @@
## main #8686 +/- ##
==========================================
- Coverage 70.88% 70.87% -0.01%
==========================================
Files 1197 1197
Lines 199012 199012
==========================================
- Hits 141069 141059 -10
- Misses 57943 57953 +10
Flags with carried forward coverage won't be shown. Click here to find out more. see 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The |
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.
+1
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.
this is weird, I tested locally on my Mac, and it finishes very fast besides the 20-second sleep on purpose...
🤔
After Bugen's two PR that makes drop sink
completely async, drop sink
is expected to be very fast.
This PR intends to detect this type of anomaly though.
Wait, can we run the tests in parallel?! 🥵 |
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.
basically LGTM
lets keep watching the CI
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
Yes, any step behind |
I mean |
Succeed once after moving to the Good to merge? |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously the
drop sink
can be stuck (not sure if it is stuck forever or comes back to normal after a long period of time)After #8672, the
drop sink
is super fast.We add
drop sink
into e2e tests to avoid similar problems from happening again in the futureChecklist For Contributors
[ ] I have written necessary rustdoc comments[ ] I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).[ ] I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future.(Please refer to the issue)./risedev check
(or alias,./risedev c
)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note