[Windows] Fix some unit tests (#6865)

This PR is a part of the effort to remove the last broken unit tests in certbot codebase for Windows, as described in #6850.

This PR fixes various unit tests on Windows, whose resolution was only to modify some logic in the tests, or minor changes in certbot codecase impacting Windows only (like handling correctly paths with DOS-style).

* Correct several tests

* Skip test definitively

* Test to be reactivated with #6497

* Mock log system to avoid errors due to multiple calls to main in main_test

* Simplify mock

* Update cli_test.py

* One test to be repaired when windows file permissions PR is merged
This commit is contained in:
Adrien Ferrand 2019-03-25 20:56:28 +01:00 committed by Brad Warren
parent c1d2efec4e
commit 537bffbc23
4 changed files with 44 additions and 44 deletions

View file

@ -106,10 +106,10 @@ class AuthenticatorTest(test_util.TempDirTestCase):
achall.validation(achall.account_key) in args[0])
self.assertFalse(kwargs['wrap'])
@test_util.broken_on_windows
def test_cleanup(self):
self.config.manual_public_ip_logging_ok = True
self.config.manual_auth_hook = 'echo foo;'
self.config.manual_auth_hook = ('{0} -c "import sys; sys.stdout.write(\'foo\')"'
.format(sys.executable))
self.config.manual_cleanup_hook = '# cleanup'
self.auth.perform(self.achalls)

View file

@ -86,26 +86,26 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods
return output.getvalue()
@test_util.broken_on_windows
@mock.patch("certbot.cli.flag_default")
def test_cli_ini_domains(self, mock_flag_default):
tmp_config = tempfile.NamedTemporaryFile()
# use a shim to get ConfigArgParse to pick up tmp_config
shim = (
lambda v: copy.deepcopy(constants.CLI_DEFAULTS[v])
if v != "config_files"
else [tmp_config.name]
)
mock_flag_default.side_effect = shim
with tempfile.NamedTemporaryFile() as tmp_config:
tmp_config.close() # close now because of compatibility issues on Windows
# use a shim to get ConfigArgParse to pick up tmp_config
shim = (
lambda v: copy.deepcopy(constants.CLI_DEFAULTS[v])
if v != "config_files"
else [tmp_config.name]
)
mock_flag_default.side_effect = shim
namespace = self.parse(["certonly"])
self.assertEqual(namespace.domains, [])
tmp_config.write(b"domains = example.com")
tmp_config.flush()
namespace = self.parse(["certonly"])
self.assertEqual(namespace.domains, ["example.com"])
namespace = self.parse(["renew"])
self.assertEqual(namespace.domains, [])
namespace = self.parse(["certonly"])
self.assertEqual(namespace.domains, [])
with open(tmp_config.name, 'w') as file_h:
file_h.write("domains = example.com")
namespace = self.parse(["certonly"])
self.assertEqual(namespace.domains, ["example.com"])
namespace = self.parse(["renew"])
self.assertEqual(namespace.domains, [])
def test_no_args(self):
namespace = self.parse([])

View file

@ -41,7 +41,6 @@ class LockFileTest(test_util.TempDirTestCase):
super(LockFileTest, self).setUp()
self.lock_path = os.path.join(self.tempdir, 'test.lock')
@test_util.broken_on_windows
def test_acquire_without_deletion(self):
# acquire the lock in another process but don't delete the file
child = multiprocessing.Process(target=self._call,
@ -59,12 +58,14 @@ class LockFileTest(test_util.TempDirTestCase):
self.assertRaises, errors.LockError, self._call, self.lock_path)
test_util.lock_and_call(assert_raises, self.lock_path)
@test_util.broken_on_windows
def test_locked_repr(self):
lock_file = self._call(self.lock_path)
locked_repr = repr(lock_file)
self._test_repr_common(lock_file, locked_repr)
self.assertTrue('acquired' in locked_repr)
try:
locked_repr = repr(lock_file)
self._test_repr_common(lock_file, locked_repr)
self.assertTrue('acquired' in locked_repr)
finally:
lock_file.release()
def test_released_repr(self):
lock_file = self._call(self.lock_path)
@ -94,7 +95,6 @@ class LockFileTest(test_util.TempDirTestCase):
self._call(self.lock_path)
self.assertFalse(should_delete)
@test_util.broken_on_windows
def test_removed(self):
lock_file = self._call(self.lock_path)
lock_file.release()

View file

