Skip to content
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

Implement testCreateTable and testDropTable in Kudu connector test #12899

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

chen-ni
Copy link
Contributor

@chen-ni chen-ni commented Jun 18, 2022

Description

Is this change a fix, improvement, new feature, refactoring, or other?

An improvement in testing.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Connector tests.

How would you describe this change to a non-technical end-user or system administrator?

Improved some tests that shouldn't change any behavior.

Related issues, pull requests, and links

A series of PRs will be created to implement #11815. This is the first one.

There are 14 tests to fix in total, this PR fixes the first 2.

Documentation

(x) No documentation is needed.

Release notes

(x) No release notes entries required.

@cla-bot
Copy link

cla-bot bot commented Jun 18, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@chen-ni
Copy link
Contributor Author

chen-ni commented Jun 18, 2022

Just emailed the signed CLA. Will the check automatically pass when the document is received?

@cla-bot
Copy link

cla-bot bot commented Jun 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

2 similar comments
@cla-bot
Copy link

cla-bot bot commented Jun 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@cla-bot
Copy link

cla-bot bot commented Jun 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@chen-ni chen-ni requested a review from ebyhr June 19, 2022 22:28
@cla-bot
Copy link

cla-bot bot commented Jun 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@ebyhr ebyhr changed the title Update Kudu connector tests Implement testCreateTable and testDropTable in Kudu connector test Jun 20, 2022
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash commits into one. You can update the commit title as "Implement testCreateTable and testDropTable in Kudu connector test".
https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#git-merge-strategy

fix #11815

GitHub automatically closes a linked issue if you use "fix {number}". I modified the description as we don't want to close it.
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label Jun 20, 2022
@chen-ni chen-ni force-pushed the issue-11815-part-1 branch from 41f398f to 7ff3b77 Compare June 20, 2022 02:50
@cla-bot
Copy link

cla-bot bot commented Jun 20, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@chen-ni
Copy link
Contributor Author

chen-ni commented Jun 20, 2022

@ebyhr Got it :) Just squashed and force-pushed the commit. Still have to wait for the CLA, though..

@ebyhr
Copy link
Member

ebyhr commented Jun 20, 2022

Could you fix checkstyle failure? https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#code-style

Error:  src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java:[39,1] (imports) ImportOrder: Wrong order for 'org.testng.Assert.assertFalse' import.

@chen-ni
Copy link
Contributor Author

chen-ni commented Jun 20, 2022

@ebyhr Sorry, I've been having trouble running the style check locally..
Will fix this today after work :)

@chen-ni chen-ni force-pushed the issue-11815-part-1 branch from 7ff3b77 to 58235d3 Compare June 20, 2022 12:49
@cla-bot
Copy link

cla-bot bot commented Jun 20, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@chen-ni
Copy link
Contributor Author

chen-ni commented Jun 20, 2022

Just fixed the checkstyle error, stashed and pushed the commit :)

@martint
Copy link
Member

martint commented Jun 21, 2022

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Jun 21, 2022
@cla-bot
Copy link

cla-bot bot commented Jun 21, 2022

The cla-bot has been summoned, and re-checked this pull request!

@mosabua
Copy link
Member

mosabua commented Jun 21, 2022

@ebyhr CLA is done now .. I am not sure if the test failure is a false alarm .. please proceed to review or merge or whatever appropriate as next step. Thanks

@ebyhr ebyhr merged commit b5bf1f5 into trinodb:master Jun 22, 2022
@ebyhr
Copy link
Member

ebyhr commented Jun 22, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 387 milestone Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

4 participants