Code review comment for lp:~nicopace/charms/trusty/nagios/all-tests

Revision history for this message
nicopace (nicopace) wrote :

> Nicolas,
>
> I think Amir's error was due to checking out the branch directly instead of
> merging it against the upstream. It seems that the missing default series was
> fixed in the upstream already. With that fix, however, there are still the
> following issues with the tests:
>
> * The first call to requests in test_web_interface_with_ssl needs to be
> https:// instead of http://

The first option tests the 'on' option. That means that nagios should provide http and https (that's why the first request is without ssl, and the second with.

>
> * Both calls to requests in test_web_interface_with_ssl need to include
> verify=False to skip certificate validation

The first one doesn't use ssl, so it is not required.

>
> * The tests need to call d.sentry.wait() immediately after d.setup() to
> ensure that the environment stabilizes before running the tests

I've updated the test to reflect this.

« Back to merge proposal