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

Wrap InterruptedException with an unchecked exception in TestSubscriber#awaitValueCount(). #4453

Merged
merged 1 commit into from
Sep 1, 2016

Conversation

vy
Copy link

@vy vy commented Sep 1, 2016

In its current form, awaitValueCount() is the only TestSubscriber#await*method that throws a checked exception (that is, InterruptedException), whereas the others wrap it with a IllegalStateException. This spreads a try-catch disease throughout the entire code base where awaitValueCount() is used. One can argue that why not just declaring the exception in the caller method footprint: Because you might be implementing an interface (e.g. Runnable) which does not allow any exceptions in its footprint. This patch wraps the InterruptedException with an unchecked exception in TestSubscriber#awaitValueCount().

@codecov-io
Copy link

codecov-io commented Sep 1, 2016

Current coverage is 84.25% (diff: 50.00%)

Merging #4453 into 1.x will increase coverage by 0.01%

@@                1.x      #4453   diff @@
==========================================
  Files           271        271          
  Lines         17599      17602     +3   
  Methods           0          0          
  Messages          0          0          
  Branches       2683       2683          
==========================================
+ Hits          14825      14831     +6   
  Misses         1915       1915          
+ Partials        859        856     -3   

Powered by Codecov. Last update 70bb06a...d0294c9

@akarnokd
Copy link
Member

akarnokd commented Sep 1, 2016

👍

@akarnokd akarnokd merged commit 30da1aa into ReactiveX:1.x Sep 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants