Code review comment for lp:~ahasenack/txaws/no-local-dir-for-ca-lookup

Jamu Kakar (jkakar) wrote :

[1]

The test is tainting the TXAWS_CERTS_PATH environment variable. You
could do something like the following. Also, I recommend you add a
simple sentence clearly describing the expected behaviour.

def test_get_ca_certs_no_current_dir(self):
    """
    A L{CertsNotFoundError} exception is raised if no certificate paths are
    available.
    """
    original_certs_path = os.environ.get("TXAWS_CERTS_PATH")
    try:
        os.environ["TXAWS_CERTS_PATH"] = "%s:" % self.no_certs_dir
        os.chdir(self.one_cert_dir)
        self.assertRaises(exception.CertsNotFoundError, ssl.get_ca_certs)
    finally:
        if original_certs_path:
            os.environ["TXAWS_CERTS_PATH"] = original_certs_path
        else:
            del os.environ["TXAWS_CERTS_PATH"]

Nice work, +1! :)

review: Approve

« Back to merge proposal