diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 74084692d..e8675a169 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1026,6 +1026,12 @@ def renew(config, unused_plugins): _renew_describe_results(config, renew_successes, renew_failures, renew_skipped, parse_failures) + if renew_failures or parse_failures: + raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format( + len(renew_failures), len(parse_failures))) + else: + logger.debug("no renewal failures") + def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 60fa3ebec..2b4e443a8 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -549,8 +549,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._certonly_new_request_common, mock_client) def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None, - args=None, renew=True): - # pylint: disable=too-many-locals + args=None, renew=True, error_expected=False): + # pylint: disable=too-many-locals,too-many-arguments cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) @@ -576,11 +576,17 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly'] if extra_args: args += extra_args - self._call(args) - - if log_out: - with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: - self.assertTrue(log_out in lf.read()) + try: + ret, _, _, _ = self._call(args) + if ret: + print "Returned", ret + raise AssertionError(ret) + assert not error_expected, "renewal should have errored" + except: # pylint: disable=bare-except + if not error_expected: + raise AssertionError( + "Unexpected renewal error:\n" + + traceback.format_exc()) if renew: mock_client.obtain_certificate.assert_called_once_with(['isnot.org']) @@ -589,6 +595,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods except: self._dump_log() raise + finally: + if log_out: + with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: + self.assertTrue(log_out in lf.read()) return mock_lineage, mock_get_utility @@ -634,11 +644,13 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._test_renewal_common(True, [], args=args, renew=True) def test_renew_verb_empty_config(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'empty.conf'), 'w'): + rd = os.path.join(self.config_dir, 'renewal') + if not os.path.exists(rd): + os.makedirs(rd) + with open(os.path.join(rd, 'empty.conf'), 'w'): pass # leave the file empty - self.test_renew_verb() + args = ["renew", "--dry-run", "-tvv"] + self._test_renewal_common(False, [], args=args, renew=False, error_expected=True) def _make_dummy_renewal_config(self): renewer_configs_dir = os.path.join(self.config_dir, 'renewal') @@ -646,7 +658,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: f.write("My contents don't matter") - def _test_renew_common(self, renewalparams=None, + def _test_renew_common(self, renewalparams=None, error_expected=False, names=None, assert_oc_called=None): self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: @@ -658,7 +670,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_lineage.names.return_value = names mock_rc.return_value = mock_lineage with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, + self._test_renewal_common(True, None, error_expected=error_expected, args=['renew'], renew=False) if assert_oc_called is not None: if assert_oc_called: @@ -667,21 +679,22 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertFalse(mock_obtain_cert.called) def test_renew_no_renewalparams(self): - self._test_renew_common(assert_oc_called=False) + self._test_renew_common(assert_oc_called=False, error_expected=True) def test_renew_no_authenticator(self): - self._test_renew_common(renewalparams={}, assert_oc_called=False) + self._test_renew_common(renewalparams={}, assert_oc_called=False, + error_expected=True) def test_renew_with_bad_int(self): renewalparams = {'authenticator': 'webroot', 'rsa_key_size': 'over 9000'} - self._test_renew_common(renewalparams=renewalparams, + self._test_renew_common(renewalparams=renewalparams, error_expected=True, assert_oc_called=False) def test_renew_with_bad_domain(self): renewalparams = {'authenticator': 'webroot'} names = ['*.example.com'] - self._test_renew_common(renewalparams=renewalparams, + self._test_renew_common(renewalparams=renewalparams, error_expected=True, names=names, assert_oc_called=False) def test_renew_plugin_config_restoration(self): @@ -695,7 +708,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods # pylint: disable=protected-access with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: mock_reconstitute.side_effect = Exception - self._test_renew_common(assert_oc_called=False) + self._test_renew_common(assert_oc_called=False, error_expected=True) def test_renew_obtain_cert_error(self): self._make_dummy_renewal_config() @@ -707,15 +720,14 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods 'renewalparams': {'authenticator': 'webroot'}} with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: mock_obtain_cert.side_effect = Exception - self._test_renewal_common(True, None, + self._test_renewal_common(True, None, error_expected=True, args=['renew'], renew=False) def test_renew_with_bad_cli_args(self): - self.assertRaises(errors.Error, self._test_renewal_common, True, None, - args='renew -d example.com'.split(), renew=False) - self.assertRaises(errors.Error, self._test_renewal_common, True, None, - args='renew --csr {0}'.format(CSR).split(), - renew=False) + self._test_renewal_common(True, None, args='renew -d example.com'.split(), + renew=False, error_expected=True) + self._test_renewal_common(True, None, args='renew --csr {0}'.format(CSR).split(), + renew=False, error_expected=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal')