Add option for background thread reporting

[kbaum]

resolves #124

Kicks off thread from the middleware on first request if Configuration#backround_reporting_enabled.

Currently only handles background reporting from web application servers through middleware.

TODO: Kick off background thread from Coverband.start for other ruby processes like sidekiq.

[danmayer]

Nice looks good @kbaum considering background jobs sidekiq and cron already require some custom code to support, do we think this would be good enough for the first usable version for 3.1... I am also going to try to think of a way to benchmark this, I am pretty sure enabling this on coverband_demo and running the apache benchmark tests against it would show an improvement.

[kbaum]

I agree that this is probably good enough for 3.1 considering sidekiq already requires custom code. That being said, I think with this addition, sidekiq and other non-rack ruby processes would work well:
1067fab
The idea is that if it's a rack server, the thread gets kicked off on the first web request. All other processes like sidekiq, delayed_job, plain scripts, rake tasks, etc kick off the background reporter on Coverband.start.

On another note, I noticed this morning that what i had pushed last night was not working since the code was actually referencing Coverband::BackgroundReporter which I had renamed to Coverband::Background. First obviously I didn't do enough manual testing but this is also telling me that we are lacking some test coverage. I looked at collectors_coverage_test and it seems to be missing coverage on report_coverage. I added a basic test that tests the changes in this PR, but I'm wondering if we should add some coverage in this area before cutting a new release.

Let me know what you think.
thx.

[danmayer]

yeah agreed we are missing some coverage, I think tests for that got lost and removed in the refactoring pushing everything back into a single collector... I created this issue to try to make it a bit easier to see when test coverage accidentally gets reduced #136

We do probably need to get some coverage back on that method, although, I do think a coverage report would be helpful as it likely does get tested in the full stack test: https://github.com/danmayer/coverband/blob/master/test/unit/full_stack_test.rb

probably need to do a pass on the test folder making integration, unit, and other tests a bit more clear.

[kbaum]

One problem I noticed is that report_coverage actually traps and logs all exceptions. While this makes sense in production, think we should avoid this somehow when we are running our tests.

[danmayer]

makes sense @kbaum we could reraise run in test mode, I guess just need to figure out what to check for being in that mode. So used to Rails.env, but guessing minitest has some environment var we can check.

[kbaum]

Think we could just set a configuration in teat_helper.rb.

…

On Mon, Nov 12, 2018 at 10:19 PM Dan Mayer ***@***.***> wrote: makes sense @kbaum <https://github.com/kbaum> we could reraise run in test mode, I guess just need to figure out what to check for being in that mode. So used to Rails.env, but guessing minitest has some environment var we can check. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#138 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAF6ElwDHctM21GcO7R63HXVp8BWI2l5ks5uujo2gaJpZM4YYuO4> .

[danmayer]

Looks good feel free to merge in when ready. I am going to start to merge some of the other stuff we expect to be in the 3.0.1. Once we have all the basics merged and I do some local tests I can cut an Alpha release to do additional testing.

[danmayer]

yeah Coverband.config.runtime_env='test' or the like