Skip to content

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Dec 14, 2019

Throwing makes cleaner code and also is more helpful if the exception is ever
thrown, as the error will be more clear.


I noticed this after #6512 was already merged. Minor, but good to fix in an example.

CC @dounan

Throwing makes cleaner code and also is more helpful if the exception is ever
thrown, as the error will be more clear.
@ejona86 ejona86 requested a review from sanjaypujare December 14, 2019 00:57
} catch (InterruptedException e) {
e.printStackTrace();
}
public void tearDown() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of points:

  • when Exception does happen, it will fail the test (before it would have passed). I think that's better but making sure that's what you want.

  • throws is wider than it needs to be (InterruptedException).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I want the test to fail. Yes, I purposefully used Exception, since that is pretty normal in tests since most exceptions should just cause the test to fail.

Copy link
Contributor

@sanjaypujare sanjaypujare left a comment

Choose a reason for hiding this comment

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

LGTM

@ejona86 ejona86 merged commit 4357f7f into grpc:master Dec 16, 2019
@ejona86 ejona86 deleted the example-test-after-throw branch December 16, 2019 16:11
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants