From 685fa7768481585d6637efda38981c0832bbb425 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 15:02:52 -0800
Subject: [PATCH 01/23] Add --dry-run flag

---
 letsencrypt/cli.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 61bc85e72..bd77e23e0 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1207,6 +1207,10 @@ def _paths_parser(helpful):
     add("testing", "--test-cert", "--staging", action='store_true', dest='staging',
         help='Use the staging server to obtain test (invalid) certs; equivalent'
              ' to --server ' + constants.STAGING_URI)
+    add("testing", "--dry-run", action="store_true", dest="dry_run",
+        help="Perform a test run of the client, obtaining test (invalid) certs"
+             " but not saving them to disk. This can currently only be used"
+             " with the 'certonly' subcommand.")
 
 
 def _plugins_parsing(helpful, plugins):

From 6797009dd06da2765b6cbcd30e126f7993791a53 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 16:48:43 -0800
Subject: [PATCH 02/23] Simplified calls to prepare_and_parse_args

---
 letsencrypt/tests/cli_test.py | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index ca5830ed0..2a40a199a 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -1,5 +1,6 @@
 """Tests for letsencrypt.cli."""
 import argparse
+import functools
 import itertools
 import os
 import shutil
@@ -349,45 +350,49 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                           self._call,
                           ['-d', '*.wildcard.tld'])
 
-    def test_parse_domains(self):
+    def _get_argument_parser(self):
         plugins = disco.PluginsRegistry.find_all()
+        return functools.partial(cli.prepare_and_parse_args, plugins)
+
+    def test_parse_domains(self):
+        parse = self._get_argument_parser()
 
         short_args = ['-d', 'example.com']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
         short_args = ['-d', 'example.com,another.net,third.org,example.com']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net',
                                              'third.org'])
 
         long_args = ['--domains', 'example.com']
-        namespace = cli.prepare_and_parse_args(plugins, long_args)
+        namespace = parse(long_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
         long_args = ['--domains', 'example.com,another.net,example.com']
-        namespace = cli.prepare_and_parse_args(plugins, long_args)
+        namespace = parse(long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
 
     def test_parse_server(self):
-        plugins = disco.PluginsRegistry.find_all()
+        parse = self._get_argument_parser()
         short_args = ['--server', 'example.com']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
         self.assertEqual(namespace.server, 'example.com')
 
         short_args = ['--staging']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 
         short_args = ['--staging', '--server', 'example.com']
-        self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args)
+        self.assertRaises(errors.Error, parse, short_args)
 
     def test_parse_webroot(self):
-        plugins = disco.PluginsRegistry.find_all()
+        parse = self._get_argument_parser()
         webroot_args = ['--webroot', '-w', '/var/www/example',
             '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
             '-d', 'superfluo.us', '-d', 'www.superfluo.us']
-        namespace = cli.prepare_and_parse_args(plugins, webroot_args)
+        namespace = parse(webroot_args)
         self.assertEqual(namespace.webroot_map, {
             'example.com': '/var/www/example',
             'www.example.com': '/var/www/example',
@@ -395,10 +400,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             'superfluo.us': '/var/www/superfluous'})
 
         webroot_args = ['-d', 'stray.example.com'] + webroot_args
-        self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args)
+        self.assertRaises(errors.Error, parse, webroot_args)
 
         webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
-        namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
+        namespace = parse(webroot_map_args)
         self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})
 
     @mock.patch('letsencrypt.cli._suggest_donate')

From 5c528849393157b85f8574b404858182285fa22c Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 17:41:47 -0800
Subject: [PATCH 03/23] Refactor --server/--staging tests to simplify --dry-run
 tests

---
 letsencrypt/tests/cli_test.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 2a40a199a..e9bb8971d 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -374,18 +374,30 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = parse(long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
 
-    def test_parse_server(self):
+    def test_server_flag(self):
         parse = self._get_argument_parser()
-        short_args = ['--server', 'example.com']
-        namespace = parse(short_args)
+        namespace = parse('--server example.com'.split())
         self.assertEqual(namespace.server, 'example.com')
 
+    def _check_server_conflict_message(self, parser_args, conflicting_args):
+        parse = self._get_argument_parser()
+        try:
+            parse(parser_args)
+            self.fail("The following flags didn't conflict with "
+                      '--server: {0}'.format(', '.join(conflicting_args)))
+        except errors.Error as error:
+            self.assertTrue('--server' in error.message)
+            for arg in conflicting_args:
+                self.assertTrue(arg in error.message)
+
+    def test_staging_flag(self):
+        parse = self._get_argument_parser()
         short_args = ['--staging']
         namespace = parse(short_args)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 
-        short_args = ['--staging', '--server', 'example.com']
-        self.assertRaises(errors.Error, parse, short_args)
+        short_args += '--server example.com'.split()
+        self._check_server_conflict_message(short_args, '--staging')
 
     def test_parse_webroot(self):
         parse = self._get_argument_parser()

From 3d840dc11d087ec8ca8e9cc9e475ba1630bc93cf Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 18:00:28 -0800
Subject: [PATCH 04/23] Add --dry-run parsing

---
 letsencrypt/cli.py            | 19 ++++++++++++++-----
 letsencrypt/tests/cli_test.py | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index bd77e23e0..25f82f7b4 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -827,15 +827,24 @@ class HelpfulArgumentParser(object):
         parsed_args.verb = self.verb
 
         # Do any post-parsing homework here
+        if parsed_args.staging or parsed_args.dry_run:
+            if (parsed_args.server not in
+                    (flag_default("server"), constants.STAGING_URI)):
+                conflicts = ["--staging"] if parsed_args.staging else []
+                if parsed_args.dry_run:
+                    conflicts.append("--dry-run")
+                raise errors.Error("--server value conflicts with {0}".format(
+                    " and ".join(conflicts)))
 
-        # argparse seemingly isn't flexible enough to give us this behaviour easily...
-        if parsed_args.staging:
-            if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
-                raise errors.Error("--server value conflicts with --staging")
             parsed_args.server = constants.STAGING_URI
 
-        return parsed_args
+            if parsed_args.dry_run:
+                if self.verb != "certonly":
+                    raise errors.Error("--dry-run currently only works with the "
+                                       "'certonly' subcommand")
+                parsed_args.staging = True
 
+        return parsed_args
 
     def determine_verb(self):
         """Determines the verb/subcommand provided by the user.
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index e9bb8971d..24c5c5c82 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -394,11 +394,34 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         parse = self._get_argument_parser()
         short_args = ['--staging']
         namespace = parse(short_args)
+        self.assertTrue(namespace.staging)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 
         short_args += '--server example.com'.split()
         self._check_server_conflict_message(short_args, '--staging')
 
+    def _assert_dry_run_flag_worked(self, namespace):
+        self.assertTrue(namespace.dry_run)
+        self.assertTrue(namespace.staging)
+        self.assertEqual(namespace.server, constants.STAGING_URI)
+
+    def test_dry_run_flag(self):
+        parse = self._get_argument_parser()
+        short_args = ['--dry-run']
+        self.assertRaises(errors.Error, parse, short_args)
+
+        self._assert_dry_run_flag_worked(parse(short_args + ['auth']))
+        short_args += ['certonly']
+        self._assert_dry_run_flag_worked(parse(short_args))
+
+        short_args += '--server example.com'.split()
+        conflicts = ['--dry-run']
+        self._check_server_conflict_message(short_args, '--dry-run')
+
+        short_args += ['--staging']
+        conflicts += ['--staging']
+        self._check_server_conflict_message(short_args, conflicts)
+
     def test_parse_webroot(self):
         parse = self._get_argument_parser()
         webroot_args = ['--webroot', '-w', '/var/www/example',

From d56d15225ec4ced0ea2a2b1b3ee3bfe10ea8c88a Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 18:09:59 -0800
Subject: [PATCH 05/23] Add --dry-run support to obtain_and_enroll_cert

---
 letsencrypt/client.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/client.py b/letsencrypt/client.py
index c2dfca1bf..54bf21d87 100644
--- a/letsencrypt/client.py
+++ b/letsencrypt/client.py
@@ -276,8 +276,8 @@ class Client(object):
         :param plugins: A PluginsFactory object.
 
         :returns: A new :class:`letsencrypt.storage.RenewableCert` instance
-            referred to the enrolled cert lineage, or False if the cert could
-            not be obtained.
+            referred to the enrolled cert lineage, False if the cert could not
+            be obtained, or None if doing a successful dry run.
 
         """
         certr, chain, key, _ = self.obtain_certificate(domains)
@@ -298,12 +298,14 @@ class Client(object):
                 "Non-standard path(s), might not work with crontab installed "
                 "by your operating system package manager")
 
-        lineage = storage.RenewableCert.new_lineage(
-            domains[0], OpenSSL.crypto.dump_certificate(
-                OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
-            key.pem, crypto_util.dump_pyopenssl_chain(chain),
-            params, config, cli_config)
-        return lineage
+        if cli_config.dry_run:
+            return None
+        else:
+            return storage.RenewableCert.new_lineage(
+                domains[0], OpenSSL.crypto.dump_certificate(
+                    OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
+                key.pem, crypto_util.dump_pyopenssl_chain(chain),
+                params, config, cli_config)
 
     def save_certificate(self, certr, chain_cert,
                          cert_path, chain_path, fullchain_path):

From 0db36afa09b6b8cf90c1d1d674fdcb23e3a6443f Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 18:14:13 -0800
Subject: [PATCH 06/23] Add --dry-run support when getting a new cert

---
 letsencrypt/cli.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 25f82f7b4..badeebd8f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -416,13 +416,15 @@ def _auth_from_domains(le_client, config, domains):
     elif action == "newcert":
         # TREAT AS NEW REQUEST
         lineage = le_client.obtain_and_enroll_certificate(domains)
-        if not lineage:
+        if lineage is False:
             raise errors.Error("Certificate could not be obtained")
 
-    _report_new_cert(lineage.cert, lineage.fullchain)
+    if lineage is not None:
+        _report_new_cert(lineage.cert, lineage.fullchain)
 
     return lineage, action
 
+
 def _avoid_invalidating_lineage(config, lineage, original_server):
     "Do not renew a valid cert with one from a staging server!"
     def _is_staging(srv):

From 688b92f52874c12e557a19990f27eb3cd8e824ef Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 18:19:01 -0800
Subject: [PATCH 07/23] Add --dry-run support when renewing a lineage

---
 letsencrypt/cli.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index badeebd8f..cc0c99d86 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -404,12 +404,13 @@ def _auth_from_domains(le_client, config, domains):
         # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574
         new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
         # TODO: Check whether it worked! <- or make sure errors are thrown (jdk)
-        lineage.save_successor(
-            lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
-                OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
-            new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain))
+        if not config.dry_run:
+            lineage.save_successor(
+                lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
+                    OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
+                new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain))
 
-        lineage.update_all_links_to(lineage.latest_common_version())
+            lineage.update_all_links_to(lineage.latest_common_version())
         # TODO: Check return value of save_successor
         # TODO: Also update lineage renewal config with any relevant
         #       configuration values from this attempt? <- Absolutely (jdkasten)
@@ -419,7 +420,7 @@ def _auth_from_domains(le_client, config, domains):
         if lineage is False:
             raise errors.Error("Certificate could not be obtained")
 
-    if lineage is not None:
+    if not config.dry_run:
         _report_new_cert(lineage.cert, lineage.fullchain)
 
     return lineage, action

From c816bfd0b7feb07411896bd8095dfe19b70bf822 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Thu, 28 Jan 2016 18:24:47 -0800
Subject: [PATCH 08/23] --dry-run implies --break-my-certs

---
 letsencrypt/cli.py            | 2 +-
 letsencrypt/tests/cli_test.py | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cc0c99d86..41e123a52 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -845,7 +845,7 @@ class HelpfulArgumentParser(object):
                 if self.verb != "certonly":
                     raise errors.Error("--dry-run currently only works with the "
                                        "'certonly' subcommand")
-                parsed_args.staging = True
+                parsed_args.break_my_certs = parsed_args.staging = True
 
         return parsed_args
 
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 24c5c5c82..ab7d8b025 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -402,6 +402,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def _assert_dry_run_flag_worked(self, namespace):
         self.assertTrue(namespace.dry_run)
+        self.assertTrue(namespace.break_my_certs)
         self.assertTrue(namespace.staging)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 

From a2cca2050046a2ed3e9f6646553adae460da8589 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 10:47:58 -0800
Subject: [PATCH 09/23] Add --dry-run support when using custom csr

---
 letsencrypt/cli.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 41e123a52..9295144a5 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -634,9 +634,10 @@ def obtain_cert(args, config, plugins):
     if args.csr is not None:
         certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
             file=args.csr[0], data=args.csr[1], form="der"))
-        cert_path, _, cert_fullchain = le_client.save_certificate(
-            certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
-        _report_new_cert(cert_path, cert_fullchain)
+        if not args.dry_run:
+            cert_path, _, cert_fullchain = le_client.save_certificate(
+                certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
+            _report_new_cert(cert_path, cert_fullchain)
     else:
         domains = _find_domains(args, installer)
         _auth_from_domains(le_client, config, domains)

From 639444bf5c3243627c2d4737d64e02f50377b837 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 12:07:38 -0800
Subject: [PATCH 10/23] Add message on successful dry run

---
 letsencrypt/cli.py            | 11 ++++++++++-
 letsencrypt/tests/cli_test.py | 32 +++++++++++++++++++++++---------
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9295144a5..15a483b68 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -383,6 +383,12 @@ def _suggest_donate():
     reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
 
 
+def _report_successful_dry_run():
+    reporter_util = zope.component.getUtility(interfaces.IReporter)
+    reporter_util.add_message("The dry run was successful.",
+                              reporter_util.HIGH_PRIORITY, on_crash=False)
+
+
 def _auth_from_domains(le_client, config, domains):
     """Authenticate and enroll certificate."""
     # Note: This can raise errors... caught above us though. This is now
@@ -642,7 +648,10 @@ def obtain_cert(args, config, plugins):
         domains = _find_domains(args, installer)
         _auth_from_domains(le_client, config, domains)
 
-    _suggest_donate()
+    if args.dry_run:
+        _report_successful_dry_run()
+    else:
+        _suggest_donate()
 
 
 def install(args, config, plugins):
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index ab7d8b025..b95e47987 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -383,8 +383,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         parse = self._get_argument_parser()
         try:
             parse(parser_args)
-            self.fail("The following flags didn't conflict with "
-                      '--server: {0}'.format(', '.join(conflicting_args)))
+            self.fail(  # pragma: no cover
+                "The following flags didn't conflict with "
+                '--server: {0}'.format(', '.join(conflicting_args)))
         except errors.Error as error:
             self.assertTrue('--server' in error.message)
             for arg in conflicting_args:
@@ -442,6 +443,26 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = parse(webroot_map_args)
         self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})
 
