-
Notifications
You must be signed in to change notification settings - Fork 32
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: Integration test admin delete #194
test: Integration test admin delete #194
Conversation
During the execution of the destructor for TestManager I would like you to evaluate whether this implementation is valid or not. |
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.
Thank you for adding tests for dy admin delete
command. I left some comments. Could you please check them?
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.
Thank you for polishing up the code. Now, LGTM. I left some comments for additional improvement. If you'd like to include them in this pull request, I'll check them after you change them.
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 think added logic is reasonable. Could you remove the unused function count_temporary_tables
?
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.
Thank you for improving the pull request. All looks great to me.
165ed2e
to
7b63afd
Compare
Added method on TestManager to remove a temporary table to prevent double deletion in Drop trait Signed-off-by: KIrie-0217 <kook.cuckoo@gmail.com>
7b63afd
to
142a7ae
Compare
Issue #, if available:
#143
Description of changes:
I have added the test for dy admin delete command.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.