@ -593,20 +593,20 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertTrue(message in str(exc))
self.assertTrue(exc is not None)
@test_util.broken_on_windows
def test_noninteractive(self):
@mock.patch('certbot.log.post_arg_parse_setup')
def test_noninteractive(self, _):
args = ['-n', 'certonly']
self._cli_missing_flag(args, "specify a plugin")
args.extend(['--standalone', '-d', 'eg.is'])
self._cli_missing_flag(args, "register before running")
@test_util.broken_on_windows
@mock.patch('certbot.log.post_arg_parse_setup')
@mock.patch('certbot.main._report_new_cert')
@mock.patch('certbot.main.client.acme_client.Client')
@mock.patch('certbot.main._determine_account')
@mock.patch('certbot.main.client.Client.obtain_and_enroll_certificate')
@mock.patch('certbot.main._get_and_save_cert')
def test_user_agent(self, gsc, _obt, det, _client, unused_report):
def test_user_agent(self, gsc, _obt, det, _client, _, __):
# Normally the client is totally mocked out, but here we need more
# arguments to automate it...
args = ["--standalone", "certonly", "-m", "none@none.com",
@ -655,11 +655,11 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertEqual(call_config.fullchain_path, test_util.temp_join('chain'))
self.assertEqual(call_config.key_path, test_util.temp_join('privkey'))
@test_util.broken_on_windows
@mock.patch('certbot.log.post_arg_parse_setup')
@mock.patch('certbot.main._install_cert')
@mock.patch('certbot.main.plug_sel.record_chosen_plugins')
@mock.patch('certbot.main.plug_sel.pick_installer')
def test_installer_param_override(self, _inst, _rec, mock_install):
def test_installer_param_override(self, _inst, _rec, mock_install, _):
mock_lineage = mock.MagicMock(cert_path=test_util.temp_join('cert'),
chain_path=test_util.temp_join('chain'),
fullchain_path=test_util.temp_join('chain'),
@ -706,10 +706,10 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertTrue(mock_getcert.called)
self.assertTrue(mock_inst.called)
@test_util.broken_on_windows
@mock.patch('certbot.log.post_arg_parse_setup')
@mock.patch('certbot.main._report_new_cert')
@mock.patch('certbot.util.exe_exists')
def test_configurator_selection(self, mock_exe_exists, unused_report):
def test_configurator_selection(self, mock_exe_exists, _, __):
mock_exe_exists.return_value = True
real_plugins = disco.PluginsRegistry.find_all()
args = ['--apache', '--authenticator', 'standalone']
@ -746,8 +746,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self._call(["auth", "--standalone"])
self.assertEqual(1, mock_certonly.call_count)
@test_util.broken_on_windows
def test_rollback(self):
@mock.patch('certbot.log.post_arg_parse_setup')
def test_rollback(self, _):
_, _, _, client = self._call(['rollback'])
self.assertEqual(1, client.rollback.call_count)
@ -774,8 +774,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self._call_no_clientmock(['delete'])
self.assertEqual(1, mock_cert_manager.call_count)
@test_util.broken_on_windows
def test_plugins(self):
@mock.patch('certbot.log.post_arg_parse_setup')
def test_plugins(self, _):
flags = ['--init', '--prepare', '--authenticators', '--installers']
for args in itertools.chain(
*(itertools.combinations(flags, r)
@ -1047,7 +1047,7 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
return mock_lineage, mock_get_utility, stdout
@mock.patch('certbot.crypto_util.notAfter')
def test_certonly_renewal(self, unused_notafter):
def test_certonly_renewal(self, _):
lineage, get_utility, _ = self._test_renewal_common(True, [])
self.assertEqual(lineage.save_successor.call_count, 1)
lineage.update_all_links_to.assert_called_once_with(
@ -1056,9 +1056,9 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertTrue('fullchain.pem' in cert_msg)
self.assertTrue('donate' in get_utility().add_message.call_args[0][0])
@test_util.broken_on_windows
@mock.patch('certbot.log.logging.handlers.RotatingFileHandler.doRollover')
@mock.patch('certbot.crypto_util.notAfter')
def test_certonly_renewal_triggers(self, unused_notafter):
def test_certonly_renewal_triggers(self, _, __):
# --dry-run should force renewal
_, get_utility, _ = self._test_renewal_common(False, ['--dry-run', '--keep'],
log_out="simulating renewal")
@ -1125,8 +1125,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertTrue('No renewals were attempted.' in stdout.getvalue())
self.assertTrue('The following certs are not due for renewal yet:' in stdout.getvalue())
@test_util.broken_on_windows
def test_quiet_renew(self):
@mock.patch('certbot.log.post_arg_parse_setup')
def test_quiet_renew(self, _):
test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
args = ["renew", "--dry-run"]
_, _, stdout = self._test_renewal_common(True, [], args=args, should_renew=True)
@ -1381,8 +1381,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self._call(['-c', test_util.vector_path('cli.ini')])
self.assertTrue(mocked_run.called)
@test_util.broken_on_windows
def test_register(self):
@mock.patch('certbot.log.post_arg_parse_setup')
def test_register(self, _):
with mock.patch('certbot.main.client') as mocked_client:
acc = mock.MagicMock()
acc.id = "imaginary_account"