search

canonical / pebble

Take control of your internal daemons!
GNU General Public License v3.0
137 stars 52 forks source link

fix(daemon): improve health state lock test, remove LockCount #373

Closed benhoyt closed 3 months ago

benhoyt commented 4 months ago

As Harry pointed out at https://github.com/canonical/pebble/pull/369#discussion_r1505423350, there's a much simpler way to test this without needing a new State method like LockCount. Just acquire the state lock, then call the endpoint. If it times out, we know it was trying to acquire the lock.

In addition, fix an issue where the health endpoint would still hold the state lock if it returned an error. Fix those and add a test for that too.

As far as I'm concerned, this additional fix isn't critical to ship out right away, as it's extremely unlikely the health check endpoint returns an error. It can just go out in the next version of Pebble.

benhoyt commented 3 months ago

Going to just merge this without further review (for the 1.10.0 release) as it's test-only changes. @hpidcock: if you have any concerns I can put up another PR.