+    def _certonly_new_request_common(self, mock_client, args=None):
+        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
+            mock_renewal.return_value = ("newcert", None)
+            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+                mock_init.return_value = mock_client
+                if args is None:
+                    args = []
+                args += '-d foo.bar -a standalone certonly'.split()
+                self._call(args)
+
+    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    def test_certonly_dry_run_success(self, mock_get_utility):
+        mock_client = mock.MagicMock()
+        mock_client.obtain_and_enroll_certificate.return_value = None
+        self._certonly_new_request_common(mock_client, ['--dry-run'])
+        self.assertEqual(
+            mock_client.obtain_and_enroll_certificate.call_count, 1)
+        self.assertTrue(
+            'dry run' in mock_get_utility().add_message.call_args[0][0])
+
     @mock.patch('letsencrypt.cli._suggest_donate')
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
@@ -467,13 +488,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertRaises(errors.Error,
                           self._certonly_new_request_common, mock_client)
 
-    def _certonly_new_request_common(self, mock_client):
-        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
-            mock_renewal.return_value = ("newcert", None)
-            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
-                mock_init.return_value = mock_client
-                self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
-
     @mock.patch('letsencrypt.cli._suggest_donate')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._treat_as_renewal')

From 08698a3de2da1ad1c77771073e47054c3e16ce64 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 17:25:33 -0800
Subject: [PATCH 11/23] Log when skipping functions due to --dry-run in cli.py

---
 letsencrypt/cli.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 15a483b68..db8cd722d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -410,7 +410,10 @@ def _auth_from_domains(le_client, config, domains):
         # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574
         new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
         # TODO: Check whether it worked! <- or make sure errors are thrown (jdk)
-        if not config.dry_run:
+        if config.dry_run:
+            logger.info("Dry run: skipping updating lineage at %s",
+                        os.path.dirname(lineage.cert))
+        else:
             lineage.save_successor(
                 lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
                     OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
@@ -640,7 +643,10 @@ def obtain_cert(args, config, plugins):
     if args.csr is not None:
         certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
             file=args.csr[0], data=args.csr[1], form="der"))
-        if not args.dry_run:
+        if args.dry_run:
+            logger.info(
+                "Dry run: skipping saving certificate to %s", args.cert_path)
+        else:
             cert_path, _, cert_fullchain = le_client.save_certificate(
                 certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
             _report_new_cert(cert_path, cert_fullchain)

From 5c363b5b984c88f62716e7643eaa629bd227c2fd Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 17:43:21 -0800
Subject: [PATCH 12/23] Log when skipping functions due to --dry-run in
 client.py

---
 letsencrypt/client.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/letsencrypt/client.py b/letsencrypt/client.py
index 54bf21d87..0b258b7eb 100644
--- a/letsencrypt/client.py
+++ b/letsencrypt/client.py
@@ -299,6 +299,8 @@ class Client(object):
                 "by your operating system package manager")
 
         if cli_config.dry_run:
+            logger.info("Dry run: Skipping creating new lineage for %s",
+                        domains[0])
             return None
         else:
             return storage.RenewableCert.new_lineage(

From 29bed26aa14bdb71b8ce388a01eadb14a5e1dda5 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 18:04:51 -0800
Subject: [PATCH 13/23] Make addition of conflicting flags look similar

---
 letsencrypt/cli.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index db8cd722d..6f9d9dbc7 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -850,8 +850,7 @@ class HelpfulArgumentParser(object):
             if (parsed_args.server not in
                     (flag_default("server"), constants.STAGING_URI)):
                 conflicts = ["--staging"] if parsed_args.staging else []
-                if parsed_args.dry_run:
-                    conflicts.append("--dry-run")
+                conflicts += ["--dry-run"] if parsed_args.dry_run else []
                 raise errors.Error("--server value conflicts with {0}".format(
                     " and ".join(conflicts)))
 

From f9ac25cd7e333cc250d5e397160e71e461a0c3d7 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 18:14:53 -0800
Subject: [PATCH 14/23] Only suggest donations sometimes

---
 letsencrypt/cli.py            | 21 +++++++++++----------
 letsencrypt/tests/cli_test.py | 12 ++++++------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6f9d9dbc7..1fd209d69 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -374,13 +374,15 @@ def _report_new_cert(cert_path, fullchain_path):
            .format(and_chain, path, expiry))
     reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY)
 
-def _suggest_donate():
-    "Suggest a donation to support Let's Encrypt"
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n"
-           "Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate\n"
-           "Donating to EFF:                    https://eff.org/donate-le\n\n")
-    reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
+
+def _suggest_donation_if_appropriate(config):
+    """Potentially suggest a donation to support Let's Encrypt."""
+    if not config.staging:  # --dry-run implies --staging
+        reporter_util = zope.component.getUtility(interfaces.IReporter)
+        msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n"
+               "Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate\n"
+               "Donating to EFF:                    https://eff.org/donate-le\n\n")
+        reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
 
 
 def _report_successful_dry_run():
@@ -620,7 +622,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     else:
         display_ops.success_renewal(domains, action)
 
-    _suggest_donate()
+    _suggest_donation_if_appropriate(config)
 
 
 def obtain_cert(args, config, plugins):
