Code review comment for ~ballot/content-cache-charm/+git/content-cache-charm:add_lua

Revision history for this message
Benjamin Allot (ballot) wrote :

Ah damn, I was rereading yesterday night but I missed quite a lot of consistency/debug stnzas. My bad.

About the separated tests:
I had done so in my previous configuration. However, it had his share of probem:
* a lot lot of files needed to be modified without adding any value (listen, backend haproxy ones notably)
* I had introduced a specific clause based on the name of the key to inject the enable_prometheus_metrics boolean to True. I found that error prone
After weighting the pros and cons, I ended up doing a separate file to test only the part relevant to the metrics exposure.

About the noqa, I had an issue using the form
```
patcher = mock.patch.multiple(....
patcher.start()
.
.
patcher.stop()
```

For some reason, the patcher.stop() was not removing the patch and I was ending with others tests failing.
I spent time trying to figure out why but could not find out so I use the context manager since it was "working".
I'm still looking for a more elegant solution to handle this but I figured it should not block the progress.

« Back to merge proposal