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

Do not fail on close() if test Kudu server is not initialized #11253

Merged
merged 1 commit into from
Mar 1, 2022

Conversation

losipiuk
Copy link
Member

@losipiuk losipiuk commented Mar 1, 2022

no release notes/docs

@cla-bot cla-bot bot added the cla-signed label Mar 1, 2022
@losipiuk losipiuk requested a review from hashhar March 1, 2022 13:42
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Note: I'm fine with the change, but i am also fine with afterclass failing if beforeclass failed.
Perhaps it helps read the logs?

@hashhar
Copy link
Member

hashhar commented Mar 1, 2022

Is Kudu server not being initialized when after-class is called an expected situation? Are we masking some existing issue?

@losipiuk
Copy link
Member Author

losipiuk commented Mar 1, 2022

Is Kudu server not being initialized when after-class is called an expected situation? Are we masking some existing issue?

With alwaysRun it is always called. Even if @BeforeClass method which is supposed to set the kuduServer fails before actually doing so.
It is not masking anything I think. We still get failure reports from intialization.

@losipiuk
Copy link
Member Author

losipiuk commented Mar 1, 2022

Note: I'm fine with the change, but i am also fine with afterclass failing if beforeclass failed.
Perhaps it helps read the logs?

AfterClass failure does not provide any useful information. It just adds noise into the logs. The actual failure is in tests initialization.

@losipiuk losipiuk force-pushed the lo/kudu-check-null-close branch from 78c56ff to 5a952ac Compare March 1, 2022 17:46
@losipiuk losipiuk merged commit fef4b2d into trinodb:master Mar 1, 2022
@github-actions github-actions bot added this to the 372 milestone Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants