upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-05-28 04:34:11 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Use better asserts. Added notes to style guide. (#8451)

Browse source
This commit is contained in:
Mads Jensen 2020-11-12 23:33:02 +01:00 committed by GitHub
parent f15f4f9838
commit b742b60c4d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 56 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
acme/tests/messages_test.py
View file

@ -108,11 +108,11 @@ class ConstantTest(unittest.TestCase):
def test_equality(self):
const_a_prime = self.MockConstant('a')
self.assertFalse(self.const_a == self.const_b)
self.assertTrue(self.const_a == const_a_prime)
self.assertNotEqual(self.const_a, self.const_b)
self.assertEqual(self.const_a, const_a_prime)
self.assertTrue(self.const_a != self.const_b)
self.assertFalse(self.const_a != const_a_prime)
self.assertNotEqual(self.const_a, self.const_b)
self.assertEqual(self.const_a, const_a_prime)
class DirectoryTest(unittest.TestCase):

8
certbot-apache/tests/configurator_test.py
View file

@ -1357,10 +1357,10 @@ class MultipleVhostsTest(util.ApacheTest):
# And the actual returned values
self.assertEqual(len(vhs), 1)
self.assertTrue(vhs[0].name == "certbot.demo")
self.assertEqual(vhs[0].name, "certbot.demo")
self.assertTrue(vhs[0].ssl)
self.assertFalse(vhs[0] == self.vh_truth[3])
self.assertNotEqual(vhs[0], self.vh_truth[3])
@mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.make_vhost_ssl")
def test_choose_vhosts_wildcard_no_ssl(self, mock_makessl):
@ -1471,10 +1471,10 @@ class MultipleVhostsTest(util.ApacheTest):
self.config.parser.aug.match = mock_match
vhs = self.config.get_virtual_hosts()
self.assertEqual(len(vhs), 2)
self.assertTrue(vhs[0] == self.vh_truth[1])
self.assertEqual(vhs[0], self.vh_truth[1])
# mock_vhost should have replaced the vh_truth[0], because its filepath
# isn't a symlink
self.assertTrue(vhs[1] == mock_vhost)
self.assertEqual(vhs[1], mock_vhost)
class AugeasVhostsTest(util.ApacheTest):

6
certbot-apache/tests/dualnode_test.py
View file

@ -412,9 +412,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public-
ancestor=self.block,
filepath="/path/to/whatever",
metadata=self.metadata)
self.assertFalse(self.block == ne_block)
self.assertFalse(self.directive == ne_directive)
self.assertFalse(self.comment == ne_comment)
self.assertNotEqual(self.block, ne_block)
self.assertNotEqual(self.directive, ne_directive)
self.assertNotEqual(self.comment, ne_comment)
def test_parsed_paths(self):
mock_p = mock.MagicMock(return_value=['/path/file.conf',

20
certbot-apache/tests/obj_test.py
View file

@ -27,14 +27,14 @@ class VirtualHostTest(unittest.TestCase):
"certbot_apache._internal.obj.Addr(('127.0.0.1', '443'))")
def test_eq(self):
self.assertTrue(self.vhost1b == self.vhost1)
self.assertFalse(self.vhost1 == self.vhost2)
self.assertEqual(self.vhost1b, self.vhost1)
self.assertNotEqual(self.vhost1, self.vhost2)
self.assertEqual(str(self.vhost1b), str(self.vhost1))
self.assertFalse(self.vhost1b == 1234)
self.assertNotEqual(self.vhost1b, 1234)
def test_ne(self):
self.assertTrue(self.vhost1 != self.vhost2)
self.assertFalse(self.vhost1 != self.vhost1b)
self.assertNotEqual(self.vhost1, self.vhost2)
self.assertEqual(self.vhost1, self.vhost1b)
def test_conflicts(self):
from certbot_apache._internal.obj import Addr
@ -128,13 +128,13 @@ class AddrTest(unittest.TestCase):
self.assertTrue(self.addr1.conflicts(self.addr2))
def test_equal(self):
self.assertTrue(self.addr1 == self.addr2)
self.assertFalse(self.addr == self.addr1)
self.assertFalse(self.addr == 123)
self.assertEqual(self.addr1, self.addr2)
self.assertNotEqual(self.addr, self.addr1)
self.assertNotEqual(self.addr, 123)
def test_not_equal(self):
self.assertFalse(self.addr1 != self.addr2)
self.assertTrue(self.addr != self.addr1)
self.assertEqual(self.addr1, self.addr2)
self.assertNotEqual(self.addr, self.addr1)
if __name__ == "__main__":

2
certbot-dns-rfc2136/tests/dns_rfc2136_test.py
View file

@ -154,7 +154,7 @@ class RFC2136ClientTest(unittest.TestCase):
# _find_domain | pylint: disable=protected-access
domain = self.rfc2136_client._find_domain('foo.bar.'+DOMAIN)
self.assertTrue(domain == DOMAIN)
self.assertEqual(domain, DOMAIN)
def test_find_domain_wraps_errors(self):
# _query_soa | pylint: disable=protected-access

2
certbot-nginx/tests/configurator_test.py
View file

@ -842,7 +842,7 @@ class NginxConfiguratorTest(util.NginxTest):
self.config.recovery_routine()
self.config.revert_challenge_config()
self.config.rollback_checkpoints()
self.assertTrue(mock_parser_load.call_count == 3)
self.assertEqual(mock_parser_load.call_count, 3)
def test_choose_vhosts_wildcard(self):
# pylint: disable=protected-access

4
certbot-nginx/tests/obj_test.py
View file

@ -75,7 +75,7 @@ class AddrTest(unittest.TestCase):
new_addr1 = Addr.fromstring("192.168.1.1 spdy")
self.assertEqual(self.addr1, new_addr1)
self.assertNotEqual(self.addr1, self.addr2)
self.assertFalse(self.addr1 == 3333)
self.assertNotEqual(self.addr1, 3333)
def test_equivalent_any_addresses(self):
from certbot_nginx._internal.obj import Addr
@ -168,7 +168,7 @@ class VirtualHostTest(unittest.TestCase):
self.assertEqual(vhost1b, self.vhost1)
self.assertEqual(str(vhost1b), str(self.vhost1))
self.assertFalse(vhost1b == 1234)
self.assertNotEqual(vhost1b, 1234)
def test_str(self):
stringified = '\n'.join(['file: filep', 'addrs: localhost',

9
certbot/docs/contributing.rst
View file

@ -431,16 +431,23 @@ Please:
4. Remember to use ``pylint``.
5. You may consider installing a plugin for `editorconfig`_ in
your editor to prevent some linting warnings.
6. Please avoid `unittest.assertTrue` or `unittest.assertFalse` when
possible, and use `assertEqual` or more specific assert. They give
better messages when it's failing, and are generally more correct.
.. _Google Python Style Guide:
https://google.github.io/styleguide/pyguide.html
.. _Sphinx-style: https://www.sphinx-doc.org/
.. _PEP 8 - Style Guide for Python Code:
https://www.python.org/dev/peps/pep-0008
.. _editorconfig: https://editorconfig.org/
Use ``certbot.compat.os`` instead of ``os``
===========================================
Python's standard library ``os`` module lacks full support for several Windows
security features about file permissions (eg. DACLs). However several files
handled by Certbot (eg. private keys) need strongly restricted access

4
certbot/tests/auth_handler_test.py
View file

@ -510,7 +510,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
auth_handler._report_failed_authzrs([self.authzr1], 'key')
call_list = mock_zope().add_message.call_args_list
self.assertTrue(len(call_list) == 1)
self.assertEqual(len(call_list), 1)
self.assertTrue("Domain: example.com\nType: tls\nDetail: detail" in call_list[0][0][0])
@test_util.patch_get_utility()
@ -518,7 +518,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
from certbot._internal import auth_handler
auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key')
self.assertTrue(mock_zope().add_message.call_count == 2)
self.assertEqual(mock_zope().add_message.call_count, 2)
def gen_auth_resp(chall_list):

4
certbot/tests/display/ops_test.py
View file

@ -43,7 +43,7 @@ class GetEmailTest(unittest.TestCase):
mock_input.return_value = (display_util.OK, "foo@bar.baz")
with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email:
mock_safe_email.return_value = True
self.assertTrue(self._call() == "foo@bar.baz")
self.assertEqual(self._call(), "foo@bar.baz")
@test_util.patch_get_utility("certbot.display.ops.z_util")
def test_ok_not_safe(self, mock_get_utility):
@ -51,7 +51,7 @@ class GetEmailTest(unittest.TestCase):
mock_input.return_value = (display_util.OK, "foo@bar.baz")
with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email:
mock_safe_email.side_effect = [False, True]
self.assertTrue(self._call() == "foo@bar.baz")
self.assertEqual(self._call(), "foo@bar.baz")
@test_util.patch_get_utility("certbot.display.ops.z_util")
def test_invalid_flag(self, mock_get_utility):

30
certbot/tests/main_test.py
View file

@ -209,20 +209,21 @@ class CertonlyTest(unittest.TestCase):
mock_lineage.names.return_value = domains
self._call(('certonly --webroot -d example.com -d test.org '
'--cert-name example.com').split())
self.assertTrue(mock_lineage.call_count == 1)
self.assertTrue(mock_domains.call_count == 1)
self.assertTrue(mock_renew_cert.call_count == 1)
self.assertTrue(mock_report_cert.call_count == 1)
self.assertTrue(mock_handle_type.call_count == 1)
self.assertEqual(mock_lineage.call_count, 1)
self.assertEqual(mock_domains.call_count, 1)
self.assertEqual(mock_renew_cert.call_count, 1)
self.assertEqual(mock_report_cert.call_count, 1)
self.assertEqual(mock_handle_type.call_count, 1)
# user confirms updating lineage with new domains
self._call(('certonly --webroot -d example.com -d test.com '
'--cert-name example.com').split())
self.assertTrue(mock_lineage.call_count == 2)
self.assertTrue(mock_domains.call_count == 2)
self.assertTrue(mock_renew_cert.call_count == 2)
self.assertTrue(mock_report_cert.call_count == 2)
self.assertTrue(mock_handle_type.call_count == 2)
self.assertEqual(mock_lineage.call_count, 2)
self.assertEqual(mock_domains.call_count, 2)
self.assertEqual(mock_renew_cert.call_count, 2)
self.assertEqual(mock_report_cert.call_count, 2)
self.assertEqual(mock_handle_type.call_count, 2)
# error in _ask_user_to_confirm_new_names
self.mock_get_utility().yesno.return_value = False
@ -240,14 +241,15 @@ class CertonlyTest(unittest.TestCase):
# no lineage with this name but we specified domains so create a new cert
self._call(('certonly --webroot -d example.com -d test.com '
'--cert-name example.com').split())
self.assertTrue(mock_lineage.call_count == 1)
self.assertTrue(mock_report_cert.call_count == 1)
self.assertEqual(mock_lineage.call_count, 1)
self.assertEqual(mock_report_cert.call_count, 1)
# no lineage with this name and we didn't give domains
mock_choose_names.return_value = ["somename"]
mock_domains_for_certname.return_value = None
self._call(('certonly --webroot --cert-name example.com').split())
self.assertTrue(mock_choose_names.called)
self.assertIs(mock_choose_names.called, True)
class FindDomainsOrCertnameTest(unittest.TestCase):
"""Tests for certbot._internal.main._find_domains_or_certname."""
@ -755,7 +757,7 @@ class MainTest(test_util.ConfigTestCase):
# This needed two calls to find_all(), which we're avoiding for now
# because of possible side effects:
# https://github.com/letsencrypt/letsencrypt/commit/51ed2b681f87b1eb29088dd48718a54f401e4855
#with mock.patch('certbot._internal.cli.plugins_testable') as plugins:
# with mock.patch('certbot._internal.cli.plugins_testable') as plugins:
# plugins.return_value = {"apache": True, "nginx": True}
# ret, _, _, _ = self._call(args)
# self.assertTrue("Too many flags setting" in ret)

4
certbot/tests/plugins/common_test.py
View file

@ -233,11 +233,11 @@ class AddrTest(unittest.TestCase):
def test_eq(self):
self.assertEqual(self.addr1, self.addr2.get_addr_obj(""))
self.assertNotEqual(self.addr1, self.addr2)
self.assertFalse(self.addr1 == 3333)
self.assertNotEqual(self.addr1, 3333)
self.assertEqual(self.addr4, self.addr4.get_addr_obj(""))
self.assertNotEqual(self.addr4, self.addr5)
self.assertFalse(self.addr4 == 3333)
self.assertNotEqual(self.addr4, 3333)
from certbot.plugins.common import Addr
self.assertEqual(self.addr4, Addr.fromstring("[fe00:0:0::1]"))
self.assertEqual(self.addr4, Addr.fromstring("[fe00:0::0:0:1]"))

2
certbot/tests/util_test.py
View file

@ -127,7 +127,7 @@ class LockDirUntilExit(test_util.TempDirTestCase):
from certbot import util
# Despite lock_dir_until_exit has been called twice to subdir, its lock should have been
# added only once. So we expect to have two lock references: for self.tempdir and subdir
self.assertTrue(len(util._LOCKS) == 2) # pylint: disable=protected-access
self.assertEqual(len(util._LOCKS), 2) # pylint: disable=protected-access
registered_func() # Exception should not be raised
# Logically, logger.debug, that would be invoked in case of unlock failure,
# should never been called.