@@ -656,8 +658,7 @@ def obtain_cert(args, config, plugins):
 
     if args.dry_run:
         _report_successful_dry_run()
-    else:
-        _suggest_donate()
+    _suggest_donation_if_appropriate(config)
 
 
 def install(args, config, plugins):
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index b95e47987..bb593437b 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -50,7 +50,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def _call(self, args):
         "Run the cli with output streams and actual client mocked out"
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
             with mock.patch('letsencrypt.cli.client') as client:
                 ret, stdout, stderr = self._call_no_clientmock(args)
                 return ret, stdout, stderr, client
@@ -58,7 +58,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     def _call_no_clientmock(self, args):
         "Run the client with output streams mocked out"
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
             with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
                 with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
                     ret = cli.main(args[:])  # NOTE: parser can alter its args!
@@ -70,7 +70,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         caller.
         """
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
             with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
                 with mock.patch('letsencrypt.cli.client') as client:
                     ret = cli.main(args[:])  # NOTE: parser can alter its args!
@@ -463,7 +463,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertTrue(
             'dry run' in mock_get_utility().add_message.call_args[0][0])
 
-    @mock.patch('letsencrypt.cli._suggest_donate')
+    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter, _suggest):
@@ -488,7 +488,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertRaises(errors.Error,
                           self._certonly_new_request_common, mock_client)
 
-    @mock.patch('letsencrypt.cli._suggest_donate')
+    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._treat_as_renewal')
     @mock.patch('letsencrypt.cli._init_le_client')
@@ -514,7 +514,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertTrue(
             chain_path in mock_get_utility().add_message.call_args[0][0])
 
-    @mock.patch('letsencrypt.cli._suggest_donate')
+    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.display_ops.pick_installer')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')

From ae6e938744be95b7cf9647272c3004216eb3c955 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Fri, 29 Jan 2016 18:50:38 -0800
Subject: [PATCH 15/23] --dry-run forces simulated renewal

---
 letsencrypt/cli.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 1fd209d69..6d55e9674 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -401,11 +401,20 @@ def _auth_from_domains(le_client, config, domains):
     # (which results in treating the request as a new certificate request).
 
     action, lineage = _treat_as_renewal(config, domains)
-    if action == "reinstall":
+    if action == "reinstall" and not config.dry_run:
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.
         return lineage, "reinstall"
-    elif action == "renew":
+    elif action == "newcert":
+        # TREAT AS NEW REQUEST
+        lineage = le_client.obtain_and_enroll_certificate(domains)
+        if lineage is False:
+            raise errors.Error("Certificate could not be obtained")
+    else:
+        assert action == "renew" or config.dry_run, "invalid auth command"
+        if config.dry_run and action == "reinstall":
+            logger.info("Cert not due for renewal, but "
+                        "simulating renewal for dry run")
         original_server = lineage.configuration["renewalparams"]["server"]
         _avoid_invalidating_lineage(config, lineage, original_server)
         # TODO: schoen wishes to reuse key - discussion
@@ -425,11 +434,6 @@ def _auth_from_domains(le_client, config, domains):
         # TODO: Check return value of save_successor
         # TODO: Also update lineage renewal config with any relevant
         #       configuration values from this attempt? <- Absolutely (jdkasten)
-    elif action == "newcert":
-        # TREAT AS NEW REQUEST
-        lineage = le_client.obtain_and_enroll_certificate(domains)
-        if lineage is False:
-            raise errors.Error("Certificate could not be obtained")
 
     if not config.dry_run:
         _report_new_cert(lineage.cert, lineage.fullchain)

From f5fa64ee9a23fc8a6dded29c2ea895653ebf30e6 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 12:01:12 -0800
Subject: [PATCH 16/23] Test _suggest_donation_if_appropriate

---
 letsencrypt/tests/cli_test.py | 50 +++++++++++++++++------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index c94336e66..b52ca6163 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -50,18 +50,16 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def _call(self, args):
         "Run the cli with output streams and actual client mocked out"
-        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
-            with mock.patch('letsencrypt.cli.client') as client:
-                ret, stdout, stderr = self._call_no_clientmock(args)
-                return ret, stdout, stderr, client
+        with mock.patch('letsencrypt.cli.client') as client:
+            ret, stdout, stderr = self._call_no_clientmock(args)
+            return ret, stdout, stderr, client
 
     def _call_no_clientmock(self, args):
         "Run the client with output streams mocked out"
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
-            with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
-                with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
-                    ret = cli.main(args[:])  # NOTE: parser can alter its args!
+        with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
+            with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
+                ret = cli.main(args[:])  # NOTE: parser can alter its args!
         return ret, stdout, stderr
 
     def _call_stdout(self, args):
@@ -70,10 +68,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         caller.
         """
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'):
-            with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
-                with mock.patch('letsencrypt.cli.client') as client:
-                    ret = cli.main(args[:])  # NOTE: parser can alter its args!
+        with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
+            with mock.patch('letsencrypt.cli.client') as client:
+                ret = cli.main(args[:])  # NOTE: parser can alter its args!
         return ret, None, stderr, client
 
     def test_no_flags(self):
