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.

Sign up for GitHub

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

Jump to bottom

Fix a crash while counting logs and disabling a module #301

Merged
merged 2 commits into from
Jan 13, 2017
Merged

Fix a crash while counting logs and disabling a module #301

merged 2 commits into from
Jan 13, 2017

Conversation

guperrot
Copy link
Member

No description provided.

@guperrot guperrot requested a review from jaeklim January 12, 2017 03:13
@msftclas
Copy link

Hi @guperrot, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Guillaume Perrot). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@codecov-io
Copy link

codecov-io commented Jan 12, 2017 •
edited
Loading

Current coverage is 100% (diff: 100%)

Merging #301 into develop will not change coverage

@@           develop   #301   diff @@
=====================================
  Files           60     60          
  Lines         2699   2704     +5   
  Methods          0      0          
  Messages         0      0          
  Branches       492    493     +1   
=====================================
+ Hits          2699   2704     +5   
  Misses           0      0          
  Partials         0      0

Powered by Codecov. Last update 1959825...b837da8

jaeklim
jaeklim approved these changes Jan 13, 2017
View reviewed changes
sdk/mobile-center/src/test/java/com/microsoft/azure/mobile/channel/DefaultChannelTest.java
try {
latch.await();
} catch (InterruptedException e) {
throw new RuntimeException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it throws a runtime exception, does the test fail?
It is running on another thread so I don't think it will fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just that I prefer the unit test to crash so we can investigate than having a silent exception and trying to understand why the test breaks (in the future).

@guperrot guperrot merged commit f2baab3 into develop Jan 13, 2017
@guperrot guperrot deleted the fix_crash_in_database_count branch January 13, 2017 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaeklim jaeklim jaeklim approved these changes

Assignees

@jaeklim jaeklim

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@guperrot @msftclas @codecov-io @jaeklim