Bug 1548535 - Cancel any pending workers when ping upload disabled

[travis79]

This cancels any pending PingUploadWorkers or MetricsPingWorkers when upload is disabled.

Pull Request checklist

• Quality: This PR builds and tests run cleanly
  • make test runs without emitting any warnings
  • make lint runs without emitting any errors
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry or an explanation of why it does not need one
  • Any breaking changes to language binding APIs are noted explicitly

[codecov-io]

Codecov Report

Merging #235 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #235      +/-   ##
============================================
+ Coverage     70.47%   70.55%   +0.08%     
- Complexity      187      188       +1     
============================================
  Files            51       51              
  Lines          3062     3067       +5     
  Branches        453      454       +1     
============================================
+ Hits           2158     2164       +6     
  Misses          579      579              
+ Partials        325      324       -1

Impacted Files	Coverage Δ	Complexity Δ
...oid/src/main/java/mozilla/telemetry/glean/Glean.kt	85.35% <100%> (+0.28%)	1 <0> (ø)	⬇️
...illa/telemetry/glean/scheduler/PingUploadWorker.kt	88.88% <100%> (+0.65%)	2 <0> (ø)	⬇️
.../telemetry/glean/scheduler/MetricsPingScheduler.kt	96.15% <100%> (+1.34%)	24 <0> (+1)	⬆️

---

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 e74e9d0...c6b8236. Read the comment docs.

[Dexterp37]

Thanks for taking care of this! It looks good overall, I have a few design considerations below. My "biggest concern" is testing: let's try to make these tests more "unit" and less "integration"

[Dexterp37]

I think I'd like to see this being more "unit" and less "integration": if we have two cancel functions, then we can test each one independently in their modules using the workmanager. Here, we'd just stub out and make sure that cancel is called when setUploadEnabled is called.

[travis79]

If I'm going to make the cancel functions static members of their respective classes, it's not really practical to stub out a static method using Mockito so I propose to leave this test as it is and file a follow up bug to investigate the best way to approach this. I did add unit tests for the cancel functions in each of the test classes associated with the workers.

[Dexterp37]

Good point, makes sense.