@@ -505,11 +502,12 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             mock_client.obtain_and_enroll_certificate.call_count, 1)
         self.assertTrue(
             'dry run' in mock_get_utility().add_message.call_args[0][0])
+        # Asserts we don't suggest donating after a successful dry run
+        self.assertEqual(mock_get_utility().add_message.call_count, 1)
 
-    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
-    def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter, _suggest):
+    def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter):
         cert_path = '/etc/letsencrypt/live/foo.bar'
         date = '1970-01-01'
         mock_notAfter().date.return_value = date
@@ -520,10 +518,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self._certonly_new_request_common(mock_client)
         self.assertEqual(
             mock_client.obtain_and_enroll_certificate.call_count, 1)
+        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue(cert_path in cert_msg)
+        self.assertTrue(date in cert_msg)
         self.assertTrue(
-            cert_path in mock_get_utility().add_message.call_args[0][0])
-        self.assertTrue(
-            date in mock_get_utility().add_message.call_args[0][0])
+            'donate' in mock_get_utility().add_message.call_args[0][0])
 
     def test_certonly_new_request_failure(self):
         mock_client = mock.MagicMock()
@@ -531,11 +530,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertRaises(errors.Error,
                           self._certonly_new_request_common, mock_client)
 
-    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._treat_as_renewal')
     @mock.patch('letsencrypt.cli._init_le_client')
-    def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility, _suggest):
+    def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility):
         cert_path = 'letsencrypt/tests/testdata/cert.pem'
         chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
 
@@ -554,17 +552,18 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(mock_lineage.save_successor.call_count, 1)
         mock_lineage.update_all_links_to.assert_called_once_with(
             mock_lineage.latest_common_version())
+        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue(chain_path in cert_msg)
         self.assertTrue(
-            chain_path in mock_get_utility().add_message.call_args[0][0])
+            'donate' in mock_get_utility().add_message.call_args[0][0])
 
-    @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate')
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.display_ops.pick_installer')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._init_le_client')
     @mock.patch('letsencrypt.cli.record_chosen_plugins')
     def test_certonly_csr(self, _rec, mock_init, mock_get_utility,
-                          mock_pick_installer, mock_notAfter, _suggest):
+                          mock_pick_installer, mock_notAfter):
         cert_path = '/etc/letsencrypt/live/blahcert.pem'
         date = '1970-01-01'
         mock_notAfter().date.return_value = date
@@ -583,10 +582,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(mock_pick_installer.call_args[0][1], installer)
         mock_client.save_certificate.assert_called_once_with(
             'certr', 'chain', cert_path, '/', '/')
+        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue(cert_path in cert_msg)
+        self.assertTrue(date in cert_msg)
         self.assertTrue(
-            cert_path in mock_get_utility().add_message.call_args[0][0])
-        self.assertTrue(
-            date in mock_get_utility().add_message.call_args[0][0])
+            'donate' in mock_get_utility().add_message.call_args[0][0])
 
     @mock.patch('letsencrypt.cli.client.acme_client')
     def test_revoke_with_key(self, mock_acme_client):

From 32034552fd69e732617a582e75a6ef9943125a80 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 12:18:43 -0800
Subject: [PATCH 17/23] Test reinstall

---
 letsencrypt/tests/cli_test.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index b52ca6163..cce7ca201 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -494,7 +494,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 self._call(args)
 
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
-    def test_certonly_dry_run_success(self, mock_get_utility):
+    def test_certonly_dry_run_new_request_success(self, mock_get_utility):
         mock_client = mock.MagicMock()
         mock_client.obtain_and_enroll_certificate.return_value = None
         self._certonly_new_request_common(mock_client, ['--dry-run'])
@@ -557,6 +557,18 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertTrue(
             'donate' in mock_get_utility().add_message.call_args[0][0])
 
+    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    @mock.patch('letsencrypt.cli._treat_as_renewal')
+    @mock.patch('letsencrypt.cli._init_le_client')
+    def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility):
+        mock_renewal.return_value = ('reinstall', mock.MagicMock())
+        mock_init.return_value = mock_client = mock.MagicMock()
+        self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
+        self.assertFalse(mock_client.obtain_certificate.called)
+        self.assertFalse(mock_client.obtain_and_enroll_certificate.called)
+        self.assertTrue(
+            'donate' in mock_get_utility().add_message.call_args[0][0])
+
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.display_ops.pick_installer')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')

From 204f8ba0f232634aade7b512bffc9d685ebedb09 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 12:45:13 -0800
Subject: [PATCH 18/23] Refactor test_certonly_renewal

---
 letsencrypt/tests/cli_test.py | 36 ++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index cce7ca201..b065a4590 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -530,30 +530,40 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertRaises(errors.Error,
                           self._certonly_new_request_common, mock_client)
 
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
-    @mock.patch('letsencrypt.cli._treat_as_renewal')
-    @mock.patch('letsencrypt.cli._init_le_client')
-    def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility):
+    def _test_certonly_renewal_common(self, renewal_verb, extra_args=None):
         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)
         mock_certr = mock.MagicMock()
         mock_key = mock.MagicMock(pem='pem_key')
