search

chaostoolkit / chaostoolkit-lib

The Chaos Toolkit core library
https://chaostoolkit.org/
Apache License 2.0
76 stars 46 forks source link

Support datetime and Exception objects when calling `notify_with_http` and flesh out `notification.py` tests #244

Closed ciaransweet closed 3 years ago

ciaransweet commented 3 years ago

This PR is in response to @goyerl's issue (https://github.com/chaostoolkit-incubator/chaostoolkit-aws/issues/123) which raises that datetime objects cause notifications with http to fail as they are not JSON serialisable.

To combat this, I've implemented a PayloadEncoder which current only maps datetime objects (to their isoformat equivalent) and also Exception objects into a string of An exception was raised: <ClassName>('<str(obj)>') as we explicitly support errors being passed to notify, but these would fail serialising (I'm not sure how we've managed to get away with this so far :rofl:)

Whilst implementing these fixes, I noticed that the tests in tests/test_notification.py weren't all actually exercising the functionality properly in the code to ensure it was doing what it was supposed to. In the spirit of leaving things cleaner than when I found them, I've fleshed out the tests which now get 100% coverage (I know that's not everything, but it's actually exercising each thing with assertions now) and added some more branches that weren't being covered (Handling plugin failures for example).

In the spirit of the inpending type tsunami, I've also added types to what I've touched.