Better setup / teardown for JobService

[natario1]

We don't rely anymore on onCreate / onDestroy to set up and tear down the job service. The service executes jobs in a separate thread so onDestroy is called before we would expect.

Fixes #744

[codecov]

Codecov Report

Merging #745 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #745      +/-   ##
============================================
- Coverage     53.39%   53.38%   -0.02%     
  Complexity     1746     1746              
============================================
  Files           132      132              
  Lines         10284    10286       +2     
  Branches       1426     1427       +1     
============================================
  Hits           5491     5491              
- Misses         4338     4340       +2     
  Partials        455      455

Impacted Files	Coverage Δ	Complexity Δ
...arse/src/main/java/com/parse/PushServiceApi26.java	0% <0%> (ø)	0 <0> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b7bb5b...ee4e62b. Read the comment docs.

[Jawnnypoo]

Maybe I am missing something, but it looks like no matter what, in the finally block, we are saying the job is done and calling jobFinished(jobParameters, false);. Shouldn't this true/false be dependent on if the push was a success or not? Otherwise, the push could fail, and we would never reschedule the push to retry

[natario1]

@Jawnnypoo by success you mean that handler.handlePush() does not throw?

[Jawnnypoo]

Yeah, it seems like there, we should be checking to see if an exception was thrown by the handler, and if so, reschedule. But maybe that is incorrect thinking, since more than likely the only reason a push would fail would be cases where it would always fail, and it should therefore not be rescheduled.

[natario1]

I got it, I think your second scenario makes sense. handler.handle() never reaches developer code, it is our own stuff. So if that throws there's something wrong with our handlers and we would end in a schedule loop

[Jawnnypoo]

Makes sense 👍

[Jawnnypoo]

Is there something we can do about this codecov thing?

[natario1]

@Jawnnypoo not easily. I had made a PushService26Test for one of these PRs, but then discarded because it requires Oreo, and we currently can't run Oreo tests due to Robolectric issues. I don't remember the exact reason right now but I can take a look

[natario1]

I have been playing with androidTests recently, that would solve a couple issues, but running them on Travis has been quite painful

[rogerhu]

I'm seeing issues in the current branch...sometimes the push handler crashes because handler is null.

[Jawnnypoo]

@rogerhu Are you seeing this in the tests, or in an app using the latest?

[rogerhu]

In the app. Let's merge and tag release. I will do more digging to understand what the issue is.