-        mock_renewal.return_value = ("renew", mock_lineage)
-        mock_client = mock.MagicMock()
-        mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
-                                                       mock_key, 'csr')
-        mock_init.return_value = mock_client
-        with mock.patch('letsencrypt.cli.OpenSSL'):
-            with mock.patch('letsencrypt.cli.crypto_util'):
-                self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
+        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
+            mock_renewal.return_value = (renewal_verb, mock_lineage)
+            mock_client = mock.MagicMock()
+            mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
+                                                           mock_key, 'csr')
+            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+                mock_init.return_value = mock_client
+                get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
+                with mock.patch(get_utility_path) as mock_get_utility:
+                    with mock.patch('letsencrypt.cli.OpenSSL'):
+                        with mock.patch('letsencrypt.cli.crypto_util'):
+                            args = ['-d', 'foo.bar', '-a',
+                                    'standalone', 'certonly']
+                            if extra_args:
+                                args += extra_args
+                            self._call(args)
+
         mock_client.obtain_certificate.assert_called_once_with(['foo.bar'])
         self.assertEqual(mock_lineage.save_successor.call_count, 1)
         mock_lineage.update_all_links_to.assert_called_once_with(
             mock_lineage.latest_common_version())
         cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
         self.assertTrue(chain_path in cert_msg)
+
+        return mock_get_utility
+
+    def test_certonly_renewal(self):
+        mock_get_utility = self._test_certonly_renewal_common("renew")
         self.assertTrue(
             'donate' in mock_get_utility().add_message.call_args[0][0])
 

From f6a3355d28798c21abfedaec15c66c760d1878c1 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 12:55:41 -0800
Subject: [PATCH 19/23] test reinstall + dry-run = renew

---
 letsencrypt/tests/cli_test.py | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index b065a4590..35a0153a4 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -554,18 +554,23 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                             self._call(args)
 
         mock_client.obtain_certificate.assert_called_once_with(['foo.bar'])
-        self.assertEqual(mock_lineage.save_successor.call_count, 1)
-        mock_lineage.update_all_links_to.assert_called_once_with(
-            mock_lineage.latest_common_version())
-        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
-        self.assertTrue(chain_path in cert_msg)
 
-        return mock_get_utility
+        return mock_lineage, mock_get_utility
 
     def test_certonly_renewal(self):
-        mock_get_utility = self._test_certonly_renewal_common("renew")
-        self.assertTrue(
-            'donate' in mock_get_utility().add_message.call_args[0][0])
+        lineage, get_utility = self._test_certonly_renewal_common('renew')
+        self.assertEqual(lineage.save_successor.call_count, 1)
+        lineage.update_all_links_to.assert_called_once_with(
+            lineage.latest_common_version())
+        cert_msg = get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue('fullchain.pem' in cert_msg)
+        self.assertTrue('donate' in get_utility().add_message.call_args[0][0])
+
+    def test_certonly_dry_run_reinstall_is_renewal(self):
+        _, get_utility = self._test_certonly_renewal_common('reinstall',
+                                                            ['--dry-run'])
+        self.assertEqual(get_utility().add_message.call_count, 1)
+        self.assertTrue('dry run' in get_utility().add_message.call_args[0][0])
 
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._treat_as_renewal')

From 5e9d5efdcbfad75fe2a95b4dd22786ca6d9d2bfe Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 13:20:46 -0800
Subject: [PATCH 20/23] Refactor test_certonly_csr

---
 letsencrypt/tests/cli_test.py | 48 ++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 35a0153a4..84c976177 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -584,34 +584,36 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertTrue(
             'donate' in mock_get_utility().add_message.call_args[0][0])
 
-    @mock.patch('letsencrypt.crypto_util.notAfter')
-    @mock.patch('letsencrypt.cli.display_ops.pick_installer')
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
-    @mock.patch('letsencrypt.cli._init_le_client')
-    @mock.patch('letsencrypt.cli.record_chosen_plugins')
-    def test_certonly_csr(self, _rec, mock_init, mock_get_utility,
-                          mock_pick_installer, mock_notAfter):
-        cert_path = '/etc/letsencrypt/live/blahcert.pem'
-        date = '1970-01-01'
-        mock_notAfter().date.return_value = date
-
+    def _test_certonly_csr_common(self, extra_args=None):
+        certr = 'certr'
+        chain = 'chain'
         mock_client = mock.MagicMock()
-        mock_client.obtain_certificate_from_csr.return_value = ('certr',
-                                                                'chain')
+        mock_client.obtain_certificate_from_csr.return_value = (certr, chain)
+        cert_path = '/etc/letsencrypt/live/example.com/cert.pem'
         mock_client.save_certificate.return_value = cert_path, None, None
-        mock_init.return_value = mock_client
+        with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+            mock_init.return_value = mock_client
+            get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
+            with mock.patch(get_utility_path) as mock_get_utility:
+                chain_path = '/etc/letsencrypt/live/example.com/chain.pem'
+                full_path = '/etc/letsencrypt/live/example.com/fullchain.pem'
+                args = ('-a standalone certonly --csr {0} --cert-path {1} '
+                        '--chain-path {2} --fullchain-path {3}').format(
+                            CSR, cert_path, chain_path, full_path).split()
+                if extra_args:
+                    args += extra_args
+                with mock.patch('letsencrypt.cli.crypto_util'):
+                    self._call(args)
 
-        installer = 'installer'
-        self._call(
-            ['-a', 'standalone', '-i', installer, 'certonly', '--csr', CSR,
-             '--cert-path', cert_path, '--fullchain-path', '/',
-             '--chain-path', '/'])
-        self.assertEqual(mock_pick_installer.call_args[0][1], installer)
         mock_client.save_certificate.assert_called_once_with(
-            'certr', 'chain', cert_path, '/', '/')
+            certr, chain, cert_path, chain_path, full_path)
+
+        return mock_get_utility
+
+    def test_certonly_csr(self):
+        mock_get_utility = self._test_certonly_csr_common()
         cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
-        self.assertTrue(cert_path in cert_msg)
-        self.assertTrue(date in cert_msg)
+        self.assertTrue('cert.pem' in cert_msg)
         self.assertTrue(
             'donate' in mock_get_utility().add_message.call_args[0][0])
 

From 149ac79b8f4ceb7469bd0663c32425d2d5033124 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 13:29:18 -0800
Subject: [PATCH 21/23] Test certonly with --csr and --dry-run

---
 letsencrypt/tests/cli_test.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 84c976177..506e19ba8 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -605,8 +605,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 with mock.patch('letsencrypt.cli.crypto_util'):
                     self._call(args)
 
-        mock_client.save_certificate.assert_called_once_with(
-            certr, chain, cert_path, chain_path, full_path)
+        if '--dry-run' in args:
+            self.assertFalse(mock_client.save_certificate.called)
+        else:
+            mock_client.save_certificate.assert_called_once_with(
+                certr, chain, cert_path, chain_path, full_path)
 
         return mock_get_utility
 
@@ -617,6 +620,12 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertTrue(
             'donate' in mock_get_utility().add_message.call_args[0][0])
 
+    def test_certonly_csr_dry_run(self):
+        mock_get_utility = self._test_certonly_csr_common(['--dry-run'])
+        self.assertEqual(mock_get_utility().add_message.call_count, 1)
+        self.assertTrue(
+            'dry run' in mock_get_utility().add_message.call_args[0][0])
+
     @mock.patch('letsencrypt.cli.client.acme_client')
     def test_revoke_with_key(self, mock_acme_client):
         server = 'foo.bar'

From c4ce168001199d2793e416a143b12281622a69d9 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 17:42:43 -0800
Subject: [PATCH 22/23] Revert "--dry-run forces simulated renewal"

This reverts commit ae6e938744be95b7cf9647272c3004216eb3c955.
---
 letsencrypt/cli.py | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index f762076d4..db381715d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -405,20 +405,11 @@ def _auth_from_domains(le_client, config, domains):
     # (which results in treating the request as a new certificate request).
 
     action, lineage = _treat_as_renewal(config, domains)
-    if action == "reinstall" and not config.dry_run:
+    if action == "reinstall":
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.
         return lineage, "reinstall"
-    elif action == "newcert":
-        # TREAT AS NEW REQUEST
-        lineage = le_client.obtain_and_enroll_certificate(domains)
-        if lineage is False:
-            raise errors.Error("Certificate could not be obtained")
-    else:
-        assert action == "renew" or config.dry_run, "invalid auth command"
-        if config.dry_run and action == "reinstall":
-            logger.info("Cert not due for renewal, but "
-                        "simulating renewal for dry run")
+    elif action == "renew":
         original_server = lineage.configuration["renewalparams"]["server"]
         _avoid_invalidating_lineage(config, lineage, original_server)
         # TODO: schoen wishes to reuse key - discussion
@@ -438,6 +429,11 @@ def _auth_from_domains(le_client, config, domains):
         # TODO: Check return value of save_successor
         # TODO: Also update lineage renewal config with any relevant
         #       configuration values from this attempt? <- Absolutely (jdkasten)
+    elif action == "newcert":
+        # TREAT AS NEW REQUEST
+        lineage = le_client.obtain_and_enroll_certificate(domains)
+        if lineage is False:
+            raise errors.Error("Certificate could not be obtained")
 
     if not config.dry_run:
         _report_new_cert(lineage.cert, lineage.fullchain)

From d18ec15165623c333563c71cfbb8653152bcdcce Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Mon, 1 Feb 2016 17:46:55 -0800
Subject: [PATCH 23/23] Change action just in case

---
 letsencrypt/cli.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index db381715d..60580cff9 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -405,6 +405,11 @@ def _auth_from_domains(le_client, config, domains):
     # (which results in treating the request as a new certificate request).
 
     action, lineage = _treat_as_renewal(config, domains)
+    if config.dry_run and action == "reinstall":
+        logger.info(
+            "Cert not due for renewal, but simulating renewal for dry run")
+        action = "renew"
+
     if action == "reinstall":
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.