From 52c3928b0995640a3b2862cd9ba0050700922e6b Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 16 Apr 2024 16:02:12 +0800 Subject: [PATCH 01/16] test: refactor some for loop tests into pytest parametrize --- test/test_jujuversion.py | 205 +++++++++++++-------------- test/test_log.py | 118 +++++++-------- test/test_pebble.py | 299 +++++++++++++++++++-------------------- test/test_private.py | 158 ++++++++++----------- 4 files changed, 384 insertions(+), 396 deletions(-) diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py index 644cb747c..f9b2bdb52 100644 --- a/test/test_jujuversion.py +++ b/test/test_jujuversion.py @@ -20,28 +20,25 @@ import ops -class TestJujuVersion(unittest.TestCase): - - def test_parsing(self): - test_cases = [ - ("0.0.0", 0, 0, '', 0, 0), - ("0.0.2", 0, 0, '', 2, 0), - ("0.1.0", 0, 1, '', 0, 0), - ("0.2.3", 0, 2, '', 3, 0), - ("10.234.3456", 10, 234, '', 3456, 0), - ("10.234.3456.1", 10, 234, '', 3456, 1), - ("1.21-alpha12", 1, 21, 'alpha', 12, 0), - ("1.21-alpha1.34", 1, 21, 'alpha', 1, 34), - ("2.7", 2, 7, '', 0, 0) - ] - - for vs, major, minor, tag, patch, build in test_cases: - v = ops.JujuVersion(vs) - assert v.major == major - assert v.minor == minor - assert v.tag == tag - assert v.patch == patch - assert v.build == build +class TestJujuVersion: + @pytest.mark.parametrize("vs,major,minor,tag,patch,build", [ + ("0.0.0", 0, 0, '', 0, 0), + ("0.0.2", 0, 0, '', 2, 0), + ("0.1.0", 0, 1, '', 0, 0), + ("0.2.3", 0, 2, '', 3, 0), + ("10.234.3456", 10, 234, '', 3456, 0), + ("10.234.3456.1", 10, 234, '', 3456, 1), + ("1.21-alpha12", 1, 21, 'alpha', 12, 0), + ("1.21-alpha1.34", 1, 21, 'alpha', 1, 34), + ("2.7", 2, 7, '', 0, 0) + ]) + def test_parsing(self, vs: str, major: int, minor: int, tag: str, patch: int, build: int): + v = ops.JujuVersion(vs) + assert v.major == major + assert v.minor == minor + assert v.tag == tag + assert v.patch == patch + assert v.build == build @unittest.mock.patch('os.environ', new={}) # type: ignore def test_from_environ(self): @@ -93,88 +90,82 @@ def test_supports_exec_service_context(self): assert ops.JujuVersion('3.3.0').supports_exec_service_context assert ops.JujuVersion('3.4.0').supports_exec_service_context - def test_parsing_errors(self): - invalid_versions = [ - "xyz", - "foo.bar", - "foo.bar.baz", - "dead.beef.ca.fe", - "1234567890.2.1", # The major version is too long. - "0.2..1", # Two periods next to each other. - "1.21.alpha1", # Tag comes after period. - "1.21-alpha", # No patch number but a tag is present. - "1.21-alpha1beta", # Non-numeric string after the patch number. - "1.21-alpha-dev", # Tag duplication. - "1.21-alpha_dev3", # Underscore in a tag. - "1.21-alpha123dev3", # Non-numeric string after the patch number. - ] - for v in invalid_versions: - with pytest.raises(RuntimeError): - ops.JujuVersion(v) - - def test_equality(self): - test_cases = [ - ("1.0.0", "1.0.0", True), - ("01.0.0", "1.0.0", True), - ("10.0.0", "9.0.0", False), - ("1.0.0", "1.0.1", False), - ("1.0.1", "1.0.0", False), - ("1.0.0", "1.1.0", False), - ("1.1.0", "1.0.0", False), - ("1.0.0", "2.0.0", False), - ("1.2-alpha1", "1.2.0", False), - ("1.2-alpha2", "1.2-alpha1", False), - ("1.2-alpha2.1", "1.2-alpha2", False), - ("1.2-alpha2.2", "1.2-alpha2.1", False), - ("1.2-beta1", "1.2-alpha1", False), - ("1.2-beta1", "1.2-alpha2.1", False), - ("1.2-beta1", "1.2.0", False), - ("1.2.1", "1.2.0", False), - ("2.0.0", "1.0.0", False), - ("2.0.0.0", "2.0.0", True), - ("2.0.0.0", "2.0.0.0", True), - ("2.0.0.1", "2.0.0.0", False), - ("2.0.1.10", "2.0.0.0", False), - ] - - for a, b, expected in test_cases: - assert (ops.JujuVersion(a) == ops.JujuVersion(b)) == expected - assert (ops.JujuVersion(a) == b) == expected - - def test_comparison(self): - test_cases = [ - ("1.0.0", "1.0.0", False, True), - ("01.0.0", "1.0.0", False, True), - ("10.0.0", "9.0.0", False, False), - ("1.0.0", "1.0.1", True, True), - ("1.0.1", "1.0.0", False, False), - ("1.0.0", "1.1.0", True, True), - ("1.1.0", "1.0.0", False, False), - ("1.0.0", "2.0.0", True, True), - ("1.2-alpha1", "1.2.0", True, True), - ("1.2-alpha2", "1.2-alpha1", False, False), - ("1.2-alpha2.1", "1.2-alpha2", False, False), - ("1.2-alpha2.2", "1.2-alpha2.1", False, False), - ("1.2-beta1", "1.2-alpha1", False, False), - ("1.2-beta1", "1.2-alpha2.1", False, False), - ("1.2-beta1", "1.2.0", True, True), - ("1.2.1", "1.2.0", False, False), - ("2.0.0", "1.0.0", False, False), - ("2.0.0.0", "2.0.0", False, True), - ("2.0.0.0", "2.0.0.0", False, True), - ("2.0.0.1", "2.0.0.0", False, False), - ("2.0.1.10", "2.0.0.0", False, False), - ("2.10.0", "2.8.0", False, False), - ] - - for a, b, expected_strict, expected_weak in test_cases: - with self.subTest(a=a, b=b): - assert (ops.JujuVersion(a) < ops.JujuVersion(b)) == expected_strict - assert (ops.JujuVersion(a) <= ops.JujuVersion(b)) == expected_weak - assert (ops.JujuVersion(b) > ops.JujuVersion(a)) == expected_strict - assert (ops.JujuVersion(b) >= ops.JujuVersion(a)) == expected_weak - # Implicit conversion. - assert (ops.JujuVersion(a) < b) == expected_strict - assert (ops.JujuVersion(a) <= b) == expected_weak - assert (b > ops.JujuVersion(a)) == expected_strict - assert (b >= ops.JujuVersion(a)) == expected_weak + @pytest.mark.parametrize("invalid_version", [ + "xyz", + "foo.bar", + "foo.bar.baz", + "dead.beef.ca.fe", + "1234567890.2.1", # The major version is too long. + "0.2..1", # Two periods next to each other. + "1.21.alpha1", # Tag comes after period. + "1.21-alpha", # No patch number but a tag is present. + "1.21-alpha1beta", # Non-numeric string after the patch number. + "1.21-alpha-dev", # Tag duplication. + "1.21-alpha_dev3", # Underscore in a tag. + "1.21-alpha123dev3", # Non-numeric string after the patch number. + ]) + def test_parsing_errors(self, invalid_version: str): + with pytest.raises(RuntimeError): + ops.JujuVersion(invalid_version) + + @pytest.mark.parametrize("a,b,expected", [ + ("1.0.0", "1.0.0", True), + ("01.0.0", "1.0.0", True), + ("10.0.0", "9.0.0", False), + ("1.0.0", "1.0.1", False), + ("1.0.1", "1.0.0", False), + ("1.0.0", "1.1.0", False), + ("1.1.0", "1.0.0", False), + ("1.0.0", "2.0.0", False), + ("1.2-alpha1", "1.2.0", False), + ("1.2-alpha2", "1.2-alpha1", False), + ("1.2-alpha2.1", "1.2-alpha2", False), + ("1.2-alpha2.2", "1.2-alpha2.1", False), + ("1.2-beta1", "1.2-alpha1", False), + ("1.2-beta1", "1.2-alpha2.1", False), + ("1.2-beta1", "1.2.0", False), + ("1.2.1", "1.2.0", False), + ("2.0.0", "1.0.0", False), + ("2.0.0.0", "2.0.0", True), + ("2.0.0.0", "2.0.0.0", True), + ("2.0.0.1", "2.0.0.0", False), + ("2.0.1.10", "2.0.0.0", False), + ]) + def test_equality(self, a: str, b: str, expected: bool): + assert (ops.JujuVersion(a) == ops.JujuVersion(b)) == expected + assert (ops.JujuVersion(a) == b) == expected + + @pytest.mark.parametrize("a,b,expected_strict,expected_weak", [ + ("1.0.0", "1.0.0", False, True), + ("01.0.0", "1.0.0", False, True), + ("10.0.0", "9.0.0", False, False), + ("1.0.0", "1.0.1", True, True), + ("1.0.1", "1.0.0", False, False), + ("1.0.0", "1.1.0", True, True), + ("1.1.0", "1.0.0", False, False), + ("1.0.0", "2.0.0", True, True), + ("1.2-alpha1", "1.2.0", True, True), + ("1.2-alpha2", "1.2-alpha1", False, False), + ("1.2-alpha2.1", "1.2-alpha2", False, False), + ("1.2-alpha2.2", "1.2-alpha2.1", False, False), + ("1.2-beta1", "1.2-alpha1", False, False), + ("1.2-beta1", "1.2-alpha2.1", False, False), + ("1.2-beta1", "1.2.0", True, True), + ("1.2.1", "1.2.0", False, False), + ("2.0.0", "1.0.0", False, False), + ("2.0.0.0", "2.0.0", False, True), + ("2.0.0.0", "2.0.0.0", False, True), + ("2.0.0.1", "2.0.0.0", False, False), + ("2.0.1.10", "2.0.0.0", False, False), + ("2.10.0", "2.8.0", False, False), + ]) + def test_comparison(self, a: str, b: str, expected_strict: bool, expected_weak: bool): + assert (ops.JujuVersion(a) < ops.JujuVersion(b)) == expected_strict + assert (ops.JujuVersion(a) <= ops.JujuVersion(b)) == expected_weak + assert (ops.JujuVersion(b) > ops.JujuVersion(a)) == expected_strict + assert (ops.JujuVersion(b) >= ops.JujuVersion(a)) == expected_weak + # Implicit conversion. + assert (ops.JujuVersion(a) < b) == expected_strict + assert (ops.JujuVersion(a) <= b) == expected_weak + assert (b > ops.JujuVersion(a)) == expected_strict + assert (b >= ops.JujuVersion(a)) == expected_weak diff --git a/test/test_log.py b/test/test_log.py index 1f6da2fdc..11f052a09 100644 --- a/test/test_log.py +++ b/test/test_log.py @@ -19,6 +19,8 @@ import unittest from unittest.mock import patch +import pytest + import ops.log from ops.model import MAX_LOG_LINE_LEN, _ModelBackend @@ -39,76 +41,78 @@ def juju_log(self, level: str, message: str): self._calls.append((level, line)) -class TestLogging(unittest.TestCase): - - def setUp(self): - self.backend = FakeModelBackend() - - def tearDown(self): - logging.getLogger().handlers.clear() - - def test_default_logging(self): - ops.log.setup_root_logging(self.backend) - - logger = logging.getLogger() +@pytest.fixture() +def backend(): + return FakeModelBackend() + + +@pytest.fixture() +def logger(): + logger = logging.getLogger() + yield logger + logging.getLogger().handlers.clear() + + +class TestLogging: + @pytest.mark.parametrize("message,result", [ + ('critical', ('CRITICAL', 'critical')), + ('error', ('ERROR', 'error')), + ('warning', ('WARNING', 'warning')), + ('info', ('INFO', 'info')), + ('debug', ('DEBUG', 'debug')), + ]) + def test_default_logging(self, + backend: FakeModelBackend, + logger: logging.Logger, + message: str, + result: typing.Tuple[str, str]): + ops.log.setup_root_logging(backend) assert logger.level == logging.DEBUG assert isinstance(logger.handlers[-1], ops.log.JujuLogHandler) - test_cases = [ - (logger.critical, 'critical', ('CRITICAL', 'critical')), - (logger.error, 'error', ('ERROR', 'error')), - (logger.warning, 'warning', ('WARNING', 'warning')), - (logger.info, 'info', ('INFO', 'info')), - (logger.debug, 'debug', ('DEBUG', 'debug')), - ] - - for method, message, result in test_cases: - with self.subTest(message): - method(message) - calls = self.backend.calls(clear=True) - assert calls == [result] + method = getattr(logger, message) + method(message) + calls = backend.calls(clear=True) + assert calls == [result] - def test_handler_filtering(self): - logger = logging.getLogger() + def test_handler_filtering(self, backend: FakeModelBackend, logger: logging.Logger): logger.setLevel(logging.INFO) - logger.addHandler(ops.log.JujuLogHandler(self.backend, logging.WARNING)) + logger.addHandler(ops.log.JujuLogHandler(backend, logging.WARNING)) logger.info('foo') - assert self.backend.calls() == [] + assert backend.calls() == [] logger.warning('bar') - assert self.backend.calls() == [('WARNING', 'bar')] + assert backend.calls() == [('WARNING', 'bar')] - def test_no_stderr_without_debug(self): + def test_no_stderr_without_debug(self, backend: FakeModelBackend, logger: logging.Logger): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(self.backend, debug=False) - logger = logging.getLogger() + ops.log.setup_root_logging(backend, debug=False) logger.debug('debug message') logger.info('info message') logger.warning('warning message') logger.critical('critical message') - assert self.backend.calls() == \ - [('DEBUG', 'debug message'), - ('INFO', 'info message'), - ('WARNING', 'warning message'), - ('CRITICAL', 'critical message'), - ] + assert backend.calls() == [ + ('DEBUG', 'debug message'), + ('INFO', 'info message'), + ('WARNING', 'warning message'), + ('CRITICAL', 'critical message'), + ] assert buffer.getvalue() == "" - def test_debug_logging(self): + def test_debug_logging(self, backend: FakeModelBackend, logger: logging.Logger): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(self.backend, debug=True) - logger = logging.getLogger() + ops.log.setup_root_logging(backend, debug=True) logger.debug('debug message') logger.info('info message') logger.warning('warning message') logger.critical('critical message') - assert self.backend.calls() == \ - [('DEBUG', 'debug message'), - ('INFO', 'info message'), - ('WARNING', 'warning message'), - ('CRITICAL', 'critical message'), - ] + assert backend.calls() == [ + ('DEBUG', 'debug message'), + ('INFO', 'info message'), + ('WARNING', 'warning message'), + ('CRITICAL', 'critical message'), + ] assert re.search( r"\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d,\d\d\d DEBUG debug message\n" r"\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d,\d\d\d INFO info message\n" @@ -117,31 +121,29 @@ def test_debug_logging(self): buffer.getvalue() ) - def test_reduced_logging(self): - ops.log.setup_root_logging(self.backend) - logger = logging.getLogger() + def test_reduced_logging(self, backend: FakeModelBackend, logger: logging.Logger): + ops.log.setup_root_logging(backend) logger.setLevel(logging.WARNING) logger.debug('debug') logger.info('info') logger.warning('warning') - assert self.backend.calls() == [('WARNING', 'warning')] + assert backend.calls() == [('WARNING', 'warning')] - def test_long_string_logging(self): + def test_long_string_logging(self, backend: FakeModelBackend, logger: logging.Logger): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(self.backend, debug=True) - logger = logging.getLogger() + ops.log.setup_root_logging(backend, debug=True) logger.debug('l' * MAX_LOG_LINE_LEN) - assert len(self.backend.calls()) == 1 + assert len(backend.calls()) == 1 - self.backend.calls(clear=True) + backend.calls(clear=True) with patch('sys.stderr', buffer): logger.debug('l' * (MAX_LOG_LINE_LEN + 9)) - calls = self.backend.calls() + calls = backend.calls() assert len(calls) == 3 # Verify that we note that we are splitting the log message. assert "Splitting into multiple chunks" in calls[0][1] diff --git a/test/test_pebble.py b/test/test_pebble.py index 08470dfae..04086921b 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -1426,159 +1426,158 @@ def build_mock_change_dict(change_id: str = '70') -> 'pebble._ChangeDict': } -class TestMultipartParser(unittest.TestCase): - class _Case: - def __init__( - self, - name: str, - data: bytes, - want_headers: typing.List[bytes], - want_bodies: typing.List[bytes], - want_bodies_done: typing.List[bool], - max_boundary: int = 14, - max_lookahead: int = 8 * 1024, - error: str = ''): - self.name = name - self.data = data - self.want_headers = want_headers - self.want_bodies = want_bodies - self.want_bodies_done = want_bodies_done - self.max_boundary = max_boundary - self.max_lookahead = max_lookahead - self.error = error - - def test_multipart_parser(self): - tests = [ - TestMultipartParser._Case( - 'baseline', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\nfoo bar\r\n--qwerty--\r\n', - [b'header foo\r\n\r\n'], - [b'foo bar\nfoo bar'], - want_bodies_done=[True], - ), - TestMultipartParser._Case( - 'incomplete header', - b'\r\n--qwerty\r\nheader foo\r\n', - [], - [], - want_bodies_done=[], - ), - TestMultipartParser._Case( - 'missing header', - b'\r\n--qwerty\r\nheader foo\r\n' + 40 * b' ', - [], - [], - want_bodies_done=[], - max_lookahead=40, - error='header terminator not found', - ), - TestMultipartParser._Case( - 'incomplete body terminator', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\rhello my name is joe and I work in a button factory', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar\r\n--qwerty\rhello my name is joe and I work in a '], - want_bodies_done=[False], - ), - TestMultipartParser._Case( - 'empty body', - b'\r\n--qwerty\r\nheader foo\r\n\r\n\r\n--qwerty\r\n', - [b'header foo\r\n\r\n'], - [b''], - want_bodies_done=[True], - ), - TestMultipartParser._Case( - 'ignore leading garbage', - b'hello my name is joe\r\n\n\n\n\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - TestMultipartParser._Case( - 'ignore trailing garbage', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nhello my name is joe', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - TestMultipartParser._Case( - 'boundary allow linear whitespace', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - max_boundary=20, - ), - TestMultipartParser._Case( - 'terminal boundary allow linear whitespace', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty-- \t \r\n', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - max_boundary=20, - ), - TestMultipartParser._Case( - 'multiple parts', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa - [b'header foo\r\n\r\n', b'header bar\r\n\r\n'], - [b'foo bar', b'foo baz'], - want_bodies_done=[True, True], - ), - TestMultipartParser._Case( - 'ignore after terminal boundary', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty--\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - ] - +class MultipartParserTestCase: + def __init__( + self, + name: str, + data: bytes, + want_headers: typing.List[bytes], + want_bodies: typing.List[bytes], + want_bodies_done: typing.List[bool], + max_boundary: int = 14, + max_lookahead: int = 8 * 1024, + error: str = ''): + self.name = name + self.data = data + self.want_headers = want_headers + self.want_bodies = want_bodies + self.want_bodies_done = want_bodies_done + self.max_boundary = max_boundary + self.max_lookahead = max_lookahead + self.error = error + + +class TestMultipartParser: + @pytest.mark.parametrize("test", [ + MultipartParserTestCase( + 'baseline', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\nfoo bar\r\n--qwerty--\r\n', + [b'header foo\r\n\r\n'], + [b'foo bar\nfoo bar'], + want_bodies_done=[True], + ), + MultipartParserTestCase( + 'incomplete header', + b'\r\n--qwerty\r\nheader foo\r\n', + [], + [], + want_bodies_done=[], + ), + MultipartParserTestCase( + 'missing header', + b'\r\n--qwerty\r\nheader foo\r\n' + 40 * b' ', + [], + [], + want_bodies_done=[], + max_lookahead=40, + error='header terminator not found', + ), + MultipartParserTestCase( + 'incomplete body terminator', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\rhello my name is joe and I work in a button factory', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar\r\n--qwerty\rhello my name is joe and I work in a '], + want_bodies_done=[False], + ), + MultipartParserTestCase( + 'empty body', + b'\r\n--qwerty\r\nheader foo\r\n\r\n\r\n--qwerty\r\n', + [b'header foo\r\n\r\n'], + [b''], + want_bodies_done=[True], + ), + MultipartParserTestCase( + 'ignore leading garbage', + b'hello my name is joe\r\n\n\n\n\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + MultipartParserTestCase( + 'ignore trailing garbage', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nhello my name is joe', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + MultipartParserTestCase( + 'boundary allow linear whitespace', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + max_boundary=20, + ), + MultipartParserTestCase( + 'terminal boundary allow linear whitespace', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty-- \t \r\n', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + max_boundary=20, + ), + MultipartParserTestCase( + 'multiple parts', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa + [b'header foo\r\n\r\n', b'header bar\r\n\r\n'], + [b'foo bar', b'foo baz'], + want_bodies_done=[True, True], + ), + MultipartParserTestCase( + 'ignore after terminal boundary', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty--\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + ]) + def test_multipart_parser(self, test: MultipartParserTestCase): chunk_sizes = [1, 2, 3, 4, 5, 7, 13, 17, 19, 23, 29, 31, 37, 42, 50, 100, 1000] marker = b'qwerty' - for i, test in enumerate(tests): - for chunk_size in chunk_sizes: - headers: typing.List[bytes] = [] - bodies: typing.List[bytes] = [] - bodies_done: typing.List[bool] = [] - - # All of the "noqa: B023" here are due to a ruff bug: - # https://github.com/astral-sh/ruff/issues/7847 - # ruff should tell us when the 'noqa's are no longer required. - def handle_header(data: typing.Any): - headers.append(bytes(data)) # noqa: B023 - bodies.append(b'') # noqa: B023 - bodies_done.append(False) # noqa: B023 - - def handle_body(data: bytes, done: bool = False): - bodies[-1] += data # noqa: B023 - bodies_done[-1] = done # noqa: B023 - - parser = pebble._MultipartParser( - marker, - handle_header, - handle_body, - max_boundary_length=test.max_boundary, - max_lookahead=test.max_lookahead) - src = io.BytesIO(test.data) - - try: - while True: - data = src.read(chunk_size) - if not data: - break - parser.feed(data) - except Exception as err: - if not test.error: - self.fail(f'unexpected error: {err}') + for chunk_size in chunk_sizes: + headers: typing.List[bytes] = [] + bodies: typing.List[bytes] = [] + bodies_done: typing.List[bool] = [] + + # All of the "noqa: B023" here are due to a ruff bug: + # https://github.com/astral-sh/ruff/issues/7847 + # ruff should tell us when the 'noqa's are no longer required. + def handle_header(data: typing.Any): + headers.append(bytes(data)) # noqa: B023 + bodies.append(b'') # noqa: B023 + bodies_done.append(False) # noqa: B023 + + def handle_body(data: bytes, done: bool = False): + bodies[-1] += data # noqa: B023 + bodies_done[-1] = done # noqa: B023 + + parser = pebble._MultipartParser( + marker, + handle_header, + handle_body, + max_boundary_length=test.max_boundary, + max_lookahead=test.max_lookahead) + src = io.BytesIO(test.data) + + try: + while True: + data = src.read(chunk_size) + if not data: break - assert test.error == str(err) - else: - if test.error: - self.fail(f'missing expected error: {test.error!r}') - - msg = f'test case {i + 1} ({test.name}), chunk size {chunk_size}' - assert test.want_headers == headers, msg - assert test.want_bodies == bodies, msg - assert test.want_bodies_done == bodies_done, msg + parser.feed(data) + except Exception as err: + if not test.error: + pytest.fail(f'unexpected error: {err}') + break + assert test.error == str(err) + else: + if test.error: + pytest.fail(f'missing expected error: {test.error!r}') + + msg = f'test case ({test.name}), chunk size {chunk_size}' + assert test.want_headers == headers, msg + assert test.want_bodies == bodies, msg + assert test.want_bodies_done == bodies_done, msg class TestClient(unittest.TestCase): diff --git a/test/test_private.py b/test/test_private.py index 54ec5fefe..0e31cec5f 100644 --- a/test/test_private.py +++ b/test/test_private.py @@ -50,7 +50,7 @@ def test_safe_dump(self): yaml.safe_dump(YAMLTest()) -class TestStrconv(unittest.TestCase): +class TestStrconv: def test_parse_rfc3339(self): nzdt = datetime.timezone(datetime.timedelta(hours=13)) utc = datetime.timezone.utc @@ -102,84 +102,80 @@ def test_parse_rfc3339(self): with pytest.raises(ValueError): timeconv.parse_rfc3339('2021-02-10T04:36:22.118970777-99:99') - def test_parse_duration(self): + @pytest.mark.parametrize("input,expected", [ # Test cases taken from Go's time.ParseDuration tests - cases = [ - # simple - ('0', datetime.timedelta(seconds=0)), - ('5s', datetime.timedelta(seconds=5)), - ('30s', datetime.timedelta(seconds=30)), - ('1478s', datetime.timedelta(seconds=1478)), - # sign - ('-5s', datetime.timedelta(seconds=-5)), - ('+5s', datetime.timedelta(seconds=5)), - ('-0', datetime.timedelta(seconds=0)), - ('+0', datetime.timedelta(seconds=0)), - # decimal - ('5.0s', datetime.timedelta(seconds=5)), - ('5.6s', datetime.timedelta(seconds=5.6)), - ('5.s', datetime.timedelta(seconds=5)), - ('.5s', datetime.timedelta(seconds=0.5)), - ('1.0s', datetime.timedelta(seconds=1)), - ('1.00s', datetime.timedelta(seconds=1)), - ('1.004s', datetime.timedelta(seconds=1.004)), - ('1.0040s', datetime.timedelta(seconds=1.004)), - ('100.00100s', datetime.timedelta(seconds=100.001)), - # different units - ('10ns', datetime.timedelta(seconds=0.000_000_010)), - ('11us', datetime.timedelta(seconds=0.000_011)), - ('12µs', datetime.timedelta(seconds=0.000_012)), # U+00B5 # noqa: RUF001 - ('12μs', datetime.timedelta(seconds=0.000_012)), # U+03BC - ('13ms', datetime.timedelta(seconds=0.013)), - ('14s', datetime.timedelta(seconds=14)), - ('15m', datetime.timedelta(seconds=15 * 60)), - ('16h', datetime.timedelta(seconds=16 * 60 * 60)), - # composite durations - ('3h30m', datetime.timedelta(seconds=3 * 60 * 60 + 30 * 60)), - ('10.5s4m', datetime.timedelta(seconds=4 * 60 + 10.5)), - ('-2m3.4s', datetime.timedelta(seconds=-(2 * 60 + 3.4))), - ('1h2m3s4ms5us6ns', datetime.timedelta(seconds=1 * 60 * 60 + 2 * 60 + 3.004_005_006)), - ('39h9m14.425s', datetime.timedelta(seconds=39 * 60 * 60 + 9 * 60 + 14.425)), - # large value - ('52763797000ns', datetime.timedelta(seconds=52.763_797_000)), - # more than 9 digits after decimal point, see https://golang.org/issue/6617 - ('0.3333333333333333333h', datetime.timedelta(seconds=20 * 60)), - # huge string; issue 15011. - ('0.100000000000000000000h', datetime.timedelta(seconds=6 * 60)), - # This value tests the first overflow check in leadingFraction. - ('0.830103483285477580700h', datetime.timedelta(seconds=49 * 60 + 48.372_539_827)), - - # Test precision handling - ('7200000h1us', datetime.timedelta(hours=7_200_000, microseconds=1)) - ] - - for input, expected in cases: - output = timeconv.parse_duration(input) - assert output == expected, \ - f'parse_duration({input!r}): expected {expected!r}, got {output!r}' - - def test_parse_duration_errors(self): - cases = [ - # Test cases taken from Go's time.ParseDuration tests - '', - '3', - '-', - 's', - '.', - '-.', - '.s', - '+.s', - '1d', - '\x85\x85', - '\xffff', - 'hello \xffff world', - - # Additional cases - 'X3h', - '3hY', - 'X3hY', - '3.4.5s', - ] - for input in cases: - with pytest.raises(ValueError): - timeconv.parse_duration(input) + # simple + ('0', datetime.timedelta(seconds=0)), + ('5s', datetime.timedelta(seconds=5)), + ('30s', datetime.timedelta(seconds=30)), + ('1478s', datetime.timedelta(seconds=1478)), + # sign + ('-5s', datetime.timedelta(seconds=-5)), + ('+5s', datetime.timedelta(seconds=5)), + ('-0', datetime.timedelta(seconds=0)), + ('+0', datetime.timedelta(seconds=0)), + # decimal + ('5.0s', datetime.timedelta(seconds=5)), + ('5.6s', datetime.timedelta(seconds=5.6)), + ('5.s', datetime.timedelta(seconds=5)), + ('.5s', datetime.timedelta(seconds=0.5)), + ('1.0s', datetime.timedelta(seconds=1)), + ('1.00s', datetime.timedelta(seconds=1)), + ('1.004s', datetime.timedelta(seconds=1.004)), + ('1.0040s', datetime.timedelta(seconds=1.004)), + ('100.00100s', datetime.timedelta(seconds=100.001)), + # different units + ('10ns', datetime.timedelta(seconds=0.000_000_010)), + ('11us', datetime.timedelta(seconds=0.000_011)), + ('12µs', datetime.timedelta(seconds=0.000_012)), # U+00B5 # noqa: RUF001 + ('12μs', datetime.timedelta(seconds=0.000_012)), # U+03BC + ('13ms', datetime.timedelta(seconds=0.013)), + ('14s', datetime.timedelta(seconds=14)), + ('15m', datetime.timedelta(seconds=15 * 60)), + ('16h', datetime.timedelta(seconds=16 * 60 * 60)), + # composite durations + ('3h30m', datetime.timedelta(seconds=3 * 60 * 60 + 30 * 60)), + ('10.5s4m', datetime.timedelta(seconds=4 * 60 + 10.5)), + ('-2m3.4s', datetime.timedelta(seconds=-(2 * 60 + 3.4))), + ('1h2m3s4ms5us6ns', datetime.timedelta(seconds=1 * 60 * 60 + 2 * 60 + 3.004_005_006)), + ('39h9m14.425s', datetime.timedelta(seconds=39 * 60 * 60 + 9 * 60 + 14.425)), + # large value + ('52763797000ns', datetime.timedelta(seconds=52.763_797_000)), + # more than 9 digits after decimal point, see https://golang.org/issue/6617 + ('0.3333333333333333333h', datetime.timedelta(seconds=20 * 60)), + # huge string; issue 15011. + ('0.100000000000000000000h', datetime.timedelta(seconds=6 * 60)), + # This value tests the first overflow check in leadingFraction. + ('0.830103483285477580700h', datetime.timedelta(seconds=49 * 60 + 48.372_539_827)), + # Test precision handling + ('7200000h1us', datetime.timedelta(hours=7_200_000, microseconds=1)) + ]) + def test_parse_duration(self, input: str, expected: datetime.timedelta): + output = timeconv.parse_duration(input) + assert output == expected, \ + f'parse_duration({input!r}): expected {expected!r}, got {output!r}' + + @pytest.mark.parametrize("input", [ + # Test cases taken from Go's time.ParseDuration tests + '', + '3', + '-', + 's', + '.', + '-.', + '.s', + '+.s', + '1d', + '\x85\x85', + '\xffff', + 'hello \xffff world', + + # Additional cases + 'X3h', + '3hY', + 'X3hY', + '3.4.5s', + ]) + def test_parse_duration_errors(self, input: str): + with pytest.raises(ValueError): + timeconv.parse_duration(input) From d97619edd7700ea1316aefcda8eb709ae1d55a0a Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Thu, 18 Apr 2024 13:38:07 +0800 Subject: [PATCH 02/16] test: fake script helper fixture --- test/test_helpers.py | 55 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/test_helpers.py b/test/test_helpers.py index 5f9b0e316..c0c6fb524 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -20,6 +20,8 @@ import typing import unittest +from _pytest.monkeypatch import MonkeyPatch + import ops from ops.model import _ModelBackend from ops.storage import SQLiteStorage @@ -74,6 +76,59 @@ def fake_script_calls(test_case: unittest.TestCase, return calls # type: ignore +class FakeScriptFixture: + def __init__(self, + monkeypatch: MonkeyPatch, + tmp_path: pathlib.Path, + fake_script_path: typing.Union[pathlib.Path, None]=None): + self.monkeypatch = monkeypatch + if fake_script_path is None: + self.fake_script_path = tmp_path + self.monkeypatch.setenv( + "PATH", + str(self.fake_script_path), + prepend=os.pathsep) # type: ignore + else: + self.fake_script_path = fake_script_path + + def fake_script(self, + name: str, + content: str): + template_args: typing.Dict[str, str] = { + 'name': name, + 'path': self.fake_script_path.as_posix(), # type: ignore + 'content': content, + } + + path: pathlib.Path = self.fake_script_path / name # type: ignore + with path.open('wt') as f: # type: ignore + # Before executing the provided script, dump the provided arguments in calls.txt. + # ASCII 1E is RS 'record separator', and 1C is FS 'file separator', which + # seem appropriate. + f.write( # type: ignore + '''#!/bin/sh + {{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt + {content}'''.format_map(template_args)) + os.chmod(str(path), 0o755) # type: ignore # noqa: S103 + # TODO: this hardcodes the path to bash.exe, which works for now but might + # need to be set via environ or something like that. + path.with_suffix(".bat").write_text( # type: ignore + f'@"C:\\Program Files\\git\\bin\\bash.exe" {path} %*\n') + + def fake_script_calls(self, clear: bool = False) -> typing.List[typing.List[str]]: + calls_file: pathlib.Path = self.fake_script_path / 'calls.txt' # type: ignore + if not calls_file.exists(): # type: ignore + return [] + + # newline and encoding forced to linuxy defaults because on + # windows they're written from git-bash + with calls_file.open('r+t', newline='\n', encoding='utf8') as f: # type: ignore + calls = [line.split('\x1e') for line in f.read().split('\x1c')[:-1]] # type: ignore + if clear: + f.truncate(0) # type: ignore + return calls # type: ignore + + class FakeScriptTest(unittest.TestCase): def test_fake_script_works(self): From 41b17bda93f7046b2678011f2921b34abf890c17 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Thu, 18 Apr 2024 13:41:46 +0800 Subject: [PATCH 03/16] chore: fix linting --- test/test_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helpers.py b/test/test_helpers.py index c0c6fb524..642c20bf3 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -80,7 +80,7 @@ class FakeScriptFixture: def __init__(self, monkeypatch: MonkeyPatch, tmp_path: pathlib.Path, - fake_script_path: typing.Union[pathlib.Path, None]=None): + fake_script_path: typing.Union[pathlib.Path, None] = None): self.monkeypatch = monkeypatch if fake_script_path is None: self.fake_script_path = tmp_path From 42fb71556b2a85b46ba4681f61d932974616874a Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Thu, 18 Apr 2024 14:29:52 +0800 Subject: [PATCH 04/16] chore: fix monkeypatch typing --- test/test_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_helpers.py b/test/test_helpers.py index 642c20bf3..c07789a83 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -20,7 +20,7 @@ import typing import unittest -from _pytest.monkeypatch import MonkeyPatch +import pytest import ops from ops.model import _ModelBackend @@ -78,7 +78,7 @@ def fake_script_calls(test_case: unittest.TestCase, class FakeScriptFixture: def __init__(self, - monkeypatch: MonkeyPatch, + monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path, fake_script_path: typing.Union[pathlib.Path, None] = None): self.monkeypatch = monkeypatch From 0c01607eba4bcb4f585a9ca3c71abac258168b07 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Thu, 18 Apr 2024 17:18:42 +0800 Subject: [PATCH 05/16] test: refactor test_charm to pytest --- pyproject.toml | 5 + test/test_charm.py | 522 ++++++++++++++++++++++++--------------------- 2 files changed, 286 insertions(+), 241 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f873965c0..a0be21123 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -196,3 +196,8 @@ reportUnnecessaryIsInstance = false reportUnnecessaryComparison = false disableBytesTypePromotions = false stubPath = "" + +[tool.pytest.ini_options] +markers = [ + "charm_meta", # Custom fixture to pass charm meta to the framework fixture. +] diff --git a/test/test_charm.py b/test/test_charm.py index 47121422b..6e1c5a3fe 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -14,7 +14,6 @@ import functools import os import pathlib -import shutil import tempfile import typing import unittest @@ -28,50 +27,51 @@ from ops.model import ModelError, _ModelBackend from ops.storage import SQLiteStorage -from .test_helpers import fake_script, fake_script_calls +from .test_helpers import FakeScriptFixture +# from .test_helpers import fake_script, fake_script_calls -class TestCharm(unittest.TestCase): - def setUp(self): - def restore_env(env: typing.Dict[str, str]): - os.environ.clear() - os.environ.update(env) - self.addCleanup(restore_env, os.environ.copy()) +@pytest.fixture(autouse=True) +def env(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("PATH", str(Path(__file__).parent / 'bin'), prepend=os.pathsep) + monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") + yield - os.environ['PATH'] = os.pathsep.join([ - str(Path(__file__).parent / 'bin'), - os.environ['PATH']]) - os.environ['JUJU_UNIT_NAME'] = 'local/0' - self.tmpdir = Path(tempfile.mkdtemp()) - self.addCleanup(shutil.rmtree, str(self.tmpdir)) - self.meta = ops.CharmMeta() +@pytest.fixture(autouse=True) +def events(): + class CustomEvent(ops.EventBase): + pass - class CustomEvent(ops.EventBase): - pass + class TestCharmEvents(ops.CharmEvents): + custom = ops.EventSource(CustomEvent) + + # Relations events are defined dynamically and modify the class attributes. + # We use a subclass temporarily to prevent these side effects from leaking. + ops.CharmBase.on = TestCharmEvents() # type: ignore + yield + ops.CharmBase.on = ops.CharmEvents() # type: ignore - class TestCharmEvents(ops.CharmEvents): - custom = ops.EventSource(CustomEvent) - # Relations events are defined dynamically and modify the class attributes. - # We use a subclass temporarily to prevent these side effects from leaking. - ops.CharmBase.on = TestCharmEvents() # type: ignore +@pytest.fixture +def framework(request: pytest.FixtureRequest, tmp_path: pathlib.Path): + marker = request.node.get_closest_marker("charm_meta") # type: ignore + meta = marker.args[0] if marker else ops.CharmMeta() # type: ignore + model = ops.Model(meta, _ModelBackend('local/0')) # type: ignore + # we can pass foo_event as event_name because we're not actually testing dispatch + framework = ops.Framework(SQLiteStorage(':memory:'), tmp_path, meta, model) # type: ignore + yield framework + framework.close() - def cleanup(): - ops.CharmBase.on = ops.CharmEvents() # type: ignore - self.addCleanup(cleanup) - def create_framework(self): - model = ops.Model(self.meta, _ModelBackend('local/0')) - # we can pass foo_event as event_name because we're not actually testing dispatch - framework = ops.Framework(SQLiteStorage(':memory:'), self.tmpdir, self.meta, - model) - self.addCleanup(framework.close) - return framework +@pytest.fixture +def fake_script_fixture(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path): + return FakeScriptFixture(monkeypatch, tmp_path) - def test_basic(self): +class TestCharm: + def test_basic(self, framework: ops.Framework): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -86,17 +86,13 @@ def _on_start(self, event: ops.EventBase): events: typing.List[str] = list(MyCharm.on.events()) # type: ignore assert 'install' in events assert 'custom' in events - - framework = self.create_framework() charm = MyCharm(framework) charm.on.start.emit() - assert charm.started - with pytest.raises(TypeError): framework.observe(charm.on.start, charm) # type: ignore - def test_observe_decorated_method(self): + def test_observe_decorated_method(self, framework: ops.Framework): # we test that charm methods decorated with @functools.wraps(wrapper) # can be observed by Framework. Simpler decorators won't work because # Framework searches for __self__ and other method things; functools.wraps @@ -124,7 +120,6 @@ def __init__(self, *args: typing.Any): def _on_start(self, event: ops.EventBase): self.seen = event - framework = self.create_framework() charm = MyCharm(framework) charm.on.start.emit() # check that the event has been seen by the decorator @@ -132,7 +127,10 @@ def _on_start(self, event: ops.EventBase): # check that the event has been seen by the observer assert isinstance(charm.seen, ops.StartEvent) - def test_observer_not_referenced_warning(self): + def test_observer_not_referenced_warning( + self, + caplog: pytest.LogCaptureFixture, + framework: ops.Framework): class MyObj(ops.Object): def __init__(self, charm: ops.CharmBase): super().__init__(charm, "obj") @@ -150,19 +148,15 @@ def __init__(self, *args: typing.Any): def _on_start(self, _: ops.StartEvent): pass # is reached - framework = self.create_framework() c = MyCharm(framework) - with self.assertLogs() as logs: - c.on.start.emit() - assert any('Reference to ops.Object' in log for log in logs.output) + c.on.start.emit() + assert 'Reference to ops.Object' in caplog.text def test_empty_action(self): meta = ops.CharmMeta.from_yaml('name: my-charm', '') assert meta.actions == {} - def test_helper_properties(self): - framework = self.create_framework() - + def test_helper_properties(self, framework: ops.Framework): class MyCharm(ops.CharmBase): pass @@ -173,7 +167,25 @@ class MyCharm(ops.CharmBase): assert charm.charm_dir == framework.charm_dir assert charm.config is framework.model.config - def test_relation_events(self): + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +requires: + req1: + interface: req1 + req-2: + interface: req2 +provides: + pro1: + interface: pro1 + pro-2: + interface: pro2 +peers: + peer1: + interface: peer1 + peer-2: + interface: peer2 +''')) + def test_relation_events(self, framework: ops.Framework): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -192,27 +204,7 @@ def on_any_relation(self, event: ops.RelationEvent): assert event.relation.app.name == 'remote' self.seen.append(type(event).__name__) - # language=YAML - self.meta = ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -requires: - req1: - interface: req1 - req-2: - interface: req2 -provides: - pro1: - interface: pro1 - pro-2: - interface: pro2 -peers: - peer1: - interface: peer1 - peer-2: - interface: peer2 -''') - - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) assert 'pro_2_relation_broken' in repr(charm.on) @@ -239,31 +231,7 @@ def on_any_relation(self, event: ops.RelationEvent): 'RelationBrokenEvent', ] - def test_storage_events(self): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) - self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) - self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) - self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) - - def _on_stor1_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - assert event.storage.location == Path("/var/srv/stor1/0") - - def _on_stor2_detach(self, event: ops.StorageDetachingEvent): - self.seen.append(type(event).__name__) - - def _on_stor3_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - - def _on_stor4_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - - # language=YAML - self.meta = ops.CharmMeta.from_yaml(''' + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(''' name: my-charm storage: stor-4: @@ -288,10 +256,34 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): multiple: range: 10+ type: filesystem -''') +''')) + def test_storage_events(self, + request: pytest.FixtureRequest, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) + self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) + self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) + self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) - fake_script( - self, + def _on_stor1_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + assert event.storage.location == Path("/var/srv/stor1/0") + + def _on_stor2_detach(self, event: ops.StorageDetachingEvent): + self.seen.append(type(event).__name__) + + def _on_stor3_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + def _on_stor4_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + fake_script_fixture.fake_script( "storage-get", """ if [ "$1" = "-s" ]; then @@ -323,21 +315,21 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): fi """, ) - fake_script( - self, + fake_script_fixture.fake_script( "storage-list", """ echo '["disks/0"]' """, ) - assert self.meta.storages['stor1'].multiple_range is None - assert self.meta.storages['stor2'].multiple_range == (2, 2) - assert self.meta.storages['stor3'].multiple_range == (2, None) - assert self.meta.storages['stor-4'].multiple_range == (2, 4) - assert self.meta.storages['stor-plus'].multiple_range == (10, None) + meta = request.node.get_closest_marker("charm_meta").args[0] # type: ignore + assert meta.storages['stor1'].multiple_range is None # type: ignore + assert meta.storages['stor2'].multiple_range == (2, 2) # type: ignore + assert meta.storages['stor3'].multiple_range == (2, None) # type: ignore + assert meta.storages['stor-4'].multiple_range == (2, 4) # type: ignore + assert meta.storages['stor-plus'].multiple_range == (10, None) # type: ignore - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) charm.on['stor1'].storage_attached.emit(ops.Storage("stor1", 0, charm.model._backend)) charm.on['stor2'].storage_detaching.emit(ops.Storage("stor2", 0, charm.model._backend)) @@ -353,7 +345,13 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): 'StorageAttachedEvent', ] - def test_workload_events(self): + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +containers: + container-a: + containerb: +''')) + def test_workload_events(self, framework: ops.Framework): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -375,15 +373,7 @@ def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self.seen.append(type(event).__name__) - # language=YAML - self.meta = ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -containers: - container-a: - containerb: -''') - - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) assert 'container_a_pebble_ready' in repr(charm.on) assert 'containerb_pebble_ready' in repr(charm.on) @@ -405,9 +395,9 @@ def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): 'PebbleCustomNoticeEvent', ] - def test_relations_meta(self): - # language=YAML - self.meta = ops.CharmMeta.from_yaml(''' + @pytest.mark.charm_meta() + def test_relations_meta(self, request: pytest.FixtureRequest,): + meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: database: @@ -419,20 +409,20 @@ def test_relations_meta(self): optional: true ''') - assert self.meta.requires['database'].interface_name == 'mongodb' - assert self.meta.requires['database'].limit == 1 - assert self.meta.requires['database'].scope == 'container' - assert not self.meta.requires['database'].optional + assert meta.requires['database'].interface_name == 'mongodb' # type: ignore + assert meta.requires['database'].limit == 1 # type: ignore + assert meta.requires['database'].scope == 'container' # type: ignore + assert not meta.requires['database'].optional # type: ignore - assert self.meta.requires['metrics'].interface_name == 'prometheus-scraping' - assert self.meta.requires['metrics'].limit is None - assert self.meta.requires['metrics'].scope == 'global' # Default value - assert self.meta.requires['metrics'].optional + assert meta.requires['metrics'].interface_name == 'prometheus-scraping' # type: ignore + assert meta.requires['metrics'].limit is None # type: ignore + assert meta.requires['metrics'].scope == 'global' # Default value # type: ignore + assert meta.requires['metrics'].optional # type: ignore def test_relations_meta_limit_type_validation(self): with pytest.raises(TypeError, match=r"limit should be an int, not "): # language=YAML - self.meta = ops.CharmMeta.from_yaml(''' + ops.CharmMeta.from_yaml(''' name: my-charm requires: database: @@ -446,7 +436,7 @@ def test_relations_meta_scope_type_validation(self): match="scope should be one of 'global', 'container'; not 'foobar'" ): # language=YAML - self.meta = ops.CharmMeta.from_yaml(''' + ops.CharmMeta.from_yaml(''' name: my-charm requires: database: @@ -454,29 +444,6 @@ def test_relations_meta_scope_type_validation(self): scope: foobar ''') - @classmethod - def _get_action_test_meta(cls): - # language=YAML - return ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -''', actions=''' -foo-bar: - description: "Foos the bar." - params: - foo-name: - description: "A foo name to bar" - type: string - silent: - default: false - description: "" - type: boolean - required: foo-bar - title: foo-bar -start: - description: "Start the unit." - additionalProperties: false -''') - def test_meta_from_charm_root(self): with tempfile.TemporaryDirectory() as d: td = pathlib.Path(d) @@ -516,14 +483,34 @@ def test_actions_from_charm_root(self): assert not meta.actions['foo'].additional_properties assert meta.actions['foo'].description == "foos the bar" - def _setup_test_action(self): - fake_script(self, 'action-get', """echo '{"foo-name": "name", "silent": true}'""") - fake_script(self, 'action-set', "") - fake_script(self, 'action-log', "") - fake_script(self, 'action-fail', "") - self.meta = self._get_action_test_meta() + def _setup_test_action(self, fake_script: typing.Callable[..., None]): + fake_script('action-get', """echo '{"foo-name": "name", "silent": true}'""") + fake_script('action-set', "") + fake_script('action-log', "") + fake_script('action-fail', "") - def test_action_events(self): + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +''', actions=''' +foo-bar: + description: "Foos the bar." + params: + foo-name: + description: "A foo name to bar" + type: string + silent: + default: false + description: "" + type: boolean + required: foo-bar + title: foo-bar +start: + description: "Start the unit." + additionalProperties: false +''')) + def test_action_events(self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): @@ -541,8 +528,7 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): def _on_start_action(self, event: ops.ActionEvent): pass - self._setup_test_action() - framework = self.create_framework() + self._setup_test_action(fake_script_fixture.fake_script) charm = MyCharm(framework) events: typing.List[str] = list(MyCharm.on.events()) # type: ignore @@ -552,14 +538,43 @@ def _on_start_action(self, event: ops.ActionEvent): action_id = "1234" charm.on.foo_bar_action.emit(id=action_id) assert charm.seen_action_params == {"foo-name": "name", "silent": True} - assert fake_script_calls(self) == [ + assert fake_script_fixture.fake_script_calls() == [ ['action-get', '--format=json'], ['action-log', "test-log"], ['action-set', "res=val with spaces", f"id={action_id}"], ['action-fail', "test-fail"], ] - def test_invalid_action_results(self): + @pytest.mark.parametrize("bad_res", [ + {'a': {'b': 'c'}, 'a.b': 'c'}, + {'a': {'B': 'c'}}, + {'a': {(1, 2): 'c'}}, + {'a': {None: 'c'}}, + {'aBc': 'd'} + ]) + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +''', actions=''' +foo-bar: + description: "Foos the bar." + params: + foo-name: + description: "A foo name to bar" + type: string + silent: + default: false + description: "" + type: boolean + required: foo-bar + title: foo-bar +start: + description: "Start the unit." + additionalProperties: false +''')) + def test_invalid_action_results(self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture, + bad_res: typing.Dict[str, typing.Any]): class MyCharm(ops.CharmBase): @@ -571,22 +586,38 @@ def __init__(self, *args: typing.Any): def _on_foo_bar_action(self, event: ops.ActionEvent): event.set_results(self.res) - self._setup_test_action() - framework = self.create_framework() + self._setup_test_action(fake_script_fixture.fake_script) charm = MyCharm(framework) - for bad_res in ( - {'a': {'b': 'c'}, 'a.b': 'c'}, - {'a': {'B': 'c'}}, - {'a': {(1, 2): 'c'}}, - {'a': {None: 'c'}}, - {'aBc': 'd'}): - charm.res = bad_res + charm.res = bad_res + with pytest.raises(ValueError): + charm.on.foo_bar_action.emit(id='1') - with pytest.raises(ValueError): - charm.on.foo_bar_action.emit(id='1') + @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +''', actions=''' +foo-bar: + description: "Foos the bar." + params: + foo-name: + description: "A foo name to bar" + type: string + silent: + default: false + description: "" + type: boolean + required: foo-bar + title: foo-bar +start: + description: "Start the unit." + additionalProperties: false +''')) + def test_action_event_defer_fails(self, + monkeypatch: pytest.MonkeyPatch, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): - def _test_action_event_defer_fails(self, cmd_type: str): + cmd_type = 'action' class MyCharm(ops.CharmBase): @@ -597,19 +628,14 @@ def __init__(self, *args: typing.Any): def _on_start_action(self, event: ops.ActionEvent): event.defer() - fake_script(self, f"{cmd_type}-get", """echo '{"foo-name": "name", "silent": true}'""") - self.meta = self._get_action_test_meta() - - os.environ[f'JUJU_{cmd_type.upper()}_NAME'] = 'start' - framework = self.create_framework() + fake_script_fixture.fake_script(f"{cmd_type}-get", + """echo '{"foo-name": "name", "silent": true}'""") + monkeypatch.setenv(f'JUJU_{cmd_type.upper()}_NAME', 'start') charm = MyCharm(framework) with pytest.raises(RuntimeError): charm.on.start_action.emit(id='2') - def test_action_event_defer_fails(self): - self._test_action_event_defer_fails('action') - def test_containers(self): meta = ops.CharmMeta.from_yaml(""" name: k8s-charm @@ -706,7 +732,7 @@ def test_containers_storage_multiple_mounts(self): with pytest.raises(RuntimeError): meta.containers["test1"].mounts["data"].location - def test_secret_events(self): + def test_secret_events(self, framework: ops.Framework): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -738,8 +764,7 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): assert event.revision == 42 self.seen.append(type(event).__name__) - self.meta = ops.CharmMeta.from_yaml(metadata='name: my-charm') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) charm.on.secret_changed.emit('secret:changed', None) charm.on.secret_rotate.emit('secret:rotate', 'rot') @@ -753,7 +778,10 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): 'SecretExpiredEvent', ] - def test_collect_app_status_leader(self): + def test_collect_app_status_leader( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -765,18 +793,21 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('waiting')) event.add_status(ops.BlockedStatus('second')) - fake_script(self, 'is-leader', 'echo true') - fake_script(self, 'status-set', 'exit 0') + fake_script_fixture.fake_script('is-leader', 'echo true') + fake_script_fixture.fake_script('status-set', 'exit 0') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=True', 'blocked', 'first'], ] - def test_collect_app_status_no_statuses(self): + def test_collect_app_status_no_statuses( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -785,16 +816,19 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): pass - fake_script(self, 'is-leader', 'echo true') + fake_script_fixture.fake_script('is-leader', 'echo true') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ] - def test_collect_app_status_non_leader(self): + def test_collect_app_status_non_leader( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -803,16 +837,19 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): raise Exception # shouldn't be called - fake_script(self, 'is-leader', 'echo false') + fake_script_fixture.fake_script('is-leader', 'echo false') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ] - def test_collect_unit_status(self): + def test_collect_unit_status( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -824,18 +861,22 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('waiting')) event.add_status(ops.BlockedStatus('second')) - fake_script(self, 'is-leader', 'echo false') # called only for collecting app statuses - fake_script(self, 'status-set', 'exit 0') + # called only for collecting app statuses + fake_script_fixture.fake_script('is-leader', 'echo false') + fake_script_fixture.fake_script('status-set', 'exit 0') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=False', 'blocked', 'first'], ] - def test_collect_unit_status_no_statuses(self): + def test_collect_unit_status_no_statuses( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -844,16 +885,20 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): pass - fake_script(self, 'is-leader', 'echo false') # called only for collecting app statuses + # called only for collecting app statuses + fake_script_fixture.fake_script('is-leader', 'echo false') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ] - def test_collect_app_and_unit_status(self): + def test_collect_app_and_unit_status( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -866,19 +911,22 @@ def _on_collect_app_status(self, event: ops.CollectStatusEvent): def _on_collect_unit_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('blah')) - fake_script(self, 'is-leader', 'echo true') - fake_script(self, 'status-set', 'exit 0') + fake_script_fixture.fake_script('is-leader', 'echo true') + fake_script_fixture.fake_script('status-set', 'exit 0') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_calls(self, True) == [ + assert fake_script_fixture.fake_script_calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=True', 'active', ''], ['status-set', '--application=False', 'waiting', 'blah'], ] - def test_add_status_type_error(self): + def test_add_status_type_error( + self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -887,13 +935,25 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status('active') # type: ignore - fake_script(self, 'is-leader', 'echo true') + fake_script_fixture.fake_script('is-leader', 'echo true') - charm = MyCharm(self.create_framework()) + charm = MyCharm(framework) with pytest.raises(TypeError): ops.charm._evaluate_status(charm) - def test_collect_status_priority(self): + @pytest.mark.parametrize("statuses,expected", [ + (['blocked', 'error'], 'error'), + (['waiting', 'blocked'], 'blocked'), + (['waiting', 'maintenance'], 'maintenance'), + (['active', 'waiting'], 'waiting'), + (['active', 'unknown'], 'active'), + (['unknown'], 'unknown') + ]) + def test_collect_status_priority(self, + framework: ops.Framework, + fake_script_fixture: FakeScriptFixture, + statuses: typing.List[str], + expected: str): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any, statuses: typing.List[str]): super().__init__(*args) @@ -904,36 +964,16 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): for status in self.statuses: event.add_status(ops.StatusBase.from_name(status, '')) - fake_script(self, 'is-leader', 'echo true') - fake_script(self, 'status-set', 'exit 0') - - charm = MyCharm(self.create_framework(), statuses=['blocked', 'error']) - ops.charm._evaluate_status(charm) - - charm = MyCharm(self.create_framework(), statuses=['waiting', 'blocked']) - ops.charm._evaluate_status(charm) - - charm = MyCharm(self.create_framework(), statuses=['waiting', 'maintenance']) - ops.charm._evaluate_status(charm) - - charm = MyCharm(self.create_framework(), statuses=['active', 'waiting']) - ops.charm._evaluate_status(charm) + fake_script_fixture.fake_script('is-leader', 'echo true') + fake_script_fixture.fake_script('status-set', 'exit 0') - charm = MyCharm(self.create_framework(), statuses=['active', 'unknown']) + charm = MyCharm(framework, statuses=statuses) ops.charm._evaluate_status(charm) - charm = MyCharm(self.create_framework(), statuses=['unknown']) - ops.charm._evaluate_status(charm) - - status_set_calls = [call for call in fake_script_calls(self, True) + status_set_calls = [call for call in fake_script_fixture.fake_script_calls(True) if call[0] == 'status-set'] assert status_set_calls == [ - ['status-set', '--application=True', 'error', ''], - ['status-set', '--application=True', 'blocked', ''], - ['status-set', '--application=True', 'maintenance', ''], - ['status-set', '--application=True', 'waiting', ''], - ['status-set', '--application=True', 'active', ''], - ['status-set', '--application=True', 'unknown', ''], + ['status-set', '--application=True', expected, ''] ] From 56d1afd8310becdf68bb1e66dceaf29047fadff6 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 19 Apr 2024 18:57:31 +0800 Subject: [PATCH 06/16] chore: remove unnecessary type ignores --- test/test_charm.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index 6e1c5a3fe..8e09d82e8 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -409,15 +409,15 @@ def test_relations_meta(self, request: pytest.FixtureRequest,): optional: true ''') - assert meta.requires['database'].interface_name == 'mongodb' # type: ignore - assert meta.requires['database'].limit == 1 # type: ignore - assert meta.requires['database'].scope == 'container' # type: ignore - assert not meta.requires['database'].optional # type: ignore - - assert meta.requires['metrics'].interface_name == 'prometheus-scraping' # type: ignore - assert meta.requires['metrics'].limit is None # type: ignore - assert meta.requires['metrics'].scope == 'global' # Default value # type: ignore - assert meta.requires['metrics'].optional # type: ignore + assert meta.requires['database'].interface_name == 'mongodb' + assert meta.requires['database'].limit == 1 + assert meta.requires['database'].scope == 'container' + assert not meta.requires['database'].optional + + assert meta.requires['metrics'].interface_name == 'prometheus-scraping' + assert meta.requires['metrics'].limit is None + assert meta.requires['metrics'].scope == 'global' # Default value + assert meta.requires['metrics'].optional def test_relations_meta_limit_type_validation(self): with pytest.raises(TypeError, match=r"limit should be an int, not "): From c18f10d9e1654d6cbec8eafcd2bf482e94f74bbb Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 19 Apr 2024 19:47:17 +0800 Subject: [PATCH 07/16] test: refactor according to discussion --- pyproject.toml | 5 - test/test_charm.py | 355 +++++++++++++++++++++---------------------- test/test_helpers.py | 48 +++++- 3 files changed, 217 insertions(+), 191 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a0be21123..f873965c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -196,8 +196,3 @@ reportUnnecessaryIsInstance = false reportUnnecessaryComparison = false disableBytesTypePromotions = false stubPath = "" - -[tool.pytest.ini_options] -markers = [ - "charm_meta", # Custom fixture to pass charm meta to the framework fixture. -] diff --git a/test/test_charm.py b/test/test_charm.py index 8e09d82e8..46471e7ec 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import functools -import os import pathlib import tempfile import typing @@ -24,54 +23,18 @@ import ops import ops.charm -from ops.model import ModelError, _ModelBackend -from ops.storage import SQLiteStorage +from ops.model import ModelError -from .test_helpers import FakeScriptFixture - -# from .test_helpers import fake_script, fake_script_calls - - -@pytest.fixture(autouse=True) -def env(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setenv("PATH", str(Path(__file__).parent / 'bin'), prepend=os.pathsep) - monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") - yield - - -@pytest.fixture(autouse=True) -def events(): - class CustomEvent(ops.EventBase): - pass - - class TestCharmEvents(ops.CharmEvents): - custom = ops.EventSource(CustomEvent) - - # Relations events are defined dynamically and modify the class attributes. - # We use a subclass temporarily to prevent these side effects from leaking. - ops.CharmBase.on = TestCharmEvents() # type: ignore - yield - ops.CharmBase.on = ops.CharmEvents() # type: ignore - - -@pytest.fixture -def framework(request: pytest.FixtureRequest, tmp_path: pathlib.Path): - marker = request.node.get_closest_marker("charm_meta") # type: ignore - meta = marker.args[0] if marker else ops.CharmMeta() # type: ignore - model = ops.Model(meta, _ModelBackend('local/0')) # type: ignore - # we can pass foo_event as event_name because we're not actually testing dispatch - framework = ops.Framework(SQLiteStorage(':memory:'), tmp_path, meta, model) # type: ignore - yield framework - framework.close() +from .test_helpers import FakeScript, create_framework @pytest.fixture def fake_script_fixture(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path): - return FakeScriptFixture(monkeypatch, tmp_path) + return FakeScript(monkeypatch, tmp_path) class TestCharm: - def test_basic(self, framework: ops.Framework): + def test_basic(self, monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -83,16 +46,24 @@ def __init__(self, *args: typing.Any): def _on_start(self, event: ops.EventBase): self.started = True + framework = create_framework(monkeypatch, request) + events: typing.List[str] = list(MyCharm.on.events()) # type: ignore assert 'install' in events assert 'custom' in events + charm = MyCharm(framework) charm.on.start.emit() + assert charm.started + with pytest.raises(TypeError): framework.observe(charm.on.start, charm) # type: ignore - def test_observe_decorated_method(self, framework: ops.Framework): + def test_observe_decorated_method( + self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest): # we test that charm methods decorated with @functools.wraps(wrapper) # can be observed by Framework. Simpler decorators won't work because # Framework searches for __self__ and other method things; functools.wraps @@ -120,6 +91,7 @@ def __init__(self, *args: typing.Any): def _on_start(self, event: ops.EventBase): self.seen = event + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) charm.on.start.emit() # check that the event has been seen by the decorator @@ -130,7 +102,8 @@ def _on_start(self, event: ops.EventBase): def test_observer_not_referenced_warning( self, caplog: pytest.LogCaptureFixture, - framework: ops.Framework): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest): class MyObj(ops.Object): def __init__(self, charm: ops.CharmBase): super().__init__(charm, "obj") @@ -148,6 +121,7 @@ def __init__(self, *args: typing.Any): def _on_start(self, _: ops.StartEvent): pass # is reached + framework = create_framework(monkeypatch, request) c = MyCharm(framework) c.on.start.emit() assert 'Reference to ops.Object' in caplog.text @@ -156,10 +130,13 @@ def test_empty_action(self): meta = ops.CharmMeta.from_yaml('name: my-charm', '') assert meta.actions == {} - def test_helper_properties(self, framework: ops.Framework): + def test_helper_properties(self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): pass + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) assert charm.app == framework.model.app assert charm.unit == framework.model.unit @@ -167,25 +144,9 @@ class MyCharm(ops.CharmBase): assert charm.charm_dir == framework.charm_dir assert charm.config is framework.model.config - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -requires: - req1: - interface: req1 - req-2: - interface: req2 -provides: - pro1: - interface: pro1 - pro-2: - interface: pro2 -peers: - peer1: - interface: peer1 - peer-2: - interface: peer2 -''')) - def test_relation_events(self, framework: ops.Framework): + def test_relation_events(self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -204,6 +165,25 @@ def on_any_relation(self, event: ops.RelationEvent): assert event.relation.app.name == 'remote' self.seen.append(type(event).__name__) + meta = ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +requires: + req1: + interface: req1 + req-2: + interface: req2 +provides: + pro1: + interface: pro1 + pro-2: + interface: pro2 +peers: + peer1: + interface: peer1 + peer-2: + interface: peer2 +''') + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) assert 'pro_2_relation_broken' in repr(charm.on) @@ -231,7 +211,33 @@ def on_any_relation(self, event: ops.RelationEvent): 'RelationBrokenEvent', ] - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(''' + def test_storage_events(self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) + self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) + self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) + self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) + + def _on_stor1_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + assert event.storage.location == Path("/var/srv/stor1/0") + + def _on_stor2_detach(self, event: ops.StorageDetachingEvent): + self.seen.append(type(event).__name__) + + def _on_stor3_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + def _on_stor4_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + meta = ops.CharmMeta.from_yaml(''' name: my-charm storage: stor-4: @@ -256,34 +262,15 @@ def on_any_relation(self, event: ops.RelationEvent): multiple: range: 10+ type: filesystem -''')) - def test_storage_events(self, - request: pytest.FixtureRequest, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) - self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) - self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) - self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) - - def _on_stor1_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - assert event.storage.location == Path("/var/srv/stor1/0") - - def _on_stor2_detach(self, event: ops.StorageDetachingEvent): - self.seen.append(type(event).__name__) - - def _on_stor3_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) +''') - def _on_stor4_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) + assert meta.storages['stor1'].multiple_range is None # type: ignore + assert meta.storages['stor2'].multiple_range == (2, 2) # type: ignore + assert meta.storages['stor3'].multiple_range == (2, None) # type: ignore + assert meta.storages['stor-4'].multiple_range == (2, 4) # type: ignore + assert meta.storages['stor-plus'].multiple_range == (10, None) # type: ignore - fake_script_fixture.fake_script( + fake_script_fixture.write( "storage-get", """ if [ "$1" = "-s" ]; then @@ -315,20 +302,14 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): fi """, ) - fake_script_fixture.fake_script( + fake_script_fixture.write( "storage-list", """ echo '["disks/0"]' """, ) - meta = request.node.get_closest_marker("charm_meta").args[0] # type: ignore - assert meta.storages['stor1'].multiple_range is None # type: ignore - assert meta.storages['stor2'].multiple_range == (2, 2) # type: ignore - assert meta.storages['stor3'].multiple_range == (2, None) # type: ignore - assert meta.storages['stor-4'].multiple_range == (2, 4) # type: ignore - assert meta.storages['stor-plus'].multiple_range == (10, None) # type: ignore - + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) charm.on['stor1'].storage_attached.emit(ops.Storage("stor1", 0, charm.model._backend)) @@ -345,13 +326,9 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): 'StorageAttachedEvent', ] - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -containers: - container-a: - containerb: -''')) - def test_workload_events(self, framework: ops.Framework): + def test_workload_events(self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -373,6 +350,13 @@ def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self.seen.append(type(event).__name__) + meta = ops.CharmMeta.from_yaml(metadata=''' +name: my-charm +containers: + container-a: + containerb: +''') + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) assert 'container_a_pebble_ready' in repr(charm.on) @@ -396,7 +380,7 @@ def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): ] @pytest.mark.charm_meta() - def test_relations_meta(self, request: pytest.FixtureRequest,): + def test_relations_meta(self): meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: @@ -421,7 +405,6 @@ def test_relations_meta(self, request: pytest.FixtureRequest,): def test_relations_meta_limit_type_validation(self): with pytest.raises(TypeError, match=r"limit should be an int, not "): - # language=YAML ops.CharmMeta.from_yaml(''' name: my-charm requires: @@ -435,7 +418,6 @@ def test_relations_meta_scope_type_validation(self): TypeError, match="scope should be one of 'global', 'container'; not 'foobar'" ): - # language=YAML ops.CharmMeta.from_yaml(''' name: my-charm requires: @@ -489,7 +471,9 @@ def _setup_test_action(self, fake_script: typing.Callable[..., None]): fake_script('action-log', "") fake_script('action-fail', "") - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' + @classmethod + def _get_action_test_meta(cls): + return ops.CharmMeta.from_yaml(metadata=''' name: my-charm ''', actions=''' foo-bar: @@ -507,10 +491,12 @@ def _setup_test_action(self, fake_script: typing.Callable[..., None]): start: description: "Start the unit." additionalProperties: false -''')) +''') + def test_action_events(self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): @@ -528,7 +514,9 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): def _on_start_action(self, event: ops.ActionEvent): pass - self._setup_test_action(fake_script_fixture.fake_script) + self._setup_test_action(fake_script_fixture.write) + meta = self._get_action_test_meta() + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) events: typing.List[str] = list(MyCharm.on.events()) # type: ignore @@ -538,7 +526,7 @@ def _on_start_action(self, event: ops.ActionEvent): action_id = "1234" charm.on.foo_bar_action.emit(id=action_id) assert charm.seen_action_params == {"foo-name": "name", "silent": True} - assert fake_script_fixture.fake_script_calls() == [ + assert fake_script_fixture.calls() == [ ['action-get', '--format=json'], ['action-log', "test-log"], ['action-set', "res=val with spaces", f"id={action_id}"], @@ -552,28 +540,10 @@ def _on_start_action(self, event: ops.ActionEvent): {'a': {None: 'c'}}, {'aBc': 'd'} ]) - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -''', actions=''' -foo-bar: - description: "Foos the bar." - params: - foo-name: - description: "A foo name to bar" - type: string - silent: - default: false - description: "" - type: boolean - required: foo-bar - title: foo-bar -start: - description: "Start the unit." - additionalProperties: false -''')) def test_invalid_action_results(self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript, bad_res: typing.Dict[str, typing.Any]): class MyCharm(ops.CharmBase): @@ -586,7 +556,9 @@ def __init__(self, *args: typing.Any): def _on_foo_bar_action(self, event: ops.ActionEvent): event.set_results(self.res) - self._setup_test_action(fake_script_fixture.fake_script) + self._setup_test_action(fake_script_fixture.write) + meta = self._get_action_test_meta() + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) charm.res = bad_res @@ -612,10 +584,11 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): description: "Start the unit." additionalProperties: false ''')) - def test_action_event_defer_fails(self, - monkeypatch: pytest.MonkeyPatch, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + def test_action_event_defer_fails( + self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): cmd_type = 'action' @@ -628,9 +601,11 @@ def __init__(self, *args: typing.Any): def _on_start_action(self, event: ops.ActionEvent): event.defer() - fake_script_fixture.fake_script(f"{cmd_type}-get", - """echo '{"foo-name": "name", "silent": true}'""") + fake_script_fixture.write(f"{cmd_type}-get", + """echo '{"foo-name": "name", "silent": true}'""") monkeypatch.setenv(f'JUJU_{cmd_type.upper()}_NAME', 'start') + meta = self._get_action_test_meta() + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) with pytest.raises(RuntimeError): @@ -732,7 +707,7 @@ def test_containers_storage_multiple_mounts(self): with pytest.raises(RuntimeError): meta.containers["test1"].mounts["data"].location - def test_secret_events(self, framework: ops.Framework): + def test_secret_events(self, monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -764,6 +739,7 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): assert event.revision == 42 self.seen.append(type(event).__name__) + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) charm.on.secret_changed.emit('secret:changed', None) @@ -780,8 +756,9 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): def test_collect_app_status_leader( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -793,21 +770,23 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('waiting')) event.add_status(ops.BlockedStatus('second')) - fake_script_fixture.fake_script('is-leader', 'echo true') - fake_script_fixture.fake_script('status-set', 'exit 0') + fake_script_fixture.write('is-leader', 'echo true') + fake_script_fixture.write('status-set', 'exit 0') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=True', 'blocked', 'first'], ] def test_collect_app_status_no_statuses( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -816,19 +795,21 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): pass - fake_script_fixture.fake_script('is-leader', 'echo true') + fake_script_fixture.write('is-leader', 'echo true') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ] def test_collect_app_status_non_leader( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -837,19 +818,21 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): raise Exception # shouldn't be called - fake_script_fixture.fake_script('is-leader', 'echo false') + fake_script_fixture.write('is-leader', 'echo false') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ] def test_collect_unit_status( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -862,21 +845,23 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.BlockedStatus('second')) # called only for collecting app statuses - fake_script_fixture.fake_script('is-leader', 'echo false') - fake_script_fixture.fake_script('status-set', 'exit 0') + fake_script_fixture.write('is-leader', 'echo false') + fake_script_fixture.write('status-set', 'exit 0') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=False', 'blocked', 'first'], ] def test_collect_unit_status_no_statuses( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -886,19 +871,21 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): pass # called only for collecting app statuses - fake_script_fixture.fake_script('is-leader', 'echo false') + fake_script_fixture.write('is-leader', 'echo false') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ] def test_collect_app_and_unit_status( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -911,13 +898,14 @@ def _on_collect_app_status(self, event: ops.CollectStatusEvent): def _on_collect_unit_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('blah')) - fake_script_fixture.fake_script('is-leader', 'echo true') - fake_script_fixture.fake_script('status-set', 'exit 0') + fake_script_fixture.write('is-leader', 'echo true') + fake_script_fixture.write('status-set', 'exit 0') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) ops.charm._evaluate_status(charm) - assert fake_script_fixture.fake_script_calls(True) == [ + assert fake_script_fixture.calls(True) == [ ['is-leader', '--format=json'], ['status-set', '--application=True', 'active', ''], ['status-set', '--application=False', 'waiting', 'blah'], @@ -925,8 +913,9 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent): def test_add_status_type_error( self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -935,8 +924,9 @@ def __init__(self, *args: typing.Any): def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status('active') # type: ignore - fake_script_fixture.fake_script('is-leader', 'echo true') + fake_script_fixture.write('is-leader', 'echo true') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework) with pytest.raises(TypeError): ops.charm._evaluate_status(charm) @@ -949,11 +939,13 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): (['active', 'unknown'], 'active'), (['unknown'], 'unknown') ]) - def test_collect_status_priority(self, - framework: ops.Framework, - fake_script_fixture: FakeScriptFixture, - statuses: typing.List[str], - expected: str): + def test_collect_status_priority( + self, + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script_fixture: FakeScript, + statuses: typing.List[str], + expected: str): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any, statuses: typing.List[str]): super().__init__(*args) @@ -964,13 +956,14 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): for status in self.statuses: event.add_status(ops.StatusBase.from_name(status, '')) - fake_script_fixture.fake_script('is-leader', 'echo true') - fake_script_fixture.fake_script('status-set', 'exit 0') + fake_script_fixture.write('is-leader', 'echo true') + fake_script_fixture.write('status-set', 'exit 0') + framework = create_framework(monkeypatch, request) charm = MyCharm(framework, statuses=statuses) ops.charm._evaluate_status(charm) - status_set_calls = [call for call in fake_script_fixture.fake_script_calls(True) + status_set_calls = [call for call in fake_script_fixture.calls(True) if call[0] == 'status-set'] assert status_set_calls == [ ['status-set', '--application=True', expected, ''] diff --git a/test/test_helpers.py b/test/test_helpers.py index c07789a83..154166d89 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import functools import os import pathlib import shutil @@ -76,7 +77,44 @@ def fake_script_calls(test_case: unittest.TestCase, return calls # type: ignore -class FakeScriptFixture: +def create_framework( + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + *, + meta: typing.Union[ops.CharmMeta, None] = None +): + monkeypatch.setenv("PATH", str(pathlib.Path(__file__).parent / 'bin'), prepend=os.pathsep) + monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") + + tmpdir = pathlib.Path(tempfile.mkdtemp()) + request.addfinalizer(functools.partial(shutil.rmtree, str(tmpdir))) + + class CustomEvent(ops.EventBase): + pass + + class TestCharmEvents(ops.CharmEvents): + custom = ops.EventSource(CustomEvent) + + # Relations events are defined dynamically and modify the class attributes. + # We use a subclass temporarily to prevent these side effects from leaking. + ops.CharmBase.on = TestCharmEvents() # type: ignore + + def cleanup(): + ops.CharmBase.on = ops.CharmEvents() # type: ignore + + request.addfinalizer(cleanup) + + if not meta: + meta = ops.CharmMeta() + model = ops.Model(meta, _ModelBackend('local/0')) # type: ignore + # we can pass foo_event as event_name because we're not actually testing dispatch + framework = ops.Framework(SQLiteStorage(':memory:'), tmpdir, meta, model) # type: ignore + request.addfinalizer(framework.close) + + return framework + + +class FakeScript: def __init__(self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path, @@ -91,9 +129,9 @@ def __init__(self, else: self.fake_script_path = fake_script_path - def fake_script(self, - name: str, - content: str): + def write(self, + name: str, + content: str): template_args: typing.Dict[str, str] = { 'name': name, 'path': self.fake_script_path.as_posix(), # type: ignore @@ -115,7 +153,7 @@ def fake_script(self, path.with_suffix(".bat").write_text( # type: ignore f'@"C:\\Program Files\\git\\bin\\bash.exe" {path} %*\n') - def fake_script_calls(self, clear: bool = False) -> typing.List[typing.List[str]]: + def calls(self, clear: bool = False) -> typing.List[typing.List[str]]: calls_file: pathlib.Path = self.fake_script_path / 'calls.txt' # type: ignore if not calls_file.exists(): # type: ignore return [] From c5e81665163f9a9e20ebe1cfe539a6cc17e7fe01 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 19 Apr 2024 19:48:41 +0800 Subject: [PATCH 08/16] chore: fmt --- test/test_helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_helpers.py b/test/test_helpers.py index 154166d89..31b86961a 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -81,8 +81,7 @@ def create_framework( monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest, *, - meta: typing.Union[ops.CharmMeta, None] = None -): + meta: typing.Union[ops.CharmMeta, None] = None): monkeypatch.setenv("PATH", str(pathlib.Path(__file__).parent / 'bin'), prepend=os.pathsep) monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") From 8f8bd781247d676a418c72b2d6c4790d8b45f84f Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 19 Apr 2024 19:50:53 +0800 Subject: [PATCH 09/16] chore: merge main --- test/test_jujuversion.py | 205 ++++++++++++++------------- test/test_log.py | 118 ++++++++------- test/test_pebble.py | 299 ++++++++++++++++++++------------------- test/test_private.py | 158 +++++++++++---------- 4 files changed, 396 insertions(+), 384 deletions(-) diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py index f9b2bdb52..644cb747c 100644 --- a/test/test_jujuversion.py +++ b/test/test_jujuversion.py @@ -20,25 +20,28 @@ import ops -class TestJujuVersion: - @pytest.mark.parametrize("vs,major,minor,tag,patch,build", [ - ("0.0.0", 0, 0, '', 0, 0), - ("0.0.2", 0, 0, '', 2, 0), - ("0.1.0", 0, 1, '', 0, 0), - ("0.2.3", 0, 2, '', 3, 0), - ("10.234.3456", 10, 234, '', 3456, 0), - ("10.234.3456.1", 10, 234, '', 3456, 1), - ("1.21-alpha12", 1, 21, 'alpha', 12, 0), - ("1.21-alpha1.34", 1, 21, 'alpha', 1, 34), - ("2.7", 2, 7, '', 0, 0) - ]) - def test_parsing(self, vs: str, major: int, minor: int, tag: str, patch: int, build: int): - v = ops.JujuVersion(vs) - assert v.major == major - assert v.minor == minor - assert v.tag == tag - assert v.patch == patch - assert v.build == build +class TestJujuVersion(unittest.TestCase): + + def test_parsing(self): + test_cases = [ + ("0.0.0", 0, 0, '', 0, 0), + ("0.0.2", 0, 0, '', 2, 0), + ("0.1.0", 0, 1, '', 0, 0), + ("0.2.3", 0, 2, '', 3, 0), + ("10.234.3456", 10, 234, '', 3456, 0), + ("10.234.3456.1", 10, 234, '', 3456, 1), + ("1.21-alpha12", 1, 21, 'alpha', 12, 0), + ("1.21-alpha1.34", 1, 21, 'alpha', 1, 34), + ("2.7", 2, 7, '', 0, 0) + ] + + for vs, major, minor, tag, patch, build in test_cases: + v = ops.JujuVersion(vs) + assert v.major == major + assert v.minor == minor + assert v.tag == tag + assert v.patch == patch + assert v.build == build @unittest.mock.patch('os.environ', new={}) # type: ignore def test_from_environ(self): @@ -90,82 +93,88 @@ def test_supports_exec_service_context(self): assert ops.JujuVersion('3.3.0').supports_exec_service_context assert ops.JujuVersion('3.4.0').supports_exec_service_context - @pytest.mark.parametrize("invalid_version", [ - "xyz", - "foo.bar", - "foo.bar.baz", - "dead.beef.ca.fe", - "1234567890.2.1", # The major version is too long. - "0.2..1", # Two periods next to each other. - "1.21.alpha1", # Tag comes after period. - "1.21-alpha", # No patch number but a tag is present. - "1.21-alpha1beta", # Non-numeric string after the patch number. - "1.21-alpha-dev", # Tag duplication. - "1.21-alpha_dev3", # Underscore in a tag. - "1.21-alpha123dev3", # Non-numeric string after the patch number. - ]) - def test_parsing_errors(self, invalid_version: str): - with pytest.raises(RuntimeError): - ops.JujuVersion(invalid_version) - - @pytest.mark.parametrize("a,b,expected", [ - ("1.0.0", "1.0.0", True), - ("01.0.0", "1.0.0", True), - ("10.0.0", "9.0.0", False), - ("1.0.0", "1.0.1", False), - ("1.0.1", "1.0.0", False), - ("1.0.0", "1.1.0", False), - ("1.1.0", "1.0.0", False), - ("1.0.0", "2.0.0", False), - ("1.2-alpha1", "1.2.0", False), - ("1.2-alpha2", "1.2-alpha1", False), - ("1.2-alpha2.1", "1.2-alpha2", False), - ("1.2-alpha2.2", "1.2-alpha2.1", False), - ("1.2-beta1", "1.2-alpha1", False), - ("1.2-beta1", "1.2-alpha2.1", False), - ("1.2-beta1", "1.2.0", False), - ("1.2.1", "1.2.0", False), - ("2.0.0", "1.0.0", False), - ("2.0.0.0", "2.0.0", True), - ("2.0.0.0", "2.0.0.0", True), - ("2.0.0.1", "2.0.0.0", False), - ("2.0.1.10", "2.0.0.0", False), - ]) - def test_equality(self, a: str, b: str, expected: bool): - assert (ops.JujuVersion(a) == ops.JujuVersion(b)) == expected - assert (ops.JujuVersion(a) == b) == expected - - @pytest.mark.parametrize("a,b,expected_strict,expected_weak", [ - ("1.0.0", "1.0.0", False, True), - ("01.0.0", "1.0.0", False, True), - ("10.0.0", "9.0.0", False, False), - ("1.0.0", "1.0.1", True, True), - ("1.0.1", "1.0.0", False, False), - ("1.0.0", "1.1.0", True, True), - ("1.1.0", "1.0.0", False, False), - ("1.0.0", "2.0.0", True, True), - ("1.2-alpha1", "1.2.0", True, True), - ("1.2-alpha2", "1.2-alpha1", False, False), - ("1.2-alpha2.1", "1.2-alpha2", False, False), - ("1.2-alpha2.2", "1.2-alpha2.1", False, False), - ("1.2-beta1", "1.2-alpha1", False, False), - ("1.2-beta1", "1.2-alpha2.1", False, False), - ("1.2-beta1", "1.2.0", True, True), - ("1.2.1", "1.2.0", False, False), - ("2.0.0", "1.0.0", False, False), - ("2.0.0.0", "2.0.0", False, True), - ("2.0.0.0", "2.0.0.0", False, True), - ("2.0.0.1", "2.0.0.0", False, False), - ("2.0.1.10", "2.0.0.0", False, False), - ("2.10.0", "2.8.0", False, False), - ]) - def test_comparison(self, a: str, b: str, expected_strict: bool, expected_weak: bool): - assert (ops.JujuVersion(a) < ops.JujuVersion(b)) == expected_strict - assert (ops.JujuVersion(a) <= ops.JujuVersion(b)) == expected_weak - assert (ops.JujuVersion(b) > ops.JujuVersion(a)) == expected_strict - assert (ops.JujuVersion(b) >= ops.JujuVersion(a)) == expected_weak - # Implicit conversion. - assert (ops.JujuVersion(a) < b) == expected_strict - assert (ops.JujuVersion(a) <= b) == expected_weak - assert (b > ops.JujuVersion(a)) == expected_strict - assert (b >= ops.JujuVersion(a)) == expected_weak + def test_parsing_errors(self): + invalid_versions = [ + "xyz", + "foo.bar", + "foo.bar.baz", + "dead.beef.ca.fe", + "1234567890.2.1", # The major version is too long. + "0.2..1", # Two periods next to each other. + "1.21.alpha1", # Tag comes after period. + "1.21-alpha", # No patch number but a tag is present. + "1.21-alpha1beta", # Non-numeric string after the patch number. + "1.21-alpha-dev", # Tag duplication. + "1.21-alpha_dev3", # Underscore in a tag. + "1.21-alpha123dev3", # Non-numeric string after the patch number. + ] + for v in invalid_versions: + with pytest.raises(RuntimeError): + ops.JujuVersion(v) + + def test_equality(self): + test_cases = [ + ("1.0.0", "1.0.0", True), + ("01.0.0", "1.0.0", True), + ("10.0.0", "9.0.0", False), + ("1.0.0", "1.0.1", False), + ("1.0.1", "1.0.0", False), + ("1.0.0", "1.1.0", False), + ("1.1.0", "1.0.0", False), + ("1.0.0", "2.0.0", False), + ("1.2-alpha1", "1.2.0", False), + ("1.2-alpha2", "1.2-alpha1", False), + ("1.2-alpha2.1", "1.2-alpha2", False), + ("1.2-alpha2.2", "1.2-alpha2.1", False), + ("1.2-beta1", "1.2-alpha1", False), + ("1.2-beta1", "1.2-alpha2.1", False), + ("1.2-beta1", "1.2.0", False), + ("1.2.1", "1.2.0", False), + ("2.0.0", "1.0.0", False), + ("2.0.0.0", "2.0.0", True), + ("2.0.0.0", "2.0.0.0", True), + ("2.0.0.1", "2.0.0.0", False), + ("2.0.1.10", "2.0.0.0", False), + ] + + for a, b, expected in test_cases: + assert (ops.JujuVersion(a) == ops.JujuVersion(b)) == expected + assert (ops.JujuVersion(a) == b) == expected + + def test_comparison(self): + test_cases = [ + ("1.0.0", "1.0.0", False, True), + ("01.0.0", "1.0.0", False, True), + ("10.0.0", "9.0.0", False, False), + ("1.0.0", "1.0.1", True, True), + ("1.0.1", "1.0.0", False, False), + ("1.0.0", "1.1.0", True, True), + ("1.1.0", "1.0.0", False, False), + ("1.0.0", "2.0.0", True, True), + ("1.2-alpha1", "1.2.0", True, True), + ("1.2-alpha2", "1.2-alpha1", False, False), + ("1.2-alpha2.1", "1.2-alpha2", False, False), + ("1.2-alpha2.2", "1.2-alpha2.1", False, False), + ("1.2-beta1", "1.2-alpha1", False, False), + ("1.2-beta1", "1.2-alpha2.1", False, False), + ("1.2-beta1", "1.2.0", True, True), + ("1.2.1", "1.2.0", False, False), + ("2.0.0", "1.0.0", False, False), + ("2.0.0.0", "2.0.0", False, True), + ("2.0.0.0", "2.0.0.0", False, True), + ("2.0.0.1", "2.0.0.0", False, False), + ("2.0.1.10", "2.0.0.0", False, False), + ("2.10.0", "2.8.0", False, False), + ] + + for a, b, expected_strict, expected_weak in test_cases: + with self.subTest(a=a, b=b): + assert (ops.JujuVersion(a) < ops.JujuVersion(b)) == expected_strict + assert (ops.JujuVersion(a) <= ops.JujuVersion(b)) == expected_weak + assert (ops.JujuVersion(b) > ops.JujuVersion(a)) == expected_strict + assert (ops.JujuVersion(b) >= ops.JujuVersion(a)) == expected_weak + # Implicit conversion. + assert (ops.JujuVersion(a) < b) == expected_strict + assert (ops.JujuVersion(a) <= b) == expected_weak + assert (b > ops.JujuVersion(a)) == expected_strict + assert (b >= ops.JujuVersion(a)) == expected_weak diff --git a/test/test_log.py b/test/test_log.py index 11f052a09..1f6da2fdc 100644 --- a/test/test_log.py +++ b/test/test_log.py @@ -19,8 +19,6 @@ import unittest from unittest.mock import patch -import pytest - import ops.log from ops.model import MAX_LOG_LINE_LEN, _ModelBackend @@ -41,78 +39,76 @@ def juju_log(self, level: str, message: str): self._calls.append((level, line)) -@pytest.fixture() -def backend(): - return FakeModelBackend() - - -@pytest.fixture() -def logger(): - logger = logging.getLogger() - yield logger - logging.getLogger().handlers.clear() - - -class TestLogging: - @pytest.mark.parametrize("message,result", [ - ('critical', ('CRITICAL', 'critical')), - ('error', ('ERROR', 'error')), - ('warning', ('WARNING', 'warning')), - ('info', ('INFO', 'info')), - ('debug', ('DEBUG', 'debug')), - ]) - def test_default_logging(self, - backend: FakeModelBackend, - logger: logging.Logger, - message: str, - result: typing.Tuple[str, str]): - ops.log.setup_root_logging(backend) +class TestLogging(unittest.TestCase): + + def setUp(self): + self.backend = FakeModelBackend() + + def tearDown(self): + logging.getLogger().handlers.clear() + + def test_default_logging(self): + ops.log.setup_root_logging(self.backend) + + logger = logging.getLogger() assert logger.level == logging.DEBUG assert isinstance(logger.handlers[-1], ops.log.JujuLogHandler) - method = getattr(logger, message) - method(message) - calls = backend.calls(clear=True) - assert calls == [result] + test_cases = [ + (logger.critical, 'critical', ('CRITICAL', 'critical')), + (logger.error, 'error', ('ERROR', 'error')), + (logger.warning, 'warning', ('WARNING', 'warning')), + (logger.info, 'info', ('INFO', 'info')), + (logger.debug, 'debug', ('DEBUG', 'debug')), + ] + + for method, message, result in test_cases: + with self.subTest(message): + method(message) + calls = self.backend.calls(clear=True) + assert calls == [result] - def test_handler_filtering(self, backend: FakeModelBackend, logger: logging.Logger): + def test_handler_filtering(self): + logger = logging.getLogger() logger.setLevel(logging.INFO) - logger.addHandler(ops.log.JujuLogHandler(backend, logging.WARNING)) + logger.addHandler(ops.log.JujuLogHandler(self.backend, logging.WARNING)) logger.info('foo') - assert backend.calls() == [] + assert self.backend.calls() == [] logger.warning('bar') - assert backend.calls() == [('WARNING', 'bar')] + assert self.backend.calls() == [('WARNING', 'bar')] - def test_no_stderr_without_debug(self, backend: FakeModelBackend, logger: logging.Logger): + def test_no_stderr_without_debug(self): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(backend, debug=False) + ops.log.setup_root_logging(self.backend, debug=False) + logger = logging.getLogger() logger.debug('debug message') logger.info('info message') logger.warning('warning message') logger.critical('critical message') - assert backend.calls() == [ - ('DEBUG', 'debug message'), - ('INFO', 'info message'), - ('WARNING', 'warning message'), - ('CRITICAL', 'critical message'), - ] + assert self.backend.calls() == \ + [('DEBUG', 'debug message'), + ('INFO', 'info message'), + ('WARNING', 'warning message'), + ('CRITICAL', 'critical message'), + ] assert buffer.getvalue() == "" - def test_debug_logging(self, backend: FakeModelBackend, logger: logging.Logger): + def test_debug_logging(self): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(backend, debug=True) + ops.log.setup_root_logging(self.backend, debug=True) + logger = logging.getLogger() logger.debug('debug message') logger.info('info message') logger.warning('warning message') logger.critical('critical message') - assert backend.calls() == [ - ('DEBUG', 'debug message'), - ('INFO', 'info message'), - ('WARNING', 'warning message'), - ('CRITICAL', 'critical message'), - ] + assert self.backend.calls() == \ + [('DEBUG', 'debug message'), + ('INFO', 'info message'), + ('WARNING', 'warning message'), + ('CRITICAL', 'critical message'), + ] assert re.search( r"\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d,\d\d\d DEBUG debug message\n" r"\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d,\d\d\d INFO info message\n" @@ -121,29 +117,31 @@ def test_debug_logging(self, backend: FakeModelBackend, logger: logging.Logger): buffer.getvalue() ) - def test_reduced_logging(self, backend: FakeModelBackend, logger: logging.Logger): - ops.log.setup_root_logging(backend) + def test_reduced_logging(self): + ops.log.setup_root_logging(self.backend) + logger = logging.getLogger() logger.setLevel(logging.WARNING) logger.debug('debug') logger.info('info') logger.warning('warning') - assert backend.calls() == [('WARNING', 'warning')] + assert self.backend.calls() == [('WARNING', 'warning')] - def test_long_string_logging(self, backend: FakeModelBackend, logger: logging.Logger): + def test_long_string_logging(self): buffer = io.StringIO() with patch('sys.stderr', buffer): - ops.log.setup_root_logging(backend, debug=True) + ops.log.setup_root_logging(self.backend, debug=True) + logger = logging.getLogger() logger.debug('l' * MAX_LOG_LINE_LEN) - assert len(backend.calls()) == 1 + assert len(self.backend.calls()) == 1 - backend.calls(clear=True) + self.backend.calls(clear=True) with patch('sys.stderr', buffer): logger.debug('l' * (MAX_LOG_LINE_LEN + 9)) - calls = backend.calls() + calls = self.backend.calls() assert len(calls) == 3 # Verify that we note that we are splitting the log message. assert "Splitting into multiple chunks" in calls[0][1] diff --git a/test/test_pebble.py b/test/test_pebble.py index 04086921b..08470dfae 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -1426,158 +1426,159 @@ def build_mock_change_dict(change_id: str = '70') -> 'pebble._ChangeDict': } -class MultipartParserTestCase: - def __init__( - self, - name: str, - data: bytes, - want_headers: typing.List[bytes], - want_bodies: typing.List[bytes], - want_bodies_done: typing.List[bool], - max_boundary: int = 14, - max_lookahead: int = 8 * 1024, - error: str = ''): - self.name = name - self.data = data - self.want_headers = want_headers - self.want_bodies = want_bodies - self.want_bodies_done = want_bodies_done - self.max_boundary = max_boundary - self.max_lookahead = max_lookahead - self.error = error - - -class TestMultipartParser: - @pytest.mark.parametrize("test", [ - MultipartParserTestCase( - 'baseline', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\nfoo bar\r\n--qwerty--\r\n', - [b'header foo\r\n\r\n'], - [b'foo bar\nfoo bar'], - want_bodies_done=[True], - ), - MultipartParserTestCase( - 'incomplete header', - b'\r\n--qwerty\r\nheader foo\r\n', - [], - [], - want_bodies_done=[], - ), - MultipartParserTestCase( - 'missing header', - b'\r\n--qwerty\r\nheader foo\r\n' + 40 * b' ', - [], - [], - want_bodies_done=[], - max_lookahead=40, - error='header terminator not found', - ), - MultipartParserTestCase( - 'incomplete body terminator', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\rhello my name is joe and I work in a button factory', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar\r\n--qwerty\rhello my name is joe and I work in a '], - want_bodies_done=[False], - ), - MultipartParserTestCase( - 'empty body', - b'\r\n--qwerty\r\nheader foo\r\n\r\n\r\n--qwerty\r\n', - [b'header foo\r\n\r\n'], - [b''], - want_bodies_done=[True], - ), - MultipartParserTestCase( - 'ignore leading garbage', - b'hello my name is joe\r\n\n\n\n\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - MultipartParserTestCase( - 'ignore trailing garbage', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nhello my name is joe', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - MultipartParserTestCase( - 'boundary allow linear whitespace', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - max_boundary=20, - ), - MultipartParserTestCase( - 'terminal boundary allow linear whitespace', - b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty-- \t \r\n', - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - max_boundary=20, - ), - MultipartParserTestCase( - 'multiple parts', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa - [b'header foo\r\n\r\n', b'header bar\r\n\r\n'], - [b'foo bar', b'foo baz'], - want_bodies_done=[True, True], - ), - MultipartParserTestCase( - 'ignore after terminal boundary', - b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty--\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa - [b'header foo\r\n\r\n'], - [b'foo bar'], - want_bodies_done=[True], - ), - ]) - def test_multipart_parser(self, test: MultipartParserTestCase): +class TestMultipartParser(unittest.TestCase): + class _Case: + def __init__( + self, + name: str, + data: bytes, + want_headers: typing.List[bytes], + want_bodies: typing.List[bytes], + want_bodies_done: typing.List[bool], + max_boundary: int = 14, + max_lookahead: int = 8 * 1024, + error: str = ''): + self.name = name + self.data = data + self.want_headers = want_headers + self.want_bodies = want_bodies + self.want_bodies_done = want_bodies_done + self.max_boundary = max_boundary + self.max_lookahead = max_lookahead + self.error = error + + def test_multipart_parser(self): + tests = [ + TestMultipartParser._Case( + 'baseline', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\nfoo bar\r\n--qwerty--\r\n', + [b'header foo\r\n\r\n'], + [b'foo bar\nfoo bar'], + want_bodies_done=[True], + ), + TestMultipartParser._Case( + 'incomplete header', + b'\r\n--qwerty\r\nheader foo\r\n', + [], + [], + want_bodies_done=[], + ), + TestMultipartParser._Case( + 'missing header', + b'\r\n--qwerty\r\nheader foo\r\n' + 40 * b' ', + [], + [], + want_bodies_done=[], + max_lookahead=40, + error='header terminator not found', + ), + TestMultipartParser._Case( + 'incomplete body terminator', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\rhello my name is joe and I work in a button factory', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar\r\n--qwerty\rhello my name is joe and I work in a '], + want_bodies_done=[False], + ), + TestMultipartParser._Case( + 'empty body', + b'\r\n--qwerty\r\nheader foo\r\n\r\n\r\n--qwerty\r\n', + [b'header foo\r\n\r\n'], + [b''], + want_bodies_done=[True], + ), + TestMultipartParser._Case( + 'ignore leading garbage', + b'hello my name is joe\r\n\n\n\n\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + TestMultipartParser._Case( + 'ignore trailing garbage', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nhello my name is joe', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + TestMultipartParser._Case( + 'boundary allow linear whitespace', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\n', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + max_boundary=20, + ), + TestMultipartParser._Case( + 'terminal boundary allow linear whitespace', + b'\r\n--qwerty\r\nheader foo\r\n\r\nfoo bar\r\n--qwerty-- \t \r\n', + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + max_boundary=20, + ), + TestMultipartParser._Case( + 'multiple parts', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa + [b'header foo\r\n\r\n', b'header bar\r\n\r\n'], + [b'foo bar', b'foo baz'], + want_bodies_done=[True, True], + ), + TestMultipartParser._Case( + 'ignore after terminal boundary', + b'\r\n--qwerty \t \r\nheader foo\r\n\r\nfoo bar\r\n--qwerty--\r\nheader bar\r\n\r\nfoo baz\r\n--qwerty--\r\n', # noqa + [b'header foo\r\n\r\n'], + [b'foo bar'], + want_bodies_done=[True], + ), + ] + chunk_sizes = [1, 2, 3, 4, 5, 7, 13, 17, 19, 23, 29, 31, 37, 42, 50, 100, 1000] marker = b'qwerty' - for chunk_size in chunk_sizes: - headers: typing.List[bytes] = [] - bodies: typing.List[bytes] = [] - bodies_done: typing.List[bool] = [] - - # All of the "noqa: B023" here are due to a ruff bug: - # https://github.com/astral-sh/ruff/issues/7847 - # ruff should tell us when the 'noqa's are no longer required. - def handle_header(data: typing.Any): - headers.append(bytes(data)) # noqa: B023 - bodies.append(b'') # noqa: B023 - bodies_done.append(False) # noqa: B023 - - def handle_body(data: bytes, done: bool = False): - bodies[-1] += data # noqa: B023 - bodies_done[-1] = done # noqa: B023 - - parser = pebble._MultipartParser( - marker, - handle_header, - handle_body, - max_boundary_length=test.max_boundary, - max_lookahead=test.max_lookahead) - src = io.BytesIO(test.data) - - try: - while True: - data = src.read(chunk_size) - if not data: + for i, test in enumerate(tests): + for chunk_size in chunk_sizes: + headers: typing.List[bytes] = [] + bodies: typing.List[bytes] = [] + bodies_done: typing.List[bool] = [] + + # All of the "noqa: B023" here are due to a ruff bug: + # https://github.com/astral-sh/ruff/issues/7847 + # ruff should tell us when the 'noqa's are no longer required. + def handle_header(data: typing.Any): + headers.append(bytes(data)) # noqa: B023 + bodies.append(b'') # noqa: B023 + bodies_done.append(False) # noqa: B023 + + def handle_body(data: bytes, done: bool = False): + bodies[-1] += data # noqa: B023 + bodies_done[-1] = done # noqa: B023 + + parser = pebble._MultipartParser( + marker, + handle_header, + handle_body, + max_boundary_length=test.max_boundary, + max_lookahead=test.max_lookahead) + src = io.BytesIO(test.data) + + try: + while True: + data = src.read(chunk_size) + if not data: + break + parser.feed(data) + except Exception as err: + if not test.error: + self.fail(f'unexpected error: {err}') break - parser.feed(data) - except Exception as err: - if not test.error: - pytest.fail(f'unexpected error: {err}') - break - assert test.error == str(err) - else: - if test.error: - pytest.fail(f'missing expected error: {test.error!r}') - - msg = f'test case ({test.name}), chunk size {chunk_size}' - assert test.want_headers == headers, msg - assert test.want_bodies == bodies, msg - assert test.want_bodies_done == bodies_done, msg + assert test.error == str(err) + else: + if test.error: + self.fail(f'missing expected error: {test.error!r}') + + msg = f'test case {i + 1} ({test.name}), chunk size {chunk_size}' + assert test.want_headers == headers, msg + assert test.want_bodies == bodies, msg + assert test.want_bodies_done == bodies_done, msg class TestClient(unittest.TestCase): diff --git a/test/test_private.py b/test/test_private.py index 0e31cec5f..54ec5fefe 100644 --- a/test/test_private.py +++ b/test/test_private.py @@ -50,7 +50,7 @@ def test_safe_dump(self): yaml.safe_dump(YAMLTest()) -class TestStrconv: +class TestStrconv(unittest.TestCase): def test_parse_rfc3339(self): nzdt = datetime.timezone(datetime.timedelta(hours=13)) utc = datetime.timezone.utc @@ -102,80 +102,84 @@ def test_parse_rfc3339(self): with pytest.raises(ValueError): timeconv.parse_rfc3339('2021-02-10T04:36:22.118970777-99:99') - @pytest.mark.parametrize("input,expected", [ + def test_parse_duration(self): # Test cases taken from Go's time.ParseDuration tests - # simple - ('0', datetime.timedelta(seconds=0)), - ('5s', datetime.timedelta(seconds=5)), - ('30s', datetime.timedelta(seconds=30)), - ('1478s', datetime.timedelta(seconds=1478)), - # sign - ('-5s', datetime.timedelta(seconds=-5)), - ('+5s', datetime.timedelta(seconds=5)), - ('-0', datetime.timedelta(seconds=0)), - ('+0', datetime.timedelta(seconds=0)), - # decimal - ('5.0s', datetime.timedelta(seconds=5)), - ('5.6s', datetime.timedelta(seconds=5.6)), - ('5.s', datetime.timedelta(seconds=5)), - ('.5s', datetime.timedelta(seconds=0.5)), - ('1.0s', datetime.timedelta(seconds=1)), - ('1.00s', datetime.timedelta(seconds=1)), - ('1.004s', datetime.timedelta(seconds=1.004)), - ('1.0040s', datetime.timedelta(seconds=1.004)), - ('100.00100s', datetime.timedelta(seconds=100.001)), - # different units - ('10ns', datetime.timedelta(seconds=0.000_000_010)), - ('11us', datetime.timedelta(seconds=0.000_011)), - ('12µs', datetime.timedelta(seconds=0.000_012)), # U+00B5 # noqa: RUF001 - ('12μs', datetime.timedelta(seconds=0.000_012)), # U+03BC - ('13ms', datetime.timedelta(seconds=0.013)), - ('14s', datetime.timedelta(seconds=14)), - ('15m', datetime.timedelta(seconds=15 * 60)), - ('16h', datetime.timedelta(seconds=16 * 60 * 60)), - # composite durations - ('3h30m', datetime.timedelta(seconds=3 * 60 * 60 + 30 * 60)), - ('10.5s4m', datetime.timedelta(seconds=4 * 60 + 10.5)), - ('-2m3.4s', datetime.timedelta(seconds=-(2 * 60 + 3.4))), - ('1h2m3s4ms5us6ns', datetime.timedelta(seconds=1 * 60 * 60 + 2 * 60 + 3.004_005_006)), - ('39h9m14.425s', datetime.timedelta(seconds=39 * 60 * 60 + 9 * 60 + 14.425)), - # large value - ('52763797000ns', datetime.timedelta(seconds=52.763_797_000)), - # more than 9 digits after decimal point, see https://golang.org/issue/6617 - ('0.3333333333333333333h', datetime.timedelta(seconds=20 * 60)), - # huge string; issue 15011. - ('0.100000000000000000000h', datetime.timedelta(seconds=6 * 60)), - # This value tests the first overflow check in leadingFraction. - ('0.830103483285477580700h', datetime.timedelta(seconds=49 * 60 + 48.372_539_827)), - # Test precision handling - ('7200000h1us', datetime.timedelta(hours=7_200_000, microseconds=1)) - ]) - def test_parse_duration(self, input: str, expected: datetime.timedelta): - output = timeconv.parse_duration(input) - assert output == expected, \ - f'parse_duration({input!r}): expected {expected!r}, got {output!r}' - - @pytest.mark.parametrize("input", [ - # Test cases taken from Go's time.ParseDuration tests - '', - '3', - '-', - 's', - '.', - '-.', - '.s', - '+.s', - '1d', - '\x85\x85', - '\xffff', - 'hello \xffff world', - - # Additional cases - 'X3h', - '3hY', - 'X3hY', - '3.4.5s', - ]) - def test_parse_duration_errors(self, input: str): - with pytest.raises(ValueError): - timeconv.parse_duration(input) + cases = [ + # simple + ('0', datetime.timedelta(seconds=0)), + ('5s', datetime.timedelta(seconds=5)), + ('30s', datetime.timedelta(seconds=30)), + ('1478s', datetime.timedelta(seconds=1478)), + # sign + ('-5s', datetime.timedelta(seconds=-5)), + ('+5s', datetime.timedelta(seconds=5)), + ('-0', datetime.timedelta(seconds=0)), + ('+0', datetime.timedelta(seconds=0)), + # decimal + ('5.0s', datetime.timedelta(seconds=5)), + ('5.6s', datetime.timedelta(seconds=5.6)), + ('5.s', datetime.timedelta(seconds=5)), + ('.5s', datetime.timedelta(seconds=0.5)), + ('1.0s', datetime.timedelta(seconds=1)), + ('1.00s', datetime.timedelta(seconds=1)), + ('1.004s', datetime.timedelta(seconds=1.004)), + ('1.0040s', datetime.timedelta(seconds=1.004)), + ('100.00100s', datetime.timedelta(seconds=100.001)), + # different units + ('10ns', datetime.timedelta(seconds=0.000_000_010)), + ('11us', datetime.timedelta(seconds=0.000_011)), + ('12µs', datetime.timedelta(seconds=0.000_012)), # U+00B5 # noqa: RUF001 + ('12μs', datetime.timedelta(seconds=0.000_012)), # U+03BC + ('13ms', datetime.timedelta(seconds=0.013)), + ('14s', datetime.timedelta(seconds=14)), + ('15m', datetime.timedelta(seconds=15 * 60)), + ('16h', datetime.timedelta(seconds=16 * 60 * 60)), + # composite durations + ('3h30m', datetime.timedelta(seconds=3 * 60 * 60 + 30 * 60)), + ('10.5s4m', datetime.timedelta(seconds=4 * 60 + 10.5)), + ('-2m3.4s', datetime.timedelta(seconds=-(2 * 60 + 3.4))), + ('1h2m3s4ms5us6ns', datetime.timedelta(seconds=1 * 60 * 60 + 2 * 60 + 3.004_005_006)), + ('39h9m14.425s', datetime.timedelta(seconds=39 * 60 * 60 + 9 * 60 + 14.425)), + # large value + ('52763797000ns', datetime.timedelta(seconds=52.763_797_000)), + # more than 9 digits after decimal point, see https://golang.org/issue/6617 + ('0.3333333333333333333h', datetime.timedelta(seconds=20 * 60)), + # huge string; issue 15011. + ('0.100000000000000000000h', datetime.timedelta(seconds=6 * 60)), + # This value tests the first overflow check in leadingFraction. + ('0.830103483285477580700h', datetime.timedelta(seconds=49 * 60 + 48.372_539_827)), + + # Test precision handling + ('7200000h1us', datetime.timedelta(hours=7_200_000, microseconds=1)) + ] + + for input, expected in cases: + output = timeconv.parse_duration(input) + assert output == expected, \ + f'parse_duration({input!r}): expected {expected!r}, got {output!r}' + + def test_parse_duration_errors(self): + cases = [ + # Test cases taken from Go's time.ParseDuration tests + '', + '3', + '-', + 's', + '.', + '-.', + '.s', + '+.s', + '1d', + '\x85\x85', + '\xffff', + 'hello \xffff world', + + # Additional cases + 'X3h', + '3hY', + 'X3hY', + '3.4.5s', + ] + for input in cases: + with pytest.raises(ValueError): + timeconv.parse_duration(input) From 7472ae74dfe53624201f823b958426e10ec3e9e1 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 19 Apr 2024 20:03:18 +0800 Subject: [PATCH 10/16] chore: refactor according to self review --- test/test_charm.py | 38 ++++++++++++-------------------------- test/test_helpers.py | 8 ++++---- 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index 46471e7ec..bb6e83848 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -165,6 +165,7 @@ def on_any_relation(self, event: ops.RelationEvent): assert event.relation.app.name == 'remote' self.seen.append(type(event).__name__) + # language=YAML meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: @@ -237,6 +238,7 @@ def _on_stor3_attach(self, event: ops.StorageAttachedEvent): def _on_stor4_attach(self, event: ops.StorageAttachedEvent): self.seen.append(type(event).__name__) + # language=YAML meta = ops.CharmMeta.from_yaml(''' name: my-charm storage: @@ -264,12 +266,6 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): type: filesystem ''') - assert meta.storages['stor1'].multiple_range is None # type: ignore - assert meta.storages['stor2'].multiple_range == (2, 2) # type: ignore - assert meta.storages['stor3'].multiple_range == (2, None) # type: ignore - assert meta.storages['stor-4'].multiple_range == (2, 4) # type: ignore - assert meta.storages['stor-plus'].multiple_range == (10, None) # type: ignore - fake_script_fixture.write( "storage-get", """ @@ -309,6 +305,12 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): """, ) + assert meta.storages['stor1'].multiple_range is None + assert meta.storages['stor2'].multiple_range == (2, 2) + assert meta.storages['stor3'].multiple_range == (2, None) + assert meta.storages['stor-4'].multiple_range == (2, 4) + assert meta.storages['stor-plus'].multiple_range == (10, None) + framework = create_framework(monkeypatch, request, meta=meta) charm = MyCharm(framework) @@ -350,6 +352,7 @@ def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self.seen.append(type(event).__name__) + # language=YAML meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm containers: @@ -379,8 +382,8 @@ def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): 'PebbleCustomNoticeEvent', ] - @pytest.mark.charm_meta() def test_relations_meta(self): + # language=YAML meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: @@ -405,6 +408,7 @@ def test_relations_meta(self): def test_relations_meta_limit_type_validation(self): with pytest.raises(TypeError, match=r"limit should be an int, not "): + # language=YAML ops.CharmMeta.from_yaml(''' name: my-charm requires: @@ -418,6 +422,7 @@ def test_relations_meta_scope_type_validation(self): TypeError, match="scope should be one of 'global', 'container'; not 'foobar'" ): + # language=YAML ops.CharmMeta.from_yaml(''' name: my-charm requires: @@ -565,25 +570,6 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): with pytest.raises(ValueError): charm.on.foo_bar_action.emit(id='1') - @pytest.mark.charm_meta(ops.CharmMeta.from_yaml(metadata=''' -name: my-charm -''', actions=''' -foo-bar: - description: "Foos the bar." - params: - foo-name: - description: "A foo name to bar" - type: string - silent: - default: false - description: "" - type: boolean - required: foo-bar - title: foo-bar -start: - description: "Start the unit." - additionalProperties: false -''')) def test_action_event_defer_fails( self, monkeypatch: pytest.MonkeyPatch, diff --git a/test/test_helpers.py b/test/test_helpers.py index 31b86961a..ad6f668f6 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -78,10 +78,10 @@ def fake_script_calls(test_case: unittest.TestCase, def create_framework( - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - *, - meta: typing.Union[ops.CharmMeta, None] = None): + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + *, + meta: typing.Union[ops.CharmMeta, None] = None): monkeypatch.setenv("PATH", str(pathlib.Path(__file__).parent / 'bin'), prepend=os.pathsep) monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") From 93b6b799b5b28475b2a59bcb596b697be8723418 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Mon, 22 Apr 2024 12:13:18 +0800 Subject: [PATCH 11/16] test: refactor test charm arccording to code review --- test/test_charm.py | 1562 +++++++++++++++++++++--------------------- test/test_helpers.py | 83 +-- 2 files changed, 821 insertions(+), 824 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index bb6e83848..7364e4143 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -15,7 +15,6 @@ import pathlib import tempfile import typing -import unittest from pathlib import Path import pytest @@ -29,144 +28,139 @@ @pytest.fixture -def fake_script_fixture(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path): - return FakeScript(monkeypatch, tmp_path) - - -class TestCharm: - def test_basic(self, monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest): - class MyCharm(ops.CharmBase): - - def __init__(self, *args: typing.Any): - super().__init__(*args) - - self.started = False - framework.observe(self.on.start, self._on_start) - - def _on_start(self, event: ops.EventBase): - self.started = True - - framework = create_framework(monkeypatch, request) - - events: typing.List[str] = list(MyCharm.on.events()) # type: ignore - assert 'install' in events - assert 'custom' in events - - charm = MyCharm(framework) - charm.on.start.emit() - - assert charm.started - - with pytest.raises(TypeError): - framework.observe(charm.on.start, charm) # type: ignore - - def test_observe_decorated_method( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest): - # we test that charm methods decorated with @functools.wraps(wrapper) - # can be observed by Framework. Simpler decorators won't work because - # Framework searches for __self__ and other method things; functools.wraps - # is more careful and it still works, this test is here to ensure that - # it keeps working in future releases, as this is presently the only - # way we know of to cleanly decorate charm event observers. - events: typing.List[ops.EventBase] = [] - - def dec(fn: typing.Any) -> typing.Callable[..., None]: - # simple decorator that appends to the nonlocal - # `events` list all events it receives - @functools.wraps(fn) - def wrapper(charm: 'MyCharm', evt: ops.EventBase): - events.append(evt) - fn(charm, evt) - return wrapper - - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - framework.observe(self.on.start, self._on_start) - self.seen = None - - @dec - def _on_start(self, event: ops.EventBase): - self.seen = event - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - charm.on.start.emit() - # check that the event has been seen by the decorator - assert len(events) == 1 - # check that the event has been seen by the observer - assert isinstance(charm.seen, ops.StartEvent) - - def test_observer_not_referenced_warning( - self, - caplog: pytest.LogCaptureFixture, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest): - class MyObj(ops.Object): - def __init__(self, charm: ops.CharmBase): - super().__init__(charm, "obj") - framework.observe(charm.on.start, self._on_start) - - def _on_start(self, _: ops.StartEvent): - raise RuntimeError() # never reached! - - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - MyObj(self) # not assigned! - framework.observe(self.on.start, self._on_start) - - def _on_start(self, _: ops.StartEvent): - pass # is reached - - framework = create_framework(monkeypatch, request) - c = MyCharm(framework) - c.on.start.emit() - assert 'Reference to ops.Object' in caplog.text - - def test_empty_action(self): - meta = ops.CharmMeta.from_yaml('name: my-charm', '') - assert meta.actions == {} - - def test_helper_properties(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest): - class MyCharm(ops.CharmBase): - pass +def fake_script(request: pytest.FixtureRequest) -> FakeScript: + return FakeScript(request) - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - assert charm.app == framework.model.app - assert charm.unit == framework.model.unit - assert charm.meta == framework.meta - assert charm.charm_dir == framework.charm_dir - assert charm.config is framework.model.config - - def test_relation_events(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest): - - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - for rel in ('req1', 'req-2', 'pro1', 'pro-2', 'peer1', 'peer-2'): - # Hook up relation events to generic handler. - self.framework.observe(self.on[rel].relation_joined, self.on_any_relation) - self.framework.observe(self.on[rel].relation_changed, self.on_any_relation) - self.framework.observe(self.on[rel].relation_departed, self.on_any_relation) - self.framework.observe(self.on[rel].relation_broken, self.on_any_relation) - - def on_any_relation(self, event: ops.RelationEvent): - assert event.relation.name == 'req1' - assert event.relation.app is not None - assert event.relation.app.name == 'remote' - self.seen.append(type(event).__name__) - # language=YAML - meta = ops.CharmMeta.from_yaml(metadata=''' +def test_basic(request: pytest.FixtureRequest): + class MyCharm(ops.CharmBase): + + def __init__(self, *args: typing.Any): + super().__init__(*args) + + self.started = False + framework.observe(self.on.start, self._on_start) + + def _on_start(self, event: ops.EventBase): + self.started = True + + framework = create_framework(request) + + events: typing.List[str] = list(MyCharm.on.events()) # type: ignore + assert 'install' in events + assert 'custom' in events + + charm = MyCharm(framework) + charm.on.start.emit() + + assert charm.started + + with pytest.raises(TypeError): + framework.observe(charm.on.start, charm) # type: ignore + + +def test_observe_decorated_method(request: pytest.FixtureRequest): + # we test that charm methods decorated with @functools.wraps(wrapper) + # can be observed by Framework. Simpler decorators won't work because + # Framework searches for __self__ and other method things; functools.wraps + # is more careful and it still works, this test is here to ensure that + # it keeps working in future releases, as this is presently the only + # way we know of to cleanly decorate charm event observers. + events: typing.List[ops.EventBase] = [] + + def dec(fn: typing.Any) -> typing.Callable[..., None]: + # simple decorator that appends to the nonlocal + # `events` list all events it receives + @functools.wraps(fn) + def wrapper(charm: 'MyCharm', evt: ops.EventBase): + events.append(evt) + fn(charm, evt) + return wrapper + + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + framework.observe(self.on.start, self._on_start) + self.seen = None + + @dec + def _on_start(self, event: ops.EventBase): + self.seen = event + + framework = create_framework(request) + charm = MyCharm(framework) + charm.on.start.emit() + # check that the event has been seen by the decorator + assert len(events) == 1 + # check that the event has been seen by the observer + assert isinstance(charm.seen, ops.StartEvent) + + +def test_observer_not_referenced_warning( + caplog: pytest.LogCaptureFixture, + request: pytest.FixtureRequest): + class MyObj(ops.Object): + def __init__(self, charm: ops.CharmBase): + super().__init__(charm, "obj") + framework.observe(charm.on.start, self._on_start) + + def _on_start(self, _: ops.StartEvent): + raise RuntimeError() # never reached! + + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + MyObj(self) # not assigned! + framework.observe(self.on.start, self._on_start) + + def _on_start(self, _: ops.StartEvent): + pass # is reached + + framework = create_framework(request) + c = MyCharm(framework) + c.on.start.emit() + assert 'Reference to ops.Object' in caplog.text + + +def test_empty_action(): + meta = ops.CharmMeta.from_yaml('name: my-charm', '') + assert meta.actions == {} + + +def test_helper_properties(request: pytest.FixtureRequest): + class MyCharm(ops.CharmBase): + pass + + framework = create_framework(request) + charm = MyCharm(framework) + assert charm.app == framework.model.app + assert charm.unit == framework.model.unit + assert charm.meta == framework.meta + assert charm.charm_dir == framework.charm_dir + assert charm.config is framework.model.config + + +def test_relation_events(request: pytest.FixtureRequest): + + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + for rel in ('req1', 'req-2', 'pro1', 'pro-2', 'peer1', 'peer-2'): + # Hook up relation events to generic handler. + self.framework.observe(self.on[rel].relation_joined, self.on_any_relation) + self.framework.observe(self.on[rel].relation_changed, self.on_any_relation) + self.framework.observe(self.on[rel].relation_departed, self.on_any_relation) + self.framework.observe(self.on[rel].relation_broken, self.on_any_relation) + + def on_any_relation(self, event: ops.RelationEvent): + assert event.relation.name == 'req1' + assert event.relation.app is not None + assert event.relation.app.name == 'remote' + self.seen.append(type(event).__name__) + + # language=YAML + meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: req1: @@ -184,62 +178,61 @@ def on_any_relation(self, event: ops.RelationEvent): peer-2: interface: peer2 ''') - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - assert 'pro_2_relation_broken' in repr(charm.on) - - rel = charm.framework.model.get_relation('req1', 1) - app = charm.framework.model.get_app('remote') - unit = charm.framework.model.get_unit('remote/0') - charm.on['req1'].relation_joined.emit(rel, app, unit) - charm.on['req1'].relation_changed.emit(rel, app, unit) - charm.on['req1'].relation_changed.emit(rel, app) - charm.on['req-2'].relation_changed.emit(rel, app, unit) - charm.on['pro1'].relation_departed.emit(rel, app, unit) - charm.on['pro-2'].relation_departed.emit(rel, app, unit) - charm.on['peer1'].relation_broken.emit(rel, app) - charm.on['peer-2'].relation_broken.emit(rel, app) - - assert charm.seen == [ - 'RelationJoinedEvent', - 'RelationChangedEvent', - 'RelationChangedEvent', - 'RelationChangedEvent', - 'RelationDepartedEvent', - 'RelationDepartedEvent', - 'RelationBrokenEvent', - 'RelationBrokenEvent', - ] - - def test_storage_events(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) - self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) - self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) - self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) - - def _on_stor1_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - assert event.storage.location == Path("/var/srv/stor1/0") - - def _on_stor2_detach(self, event: ops.StorageDetachingEvent): - self.seen.append(type(event).__name__) - - def _on_stor3_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - - def _on_stor4_attach(self, event: ops.StorageAttachedEvent): - self.seen.append(type(event).__name__) - - # language=YAML - meta = ops.CharmMeta.from_yaml(''' + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + assert 'pro_2_relation_broken' in repr(charm.on) + + rel = charm.framework.model.get_relation('req1', 1) + app = charm.framework.model.get_app('remote') + unit = charm.framework.model.get_unit('remote/0') + charm.on['req1'].relation_joined.emit(rel, app, unit) + charm.on['req1'].relation_changed.emit(rel, app, unit) + charm.on['req1'].relation_changed.emit(rel, app) + charm.on['req-2'].relation_changed.emit(rel, app, unit) + charm.on['pro1'].relation_departed.emit(rel, app, unit) + charm.on['pro-2'].relation_departed.emit(rel, app, unit) + charm.on['peer1'].relation_broken.emit(rel, app) + charm.on['peer-2'].relation_broken.emit(rel, app) + + assert charm.seen == [ + 'RelationJoinedEvent', + 'RelationChangedEvent', + 'RelationChangedEvent', + 'RelationChangedEvent', + 'RelationDepartedEvent', + 'RelationDepartedEvent', + 'RelationBrokenEvent', + 'RelationBrokenEvent', + ] + + +def test_storage_events(request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) + self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) + self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) + self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) + + def _on_stor1_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + assert event.storage.location == Path("/var/srv/stor1/0") + + def _on_stor2_detach(self, event: ops.StorageDetachingEvent): + self.seen.append(type(event).__name__) + + def _on_stor3_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + def _on_stor4_attach(self, event: ops.StorageAttachedEvent): + self.seen.append(type(event).__name__) + + # language=YAML + meta = ops.CharmMeta.from_yaml(''' name: my-charm storage: stor-4: @@ -266,125 +259,125 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): type: filesystem ''') - fake_script_fixture.write( - "storage-get", - """ - if [ "$1" = "-s" ]; then - id=${2#*/} - key=${2%/*} - echo "\\"/var/srv/${key}/${id}\\"" # NOQA: test_quote_backslashes - elif [ "$1" = '--help' ]; then - printf '%s\\n' \\ - 'Usage: storage-get [options] []' \\ - ' ' \\ - 'Summary:' \\ - 'print information for storage instance with specified id' \\ - ' ' \\ - 'Options:' \\ - '--format (= smart)' \\ - ' Specify output format (json|smart|yaml)' \\ - '-o, --output (= "")' \\ - ' Specify an output file' \\ - '-s (= test-stor/0)' \\ - ' specify a storage instance by id' \\ - ' ' \\ - 'Details:' \\ - 'When no is supplied, all keys values are printed.' - else - # Return the same path for all disks since `storage-get` - # on attach and detach takes no parameters and is not - # deterministically faked with fake_script - exit 1 - fi - """, - ) - fake_script_fixture.write( - "storage-list", - """ - echo '["disks/0"]' - """, - ) + fake_script.write( + "storage-get", + """ + if [ "$1" = "-s" ]; then + id=${2#*/} + key=${2%/*} + echo "\\"/var/srv/${key}/${id}\\"" # NOQA: test_quote_backslashes + elif [ "$1" = '--help' ]; then + printf '%s\\n' \\ + 'Usage: storage-get [options] []' \\ + ' ' \\ + 'Summary:' \\ + 'print information for storage instance with specified id' \\ + ' ' \\ + 'Options:' \\ + '--format (= smart)' \\ + ' Specify output format (json|smart|yaml)' \\ + '-o, --output (= "")' \\ + ' Specify an output file' \\ + '-s (= test-stor/0)' \\ + ' specify a storage instance by id' \\ + ' ' \\ + 'Details:' \\ + 'When no is supplied, all keys values are printed.' + else + # Return the same path for all disks since `storage-get` + # on attach and detach takes no parameters and is not + # deterministically faked with fake_script + exit 1 + fi + """, + ) + fake_script.write( + "storage-list", + """ + echo '["disks/0"]' + """, + ) + + assert meta.storages['stor1'].multiple_range is None + assert meta.storages['stor2'].multiple_range == (2, 2) + assert meta.storages['stor3'].multiple_range == (2, None) + assert meta.storages['stor-4'].multiple_range == (2, 4) + assert meta.storages['stor-plus'].multiple_range == (10, None) + + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + charm.on['stor1'].storage_attached.emit(ops.Storage("stor1", 0, charm.model._backend)) + charm.on['stor2'].storage_detaching.emit(ops.Storage("stor2", 0, charm.model._backend)) + charm.on['stor3'].storage_attached.emit(ops.Storage("stor3", 0, charm.model._backend)) + charm.on['stor-4'].storage_attached.emit(ops.Storage("stor-4", 0, charm.model._backend)) + charm.on['stor-multiple-dashes'].storage_attached.emit( + ops.Storage("stor-multiple-dashes", 0, charm.model._backend)) + + assert charm.seen == [ + 'StorageAttachedEvent', + 'StorageDetachingEvent', + 'StorageAttachedEvent', + 'StorageAttachedEvent', + ] + + +def test_workload_events(request: pytest.FixtureRequest): + + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + for workload in ('container-a', 'containerb'): + # Hook up relation events to generic handler. + self.framework.observe( + self.on[workload].pebble_ready, + self.on_any_pebble_ready) + self.framework.observe( + self.on[workload].pebble_custom_notice, + self.on_any_pebble_custom_notice, + ) - assert meta.storages['stor1'].multiple_range is None - assert meta.storages['stor2'].multiple_range == (2, 2) - assert meta.storages['stor3'].multiple_range == (2, None) - assert meta.storages['stor-4'].multiple_range == (2, 4) - assert meta.storages['stor-plus'].multiple_range == (10, None) - - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - charm.on['stor1'].storage_attached.emit(ops.Storage("stor1", 0, charm.model._backend)) - charm.on['stor2'].storage_detaching.emit(ops.Storage("stor2", 0, charm.model._backend)) - charm.on['stor3'].storage_attached.emit(ops.Storage("stor3", 0, charm.model._backend)) - charm.on['stor-4'].storage_attached.emit(ops.Storage("stor-4", 0, charm.model._backend)) - charm.on['stor-multiple-dashes'].storage_attached.emit( - ops.Storage("stor-multiple-dashes", 0, charm.model._backend)) - - assert charm.seen == [ - 'StorageAttachedEvent', - 'StorageDetachingEvent', - 'StorageAttachedEvent', - 'StorageAttachedEvent', - ] - - def test_workload_events(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest): - - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - for workload in ('container-a', 'containerb'): - # Hook up relation events to generic handler. - self.framework.observe( - self.on[workload].pebble_ready, - self.on_any_pebble_ready) - self.framework.observe( - self.on[workload].pebble_custom_notice, - self.on_any_pebble_custom_notice, - ) - - def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): - self.seen.append(type(event).__name__) - - def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): - self.seen.append(type(event).__name__) + def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): + self.seen.append(type(event).__name__) - # language=YAML - meta = ops.CharmMeta.from_yaml(metadata=''' + def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): + self.seen.append(type(event).__name__) + + # language=YAML + meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm containers: container-a: containerb: ''') - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - assert 'container_a_pebble_ready' in repr(charm.on) - assert 'containerb_pebble_ready' in repr(charm.on) - - charm.on['container-a'].pebble_ready.emit( - charm.framework.model.unit.get_container('container-a')) - charm.on['containerb'].pebble_ready.emit( - charm.framework.model.unit.get_container('containerb')) - - charm.on['container-a'].pebble_custom_notice.emit( - charm.framework.model.unit.get_container('container-a'), '1', 'custom', 'x') - charm.on['containerb'].pebble_custom_notice.emit( - charm.framework.model.unit.get_container('containerb'), '2', 'custom', 'y') - - assert charm.seen == [ - 'PebbleReadyEvent', - 'PebbleReadyEvent', - 'PebbleCustomNoticeEvent', - 'PebbleCustomNoticeEvent', - ] - - def test_relations_meta(self): - # language=YAML - meta = ops.CharmMeta.from_yaml(metadata=''' + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + assert 'container_a_pebble_ready' in repr(charm.on) + assert 'containerb_pebble_ready' in repr(charm.on) + + charm.on['container-a'].pebble_ready.emit( + charm.framework.model.unit.get_container('container-a')) + charm.on['containerb'].pebble_ready.emit( + charm.framework.model.unit.get_container('containerb')) + + charm.on['container-a'].pebble_custom_notice.emit( + charm.framework.model.unit.get_container('container-a'), '1', 'custom', 'x') + charm.on['containerb'].pebble_custom_notice.emit( + charm.framework.model.unit.get_container('containerb'), '2', 'custom', 'y') + + assert charm.seen == [ + 'PebbleReadyEvent', + 'PebbleReadyEvent', + 'PebbleCustomNoticeEvent', + 'PebbleCustomNoticeEvent', + ] + + +def test_relations_meta(): + # language=YAML + meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm requires: database: @@ -396,20 +389,21 @@ def test_relations_meta(self): optional: true ''') - assert meta.requires['database'].interface_name == 'mongodb' - assert meta.requires['database'].limit == 1 - assert meta.requires['database'].scope == 'container' - assert not meta.requires['database'].optional + assert meta.requires['database'].interface_name == 'mongodb' + assert meta.requires['database'].limit == 1 + assert meta.requires['database'].scope == 'container' + assert not meta.requires['database'].optional + + assert meta.requires['metrics'].interface_name == 'prometheus-scraping' + assert meta.requires['metrics'].limit is None + assert meta.requires['metrics'].scope == 'global' # Default value + assert meta.requires['metrics'].optional - assert meta.requires['metrics'].interface_name == 'prometheus-scraping' - assert meta.requires['metrics'].limit is None - assert meta.requires['metrics'].scope == 'global' # Default value - assert meta.requires['metrics'].optional - def test_relations_meta_limit_type_validation(self): - with pytest.raises(TypeError, match=r"limit should be an int, not "): - # language=YAML - ops.CharmMeta.from_yaml(''' +def test_relations_meta_limit_type_validation(): + with pytest.raises(TypeError, match=r"limit should be an int, not "): + # language=YAML + ops.CharmMeta.from_yaml(''' name: my-charm requires: database: @@ -417,13 +411,14 @@ def test_relations_meta_limit_type_validation(self): limit: foobar ''') - def test_relations_meta_scope_type_validation(self): - with pytest.raises( - TypeError, - match="scope should be one of 'global', 'container'; not 'foobar'" - ): - # language=YAML - ops.CharmMeta.from_yaml(''' + +def test_relations_meta_scope_type_validation(): + with pytest.raises( + TypeError, + match="scope should be one of 'global', 'container'; not 'foobar'" + ): + # language=YAML + ops.CharmMeta.from_yaml(''' name: my-charm requires: database: @@ -431,54 +426,57 @@ def test_relations_meta_scope_type_validation(self): scope: foobar ''') - def test_meta_from_charm_root(self): - with tempfile.TemporaryDirectory() as d: - td = pathlib.Path(d) - (td / 'metadata.yaml').write_text( - yaml.safe_dump( - {"name": "bob", - "requires": { - "foo": - {"interface": "bar"} - }})) - meta = ops.CharmMeta.from_charm_root(td) - assert meta.name == "bob" - assert meta.requires['foo'].interface_name == "bar" - - def test_actions_from_charm_root(self): - with tempfile.TemporaryDirectory() as d: - td = pathlib.Path(d) - (td / 'actions.yaml').write_text( - yaml.safe_dump( - {"foo": { - "description": "foos the bar", - "additionalProperties": False - }} - ) + +def test_meta_from_charm_root(): + with tempfile.TemporaryDirectory() as d: + td = pathlib.Path(d) + (td / 'metadata.yaml').write_text( + yaml.safe_dump( + {"name": "bob", + "requires": { + "foo": + {"interface": "bar"} + }})) + meta = ops.CharmMeta.from_charm_root(td) + assert meta.name == "bob" + assert meta.requires['foo'].interface_name == "bar" + + +def test_actions_from_charm_root(): + with tempfile.TemporaryDirectory() as d: + td = pathlib.Path(d) + (td / 'actions.yaml').write_text( + yaml.safe_dump( + {"foo": { + "description": "foos the bar", + "additionalProperties": False + }} ) - (td / 'metadata.yaml').write_text( - yaml.safe_dump( - {"name": "bob", - "requires": { - "foo": - {"interface": "bar"} - }})) - - meta = ops.CharmMeta.from_charm_root(td) - assert meta.name == "bob" - assert meta.requires['foo'].interface_name == "bar" - assert not meta.actions['foo'].additional_properties - assert meta.actions['foo'].description == "foos the bar" - - def _setup_test_action(self, fake_script: typing.Callable[..., None]): - fake_script('action-get', """echo '{"foo-name": "name", "silent": true}'""") - fake_script('action-set', "") - fake_script('action-log', "") - fake_script('action-fail', "") - - @classmethod - def _get_action_test_meta(cls): - return ops.CharmMeta.from_yaml(metadata=''' + ) + (td / 'metadata.yaml').write_text( + yaml.safe_dump( + {"name": "bob", + "requires": { + "foo": + {"interface": "bar"} + }})) + + meta = ops.CharmMeta.from_charm_root(td) + assert meta.name == "bob" + assert meta.requires['foo'].interface_name == "bar" + assert not meta.actions['foo'].additional_properties + assert meta.actions['foo'].description == "foos the bar" + + +def _setup_test_action(fake_script: typing.Callable[..., None]): + fake_script('action-get', """echo '{"foo-name": "name", "silent": true}'""") + fake_script('action-set', "") + fake_script('action-log', "") + fake_script('action-fail', "") + + +def _get_action_test_meta(): + return ops.CharmMeta.from_yaml(metadata=''' name: my-charm ''', actions=''' foo-bar: @@ -498,107 +496,105 @@ def _get_action_test_meta(cls): additionalProperties: false ''') - def test_action_events(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - - class MyCharm(ops.CharmBase): - - def __init__(self, *args: typing.Any): - super().__init__(*args) - framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) - framework.observe(self.on.start_action, self._on_start_action) - - def _on_foo_bar_action(self, event: ops.ActionEvent): - self.seen_action_params = event.params - event.log('test-log') - event.set_results({'res': 'val with spaces', 'id': event.id}) - event.fail('test-fail') - - def _on_start_action(self, event: ops.ActionEvent): - pass - - self._setup_test_action(fake_script_fixture.write) - meta = self._get_action_test_meta() - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - events: typing.List[str] = list(MyCharm.on.events()) # type: ignore - assert 'foo_bar_action' in events - assert 'start_action' in events - - action_id = "1234" - charm.on.foo_bar_action.emit(id=action_id) - assert charm.seen_action_params == {"foo-name": "name", "silent": True} - assert fake_script_fixture.calls() == [ - ['action-get', '--format=json'], - ['action-log', "test-log"], - ['action-set', "res=val with spaces", f"id={action_id}"], - ['action-fail', "test-fail"], - ] - - @pytest.mark.parametrize("bad_res", [ - {'a': {'b': 'c'}, 'a.b': 'c'}, - {'a': {'B': 'c'}}, - {'a': {(1, 2): 'c'}}, - {'a': {None: 'c'}}, - {'aBc': 'd'} - ]) - def test_invalid_action_results(self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript, - bad_res: typing.Dict[str, typing.Any]): - - class MyCharm(ops.CharmBase): - - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.res: typing.Dict[str, typing.Any] = {} - framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) - - def _on_foo_bar_action(self, event: ops.ActionEvent): - event.set_results(self.res) - - self._setup_test_action(fake_script_fixture.write) - meta = self._get_action_test_meta() - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - charm.res = bad_res - with pytest.raises(ValueError): - charm.on.foo_bar_action.emit(id='1') - - def test_action_event_defer_fails( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - - cmd_type = 'action' - - class MyCharm(ops.CharmBase): - - def __init__(self, *args: typing.Any): - super().__init__(*args) - framework.observe(self.on.start_action, self._on_start_action) - - def _on_start_action(self, event: ops.ActionEvent): - event.defer() - - fake_script_fixture.write(f"{cmd_type}-get", - """echo '{"foo-name": "name", "silent": true}'""") - monkeypatch.setenv(f'JUJU_{cmd_type.upper()}_NAME', 'start') - meta = self._get_action_test_meta() - framework = create_framework(monkeypatch, request, meta=meta) - charm = MyCharm(framework) - - with pytest.raises(RuntimeError): - charm.on.start_action.emit(id='2') - - def test_containers(self): - meta = ops.CharmMeta.from_yaml(""" + +def test_action_events(request: pytest.FixtureRequest, fake_script: FakeScript): + + class MyCharm(ops.CharmBase): + + def __init__(self, *args: typing.Any): + super().__init__(*args) + framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) + framework.observe(self.on.start_action, self._on_start_action) + + def _on_foo_bar_action(self, event: ops.ActionEvent): + self.seen_action_params = event.params + event.log('test-log') + event.set_results({'res': 'val with spaces', 'id': event.id}) + event.fail('test-fail') + + def _on_start_action(self, event: ops.ActionEvent): + pass + + _setup_test_action(fake_script.write) + meta = _get_action_test_meta() + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + events: typing.List[str] = list(MyCharm.on.events()) # type: ignore + assert 'foo_bar_action' in events + assert 'start_action' in events + + action_id = "1234" + charm.on.foo_bar_action.emit(id=action_id) + assert charm.seen_action_params == {"foo-name": "name", "silent": True} + assert fake_script.calls() == [ + ['action-get', '--format=json'], + ['action-log', "test-log"], + ['action-set', "res=val with spaces", f"id={action_id}"], + ['action-fail', "test-fail"], + ] + + +@pytest.mark.parametrize("bad_res", [ + {'a': {'b': 'c'}, 'a.b': 'c'}, + {'a': {'B': 'c'}}, + {'a': {(1, 2): 'c'}}, + {'a': {None: 'c'}}, + {'aBc': 'd'} +]) +def test_invalid_action_results(request: pytest.FixtureRequest, + fake_script: FakeScript, + bad_res: typing.Dict[str, typing.Any]): + + class MyCharm(ops.CharmBase): + + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.res: typing.Dict[str, typing.Any] = {} + framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) + + def _on_foo_bar_action(self, event: ops.ActionEvent): + event.set_results(self.res) + + _setup_test_action(fake_script.write) + meta = _get_action_test_meta() + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + charm.res = bad_res + with pytest.raises(ValueError): + charm.on.foo_bar_action.emit(id='1') + + +def test_action_event_defer_fails( + monkeypatch: pytest.MonkeyPatch, + request: pytest.FixtureRequest, + fake_script: FakeScript): + + cmd_type = 'action' + + class MyCharm(ops.CharmBase): + + def __init__(self, *args: typing.Any): + super().__init__(*args) + framework.observe(self.on.start_action, self._on_start_action) + + def _on_start_action(self, event: ops.ActionEvent): + event.defer() + + fake_script.write(f"{cmd_type}-get", + """echo '{"foo-name": "name", "silent": true}'""") + monkeypatch.setenv(f'JUJU_{cmd_type.upper()}_NAME', 'start') + meta = _get_action_test_meta() + framework = create_framework(request, meta=meta) + charm = MyCharm(framework) + + with pytest.raises(RuntimeError): + charm.on.start_action.emit(id='2') + + +def test_containers(): + meta = ops.CharmMeta.from_yaml(""" name: k8s-charm containers: test1: @@ -606,13 +602,14 @@ def test_containers(self): test2: k: v """) - assert isinstance(meta.containers['test1'], ops.ContainerMeta) - assert isinstance(meta.containers['test2'], ops.ContainerMeta) - assert meta.containers['test1'].name == 'test1' - assert meta.containers['test2'].name == 'test2' + assert isinstance(meta.containers['test1'], ops.ContainerMeta) + assert isinstance(meta.containers['test2'], ops.ContainerMeta) + assert meta.containers['test1'].name == 'test1' + assert meta.containers['test2'].name == 'test2' + - def test_containers_storage(self): - meta = ops.CharmMeta.from_yaml(""" +def test_containers_storage(): + meta = ops.CharmMeta.from_yaml(""" name: k8s-charm storage: data: @@ -642,23 +639,23 @@ def test_containers_storage(self): architectures: - arm """) - assert isinstance(meta.containers['test1'], ops.ContainerMeta) - assert isinstance(meta.containers['test1'].mounts["data"], ops.ContainerStorageMeta) - assert meta.containers['test1'].mounts["data"].location == '/test/storagemount' - assert meta.containers['test1'].mounts["other"].location == '/test/otherdata' - assert meta.storages['other'].properties == ['transient'] - assert meta.containers['test1'].resource == 'ubuntu-22.10' - assert meta.containers['test2'].bases is not None - assert len(meta.containers['test2'].bases) == 2 - assert meta.containers['test2'].bases[0].os_name == 'ubuntu' - assert meta.containers['test2'].bases[0].channel == '23.10' - assert meta.containers['test2'].bases[0].architectures == ['amd64'] - assert meta.containers['test2'].bases[1].os_name == 'ubuntu' - assert meta.containers['test2'].bases[1].channel == '23.04/stable/fips' - assert meta.containers['test2'].bases[1].architectures == ['arm'] - # It's an error to specify both the 'resource' and the 'bases' fields. - with pytest.raises(ModelError): - ops.CharmMeta.from_yaml(""" + assert isinstance(meta.containers['test1'], ops.ContainerMeta) + assert isinstance(meta.containers['test1'].mounts["data"], ops.ContainerStorageMeta) + assert meta.containers['test1'].mounts["data"].location == '/test/storagemount' + assert meta.containers['test1'].mounts["other"].location == '/test/otherdata' + assert meta.storages['other'].properties == ['transient'] + assert meta.containers['test1'].resource == 'ubuntu-22.10' + assert meta.containers['test2'].bases is not None + assert len(meta.containers['test2'].bases) == 2 + assert meta.containers['test2'].bases[0].os_name == 'ubuntu' + assert meta.containers['test2'].bases[0].channel == '23.10' + assert meta.containers['test2'].bases[0].architectures == ['amd64'] + assert meta.containers['test2'].bases[1].os_name == 'ubuntu' + assert meta.containers['test2'].bases[1].channel == '23.04/stable/fips' + assert meta.containers['test2'].bases[1].architectures == ['arm'] + # It's an error to specify both the 'resource' and the 'bases' fields. + with pytest.raises(ModelError): + ops.CharmMeta.from_yaml(""" name: invalid-charm containers: test1: @@ -669,8 +666,9 @@ def test_containers_storage(self): resource: ubuntu-23.10 """) - def test_containers_storage_multiple_mounts(self): - meta = ops.CharmMeta.from_yaml(""" + +def test_containers_storage_multiple_mounts(): + meta = ops.CharmMeta.from_yaml(""" name: k8s-charm storage: data: @@ -684,294 +682,286 @@ def test_containers_storage_multiple_mounts(self): - storage: data location: /test/otherdata """) - assert isinstance(meta.containers['test1'], ops.ContainerMeta) - assert isinstance(meta.containers['test1'].mounts["data"], ops.ContainerStorageMeta) - assert meta.containers['test1'].mounts["data"].locations[0] == \ - '/test/storagemount' - assert meta.containers['test1'].mounts["data"].locations[1] == '/test/otherdata' - - with pytest.raises(RuntimeError): - meta.containers["test1"].mounts["data"].location - - def test_secret_events(self, monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.seen: typing.List[str] = [] - self.framework.observe(self.on.secret_changed, self.on_secret_changed) - self.framework.observe(self.on.secret_rotate, self.on_secret_rotate) - self.framework.observe(self.on.secret_remove, self.on_secret_remove) - self.framework.observe(self.on.secret_expired, self.on_secret_expired) - - def on_secret_changed(self, event: ops.SecretChangedEvent): - assert event.secret.id == 'secret:changed' - assert event.secret.label is None - self.seen.append(type(event).__name__) - - def on_secret_rotate(self, event: ops.SecretRotateEvent): - assert event.secret.id == 'secret:rotate' - assert event.secret.label == 'rot' - self.seen.append(type(event).__name__) - - def on_secret_remove(self, event: ops.SecretRemoveEvent): - assert event.secret.id == 'secret:remove' - assert event.secret.label == 'rem' - assert event.revision == 7 - self.seen.append(type(event).__name__) - - def on_secret_expired(self, event: ops.SecretExpiredEvent): - assert event.secret.id == 'secret:expired' - assert event.secret.label == 'exp' - assert event.revision == 42 - self.seen.append(type(event).__name__) - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - - charm.on.secret_changed.emit('secret:changed', None) - charm.on.secret_rotate.emit('secret:rotate', 'rot') - charm.on.secret_remove.emit('secret:remove', 'rem', 7) - charm.on.secret_expired.emit('secret:expired', 'exp', 42) - - assert charm.seen == [ - 'SecretChangedEvent', - 'SecretRotateEvent', - 'SecretRemoveEvent', - 'SecretExpiredEvent', - ] - - def test_collect_app_status_leader( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_status) - - def _on_collect_status(self, event: ops.CollectStatusEvent): - event.add_status(ops.ActiveStatus()) - event.add_status(ops.BlockedStatus('first')) - event.add_status(ops.WaitingStatus('waiting')) - event.add_status(ops.BlockedStatus('second')) - - fake_script_fixture.write('is-leader', 'echo true') - fake_script_fixture.write('status-set', 'exit 0') - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) + assert isinstance(meta.containers['test1'], ops.ContainerMeta) + assert isinstance(meta.containers['test1'].mounts["data"], ops.ContainerStorageMeta) + assert meta.containers['test1'].mounts["data"].locations[0] == \ + '/test/storagemount' + assert meta.containers['test1'].mounts["data"].locations[1] == '/test/otherdata' + + with pytest.raises(RuntimeError): + meta.containers["test1"].mounts["data"].location + + +def test_secret_events(request: pytest.FixtureRequest): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.seen: typing.List[str] = [] + self.framework.observe(self.on.secret_changed, self.on_secret_changed) + self.framework.observe(self.on.secret_rotate, self.on_secret_rotate) + self.framework.observe(self.on.secret_remove, self.on_secret_remove) + self.framework.observe(self.on.secret_expired, self.on_secret_expired) + + def on_secret_changed(self, event: ops.SecretChangedEvent): + assert event.secret.id == 'secret:changed' + assert event.secret.label is None + self.seen.append(type(event).__name__) + + def on_secret_rotate(self, event: ops.SecretRotateEvent): + assert event.secret.id == 'secret:rotate' + assert event.secret.label == 'rot' + self.seen.append(type(event).__name__) + + def on_secret_remove(self, event: ops.SecretRemoveEvent): + assert event.secret.id == 'secret:remove' + assert event.secret.label == 'rem' + assert event.revision == 7 + self.seen.append(type(event).__name__) + + def on_secret_expired(self, event: ops.SecretExpiredEvent): + assert event.secret.id == 'secret:expired' + assert event.secret.label == 'exp' + assert event.revision == 42 + self.seen.append(type(event).__name__) + + framework = create_framework(request) + charm = MyCharm(framework) + + charm.on.secret_changed.emit('secret:changed', None) + charm.on.secret_rotate.emit('secret:rotate', 'rot') + charm.on.secret_remove.emit('secret:remove', 'rem', 7) + charm.on.secret_expired.emit('secret:expired', 'exp', 42) + + assert charm.seen == [ + 'SecretChangedEvent', + 'SecretRotateEvent', + 'SecretRemoveEvent', + 'SecretExpiredEvent', + ] + + +def test_collect_app_status_leader( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_status) + + def _on_collect_status(self, event: ops.CollectStatusEvent): + event.add_status(ops.ActiveStatus()) + event.add_status(ops.BlockedStatus('first')) + event.add_status(ops.WaitingStatus('waiting')) + event.add_status(ops.BlockedStatus('second')) + + fake_script.write('is-leader', 'echo true') + fake_script.write('status-set', 'exit 0') + + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) + + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ['status-set', '--application=True', 'blocked', 'first'], + ] + + +def test_collect_app_status_no_statuses( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_status) + + def _on_collect_status(self, event: ops.CollectStatusEvent): + pass - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ['status-set', '--application=True', 'blocked', 'first'], - ] + fake_script.write('is-leader', 'echo true') - def test_collect_app_status_no_statuses( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_status) + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) - def _on_collect_status(self, event: ops.CollectStatusEvent): - pass + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ] - fake_script_fixture.write('is-leader', 'echo true') - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) +def test_collect_app_status_non_leader( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_status) - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ] + def _on_collect_status(self, event: ops.CollectStatusEvent): + raise Exception # shouldn't be called - def test_collect_app_status_non_leader( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_status) + fake_script.write('is-leader', 'echo false') - def _on_collect_status(self, event: ops.CollectStatusEvent): - raise Exception # shouldn't be called + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) - fake_script_fixture.write('is-leader', 'echo false') + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ] - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ] - - def test_collect_unit_status( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - - def _on_collect_status(self, event: ops.CollectStatusEvent): - event.add_status(ops.ActiveStatus()) - event.add_status(ops.BlockedStatus('first')) - event.add_status(ops.WaitingStatus('waiting')) - event.add_status(ops.BlockedStatus('second')) - - # called only for collecting app statuses - fake_script_fixture.write('is-leader', 'echo false') - fake_script_fixture.write('status-set', 'exit 0') - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) +def test_collect_unit_status( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ['status-set', '--application=False', 'blocked', 'first'], - ] - - def test_collect_unit_status_no_statuses( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - - def _on_collect_status(self, event: ops.CollectStatusEvent): - pass - - # called only for collecting app statuses - fake_script_fixture.write('is-leader', 'echo false') - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) + def _on_collect_status(self, event: ops.CollectStatusEvent): + event.add_status(ops.ActiveStatus()) + event.add_status(ops.BlockedStatus('first')) + event.add_status(ops.WaitingStatus('waiting')) + event.add_status(ops.BlockedStatus('second')) - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ] + # called only for collecting app statuses + fake_script.write('is-leader', 'echo false') + fake_script.write('status-set', 'exit 0') - def test_collect_app_and_unit_status( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_app_status) - self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status) + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) - def _on_collect_app_status(self, event: ops.CollectStatusEvent): - event.add_status(ops.ActiveStatus()) + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ['status-set', '--application=False', 'blocked', 'first'], + ] - def _on_collect_unit_status(self, event: ops.CollectStatusEvent): - event.add_status(ops.WaitingStatus('blah')) - fake_script_fixture.write('is-leader', 'echo true') - fake_script_fixture.write('status-set', 'exit 0') +def test_collect_unit_status_no_statuses( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - ops.charm._evaluate_status(charm) + def _on_collect_status(self, event: ops.CollectStatusEvent): + pass - assert fake_script_fixture.calls(True) == [ - ['is-leader', '--format=json'], - ['status-set', '--application=True', 'active', ''], - ['status-set', '--application=False', 'waiting', 'blah'], - ] - - def test_add_status_type_error( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_status) - - def _on_collect_status(self, event: ops.CollectStatusEvent): - event.add_status('active') # type: ignore - - fake_script_fixture.write('is-leader', 'echo true') - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework) - with pytest.raises(TypeError): - ops.charm._evaluate_status(charm) - - @pytest.mark.parametrize("statuses,expected", [ - (['blocked', 'error'], 'error'), - (['waiting', 'blocked'], 'blocked'), - (['waiting', 'maintenance'], 'maintenance'), - (['active', 'waiting'], 'waiting'), - (['active', 'unknown'], 'active'), - (['unknown'], 'unknown') - ]) - def test_collect_status_priority( - self, - monkeypatch: pytest.MonkeyPatch, - request: pytest.FixtureRequest, - fake_script_fixture: FakeScript, - statuses: typing.List[str], - expected: str): - class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any, statuses: typing.List[str]): - super().__init__(*args) - self.framework.observe(self.on.collect_app_status, self._on_collect_status) - self.statuses = statuses - - def _on_collect_status(self, event: ops.CollectStatusEvent): - for status in self.statuses: - event.add_status(ops.StatusBase.from_name(status, '')) - - fake_script_fixture.write('is-leader', 'echo true') - fake_script_fixture.write('status-set', 'exit 0') - - framework = create_framework(monkeypatch, request) - charm = MyCharm(framework, statuses=statuses) - ops.charm._evaluate_status(charm) + # called only for collecting app statuses + fake_script.write('is-leader', 'echo false') + + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) + + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ] + + +def test_collect_app_and_unit_status( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_app_status) + self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status) + + def _on_collect_app_status(self, event: ops.CollectStatusEvent): + event.add_status(ops.ActiveStatus()) + + def _on_collect_unit_status(self, event: ops.CollectStatusEvent): + event.add_status(ops.WaitingStatus('blah')) + + fake_script.write('is-leader', 'echo true') + fake_script.write('status-set', 'exit 0') - status_set_calls = [call for call in fake_script_fixture.calls(True) - if call[0] == 'status-set'] - assert status_set_calls == [ - ['status-set', '--application=True', expected, ''] - ] + framework = create_framework(request) + charm = MyCharm(framework) + ops.charm._evaluate_status(charm) + assert fake_script.calls(True) == [ + ['is-leader', '--format=json'], + ['status-set', '--application=True', 'active', ''], + ['status-set', '--application=False', 'waiting', 'blah'], + ] -class TestCharmMeta(unittest.TestCase): - def test_links(self): - # Each type of link can be a single item. - meta = ops.CharmMeta.from_yaml(""" + +def test_add_status_type_error( + request: pytest.FixtureRequest, + fake_script: FakeScript): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_status) + + def _on_collect_status(self, event: ops.CollectStatusEvent): + event.add_status('active') # type: ignore + + fake_script.write('is-leader', 'echo true') + + framework = create_framework(request) + charm = MyCharm(framework) + with pytest.raises(TypeError): + ops.charm._evaluate_status(charm) + + +@pytest.mark.parametrize("statuses,expected", [ + (['blocked', 'error'], 'error'), + (['waiting', 'blocked'], 'blocked'), + (['waiting', 'maintenance'], 'maintenance'), + (['active', 'waiting'], 'waiting'), + (['active', 'unknown'], 'active'), + (['unknown'], 'unknown') +]) +def test_collect_status_priority( + request: pytest.FixtureRequest, + fake_script: FakeScript, + statuses: typing.List[str], + expected: str): + class MyCharm(ops.CharmBase): + def __init__(self, *args: typing.Any, statuses: typing.List[str]): + super().__init__(*args) + self.framework.observe(self.on.collect_app_status, self._on_collect_status) + self.statuses = statuses + + def _on_collect_status(self, event: ops.CollectStatusEvent): + for status in self.statuses: + event.add_status(ops.StatusBase.from_name(status, '')) + + fake_script.write('is-leader', 'echo true') + fake_script.write('status-set', 'exit 0') + + framework = create_framework(request) + charm = MyCharm(framework, statuses=statuses) + ops.charm._evaluate_status(charm) + + status_set_calls = [call for call in fake_script.calls(True) + if call[0] == 'status-set'] + assert status_set_calls == [ + ['status-set', '--application=True', expected, ''] + ] + + +def test_meta_links(): + # Each type of link can be a single item. + meta = ops.CharmMeta.from_yaml(""" name: my-charm website: https://example.com source: https://git.example.com issues: https://bugs.example.com docs: https://docs.example.com """) - assert meta.links.websites == ['https://example.com'] - assert meta.links.sources == ['https://git.example.com'] - assert meta.links.issues == ['https://bugs.example.com'] - assert meta.links.documentation == 'https://docs.example.com' - # Other than documentation, they can also all be lists of items. - meta = ops.CharmMeta.from_yaml(""" + assert meta.links.websites == ['https://example.com'] + assert meta.links.sources == ['https://git.example.com'] + assert meta.links.issues == ['https://bugs.example.com'] + assert meta.links.documentation == 'https://docs.example.com' + # Other than documentation, they can also all be lists of items. + meta = ops.CharmMeta.from_yaml(""" name: my-charm website: - https://example.com @@ -983,14 +973,15 @@ def test_links(self): - https://bugs.example.com - https://features.example.com """) - assert meta.links.websites == ['https://example.com', 'https://example.org'] - assert meta.links.sources == [ - 'https://git.example.com', 'https://bzr.example.com'] - assert meta.links.issues == [ - 'https://bugs.example.com', 'https://features.example.com'] - - def test_links_charmcraft_yaml(self): - meta = ops.CharmMeta.from_yaml(""" + assert meta.links.websites == ['https://example.com', 'https://example.org'] + assert meta.links.sources == [ + 'https://git.example.com', 'https://bzr.example.com'] + assert meta.links.issues == [ + 'https://bugs.example.com', 'https://features.example.com'] + + +def test_meta_links_charmcraft_yaml(): + meta = ops.CharmMeta.from_yaml(""" links: documentation: https://discourse.example.com/ issues: @@ -1001,25 +992,26 @@ def test_links_charmcraft_yaml(self): - https://example.com/ contact: Support Team """) - assert meta.links.websites == ['https://example.com/'] - assert meta.links.sources == ['https://git.example.com/issues/'] - assert meta.links.issues == ['https://git.example.com/'] - assert meta.links.documentation == 'https://discourse.example.com/' - assert meta.maintainers == ['Support Team '] - - def test_assumes(self): - meta = ops.CharmMeta.from_yaml(""" + assert meta.links.websites == ['https://example.com/'] + assert meta.links.sources == ['https://git.example.com/issues/'] + assert meta.links.issues == ['https://git.example.com/'] + assert meta.links.documentation == 'https://discourse.example.com/' + assert meta.maintainers == ['Support Team '] + + +def test_meta_assumes(): + meta = ops.CharmMeta.from_yaml(""" assumes: - juju """) - assert meta.assumes.features == ['juju'] - meta = ops.CharmMeta.from_yaml(""" + assert meta.assumes.features == ['juju'] + meta = ops.CharmMeta.from_yaml(""" assumes: - juju > 3 - k8s-api """) - assert meta.assumes.features == ['juju > 3', 'k8s-api'] - meta = ops.CharmMeta.from_yaml(""" + assert meta.assumes.features == ['juju > 3', 'k8s-api'] + meta = ops.CharmMeta.from_yaml(""" assumes: - k8s-api - any-of: @@ -1030,11 +1022,11 @@ def test_assumes(self): - juju >= 3.1.5 - juju < 4 """) - assert meta.assumes.features == [ - 'k8s-api', - ops.JujuAssumes( - [ops.JujuAssumes(['juju >= 2.9.44', 'juju < 3']), - ops.JujuAssumes(['juju >= 3.1.5', 'juju < 4'])], - ops.JujuAssumesCondition.ANY - ), - ] + assert meta.assumes.features == [ + 'k8s-api', + ops.JujuAssumes( + [ops.JujuAssumes(['juju >= 2.9.44', 'juju < 3']), + ops.JujuAssumes(['juju >= 3.1.5', 'juju < 4'])], + ops.JujuAssumesCondition.ANY + ), + ] diff --git a/test/test_helpers.py b/test/test_helpers.py index ad6f668f6..3c3a85a36 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import functools import os import pathlib import shutil @@ -78,15 +77,16 @@ def fake_script_calls(test_case: unittest.TestCase, def create_framework( - monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest, *, - meta: typing.Union[ops.CharmMeta, None] = None): - monkeypatch.setenv("PATH", str(pathlib.Path(__file__).parent / 'bin'), prepend=os.pathsep) - monkeypatch.setenv("JUJU_UNIT_NAME", "local/0") + meta: typing.Optional[ops.CharmMeta] = None): + env_backup = os.environ.copy() + os.environ['PATH'] = os.pathsep.join([ + str(pathlib.Path(__file__).parent / 'bin'), + os.environ['PATH']]) + os.environ['JUJU_UNIT_NAME'] = 'local/0' tmpdir = pathlib.Path(tempfile.mkdtemp()) - request.addfinalizer(functools.partial(shutil.rmtree, str(tmpdir))) class CustomEvent(ops.EventBase): pass @@ -98,51 +98,56 @@ class TestCharmEvents(ops.CharmEvents): # We use a subclass temporarily to prevent these side effects from leaking. ops.CharmBase.on = TestCharmEvents() # type: ignore - def cleanup(): - ops.CharmBase.on = ops.CharmEvents() # type: ignore - - request.addfinalizer(cleanup) - - if not meta: + if meta is None: meta = ops.CharmMeta() model = ops.Model(meta, _ModelBackend('local/0')) # type: ignore - # we can pass foo_event as event_name because we're not actually testing dispatch + # We can pass foo_event as event_name because we're not actually testing dispatch. framework = ops.Framework(SQLiteStorage(':memory:'), tmpdir, meta, model) # type: ignore - request.addfinalizer(framework.close) + + def finalizer(): + os.environ.clear() + os.environ.update(env_backup) + shutil.rmtree(tmpdir) + ops.CharmBase.on = ops.CharmEvents() # type: ignore + framework.close() + + request.addfinalizer(finalizer) return framework class FakeScript: def __init__(self, - monkeypatch: pytest.MonkeyPatch, - tmp_path: pathlib.Path, - fake_script_path: typing.Union[pathlib.Path, None] = None): - self.monkeypatch = monkeypatch - if fake_script_path is None: - self.fake_script_path = tmp_path - self.monkeypatch.setenv( - "PATH", - str(self.fake_script_path), - prepend=os.pathsep) # type: ignore + request: pytest.FixtureRequest, + path: typing.Optional[pathlib.Path] = None): + if path is None: + fake_script_path = tempfile.mkdtemp('-fake_script') + self.path = pathlib.Path(fake_script_path) + old_path = os.environ["PATH"] + os.environ['PATH'] = os.pathsep.join([fake_script_path, os.environ["PATH"]]) + + def cleanup(): + shutil.rmtree(self.path) + os.environ['PATH'] = old_path + + request.addfinalizer(cleanup) else: - self.fake_script_path = fake_script_path + self.path = path def write(self, name: str, content: str): template_args: typing.Dict[str, str] = { 'name': name, - 'path': self.fake_script_path.as_posix(), # type: ignore + 'path': self.path.as_posix(), 'content': content, } - path: pathlib.Path = self.fake_script_path / name # type: ignore - with path.open('wt') as f: # type: ignore + path: pathlib.Path = self.path / name + with path.open('wt') as f: # Before executing the provided script, dump the provided arguments in calls.txt. - # ASCII 1E is RS 'record separator', and 1C is FS 'file separator', which - # seem appropriate. - f.write( # type: ignore + # RS 'record separator' (octal 036 in ASCII), FS 'file separator' (octal 034 in ASCII). + f.write( '''#!/bin/sh {{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt {content}'''.format_map(template_args)) @@ -153,17 +158,17 @@ def write(self, f'@"C:\\Program Files\\git\\bin\\bash.exe" {path} %*\n') def calls(self, clear: bool = False) -> typing.List[typing.List[str]]: - calls_file: pathlib.Path = self.fake_script_path / 'calls.txt' # type: ignore - if not calls_file.exists(): # type: ignore + calls_file: pathlib.Path = self.path / 'calls.txt' + if not calls_file.exists(): return [] - # newline and encoding forced to linuxy defaults because on - # windows they're written from git-bash - with calls_file.open('r+t', newline='\n', encoding='utf8') as f: # type: ignore - calls = [line.split('\x1e') for line in f.read().split('\x1c')[:-1]] # type: ignore + # Newline and encoding forced to Linux-y defaults because on + # windows they're written from git-bash. + with calls_file.open('r+t', newline='\n', encoding='utf8') as f: + calls = [line.split('\x1e') for line in f.read().split('\x1c')[:-1]] if clear: - f.truncate(0) # type: ignore - return calls # type: ignore + f.truncate(0) + return calls class FakeScriptTest(unittest.TestCase): From d5944020c5cab34d49ab68d4ddb086bf37ab79af Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 23 Apr 2024 10:45:15 +0800 Subject: [PATCH 12/16] chore: move request fixture as the first in test charm --- test/test_charm.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index 7364e4143..a6167e6e2 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -97,8 +97,9 @@ def _on_start(self, event: ops.EventBase): def test_observer_not_referenced_warning( - caplog: pytest.LogCaptureFixture, - request: pytest.FixtureRequest): + request: pytest.FixtureRequest, + caplog: pytest.LogCaptureFixture + ): class MyObj(ops.Object): def __init__(self, charm: ops.CharmBase): super().__init__(charm, "obj") @@ -567,8 +568,8 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): def test_action_event_defer_fails( - monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest, + monkeypatch: pytest.MonkeyPatch, fake_script: FakeScript): cmd_type = 'action' From 487a2a7cebaccc35055d192297bbbecd7e5a659d Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 23 Apr 2024 10:51:45 +0800 Subject: [PATCH 13/16] chore: reformat test function params --- test/test_charm.py | 75 ++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 43 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index a6167e6e2..85a39f113 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -97,9 +97,9 @@ def _on_start(self, event: ops.EventBase): def test_observer_not_referenced_warning( - request: pytest.FixtureRequest, - caplog: pytest.LogCaptureFixture - ): + request: pytest.FixtureRequest, + caplog: pytest.LogCaptureFixture +): class MyObj(ops.Object): def __init__(self, charm: ops.CharmBase): super().__init__(charm, "obj") @@ -208,8 +208,7 @@ def on_any_relation(self, event: ops.RelationEvent): ] -def test_storage_events(request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_storage_events(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -469,11 +468,11 @@ def test_actions_from_charm_root(): assert meta.actions['foo'].description == "foos the bar" -def _setup_test_action(fake_script: typing.Callable[..., None]): - fake_script('action-get', """echo '{"foo-name": "name", "silent": true}'""") - fake_script('action-set', "") - fake_script('action-log', "") - fake_script('action-fail', "") +def _setup_test_action(fake_script: FakeScript): + fake_script.write('action-get', """echo '{"foo-name": "name", "silent": true}'""") + fake_script.write('action-set', "") + fake_script.write('action-log', "") + fake_script.write('action-fail', "") def _get_action_test_meta(): @@ -516,7 +515,7 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): def _on_start_action(self, event: ops.ActionEvent): pass - _setup_test_action(fake_script.write) + _setup_test_action(fake_script) meta = _get_action_test_meta() framework = create_framework(request, meta=meta) charm = MyCharm(framework) @@ -543,9 +542,11 @@ def _on_start_action(self, event: ops.ActionEvent): {'a': {None: 'c'}}, {'aBc': 'd'} ]) -def test_invalid_action_results(request: pytest.FixtureRequest, - fake_script: FakeScript, - bad_res: typing.Dict[str, typing.Any]): +def test_invalid_action_results( + request: pytest.FixtureRequest, + fake_script: FakeScript, + bad_res: typing.Dict[str, typing.Any] +): class MyCharm(ops.CharmBase): @@ -557,7 +558,7 @@ def __init__(self, *args: typing.Any): def _on_foo_bar_action(self, event: ops.ActionEvent): event.set_results(self.res) - _setup_test_action(fake_script.write) + _setup_test_action(fake_script) meta = _get_action_test_meta() framework = create_framework(request, meta=meta) charm = MyCharm(framework) @@ -568,9 +569,10 @@ def _on_foo_bar_action(self, event: ops.ActionEvent): def test_action_event_defer_fails( - request: pytest.FixtureRequest, - monkeypatch: pytest.MonkeyPatch, - fake_script: FakeScript): + request: pytest.FixtureRequest, + monkeypatch: pytest.MonkeyPatch, + fake_script: FakeScript +): cmd_type = 'action' @@ -741,9 +743,7 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): ] -def test_collect_app_status_leader( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_app_status_leader(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -768,9 +768,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): ] -def test_collect_app_status_no_statuses( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_app_status_no_statuses(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -790,9 +788,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): ] -def test_collect_app_status_non_leader( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_app_status_non_leader(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -812,9 +808,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): ] -def test_collect_unit_status( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_unit_status(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -840,9 +834,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): ] -def test_collect_unit_status_no_statuses( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_unit_status_no_statuses(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -863,9 +855,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): ] -def test_collect_app_and_unit_status( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_collect_app_and_unit_status(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -892,9 +882,7 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent): ] -def test_add_status_type_error( - request: pytest.FixtureRequest, - fake_script: FakeScript): +def test_add_status_type_error(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): super().__init__(*args) @@ -920,10 +908,11 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): (['unknown'], 'unknown') ]) def test_collect_status_priority( - request: pytest.FixtureRequest, - fake_script: FakeScript, - statuses: typing.List[str], - expected: str): + request: pytest.FixtureRequest, + fake_script: FakeScript, + statuses: typing.List[str], + expected: str +): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any, statuses: typing.List[str]): super().__init__(*args) From 32394d708617d078719b571b83cfb91c502b3965 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 23 Apr 2024 11:03:45 +0800 Subject: [PATCH 14/16] chore: make quotes consistent in the same func --- test/test_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_helpers.py b/test/test_helpers.py index 3c3a85a36..e1669b113 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -123,8 +123,8 @@ def __init__(self, if path is None: fake_script_path = tempfile.mkdtemp('-fake_script') self.path = pathlib.Path(fake_script_path) - old_path = os.environ["PATH"] - os.environ['PATH'] = os.pathsep.join([fake_script_path, os.environ["PATH"]]) + old_path = os.environ['PATH'] + os.environ['PATH'] = os.pathsep.join([fake_script_path, os.environ['PATH']]) def cleanup(): shutil.rmtree(self.path) From 32970a1d48aeb958575fc7a7520aec750a92a289 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Wed, 24 Apr 2024 12:10:39 +0800 Subject: [PATCH 15/16] chore: refactor according to reviews --- test/test_charm.py | 76 ++++++++++++++++++++++---------------------- test/test_helpers.py | 20 ++++++------ 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index 85a39f113..5d72bc7d2 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -35,8 +35,8 @@ def fake_script(request: pytest.FixtureRequest) -> FakeScript: def test_basic(request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.started = False framework.observe(self.on.start, self._on_start) @@ -60,7 +60,7 @@ def _on_start(self, event: ops.EventBase): def test_observe_decorated_method(request: pytest.FixtureRequest): - # we test that charm methods decorated with @functools.wraps(wrapper) + # We test that charm methods decorated with @functools.wraps(wrapper) # can be observed by Framework. Simpler decorators won't work because # Framework searches for __self__ and other method things; functools.wraps # is more careful and it still works, this test is here to ensure that @@ -78,8 +78,8 @@ def wrapper(charm: 'MyCharm', evt: ops.EventBase): return wrapper class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) framework.observe(self.on.start, self._on_start) self.seen = None @@ -109,8 +109,8 @@ def _on_start(self, _: ops.StartEvent): raise RuntimeError() # never reached! class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) MyObj(self) # not assigned! framework.observe(self.on.start, self._on_start) @@ -144,8 +144,8 @@ class MyCharm(ops.CharmBase): def test_relation_events(request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.seen: typing.List[str] = [] for rel in ('req1', 'req-2', 'pro1', 'pro-2', 'peer1', 'peer-2'): # Hook up relation events to generic handler. @@ -210,8 +210,8 @@ def on_any_relation(self, event: ops.RelationEvent): def test_storage_events(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.seen: typing.List[str] = [] self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) @@ -325,8 +325,8 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): def test_workload_events(request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.seen: typing.List[str] = [] for workload in ('container-a', 'containerb'): # Hook up relation events to generic handler. @@ -501,8 +501,8 @@ def test_action_events(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) framework.observe(self.on.start_action, self._on_start_action) @@ -550,8 +550,8 @@ def test_invalid_action_results( class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.res: typing.Dict[str, typing.Any] = {} framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) @@ -578,8 +578,8 @@ def test_action_event_defer_fails( class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) framework.observe(self.on.start_action, self._on_start_action) def _on_start_action(self, event: ops.ActionEvent): @@ -697,8 +697,8 @@ def test_containers_storage_multiple_mounts(): def test_secret_events(request: pytest.FixtureRequest): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.seen: typing.List[str] = [] self.framework.observe(self.on.secret_changed, self.on_secret_changed) self.framework.observe(self.on.secret_rotate, self.on_secret_rotate) @@ -745,8 +745,8 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent): def test_collect_app_status_leader(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -770,8 +770,8 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): def test_collect_app_status_no_statuses(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -790,8 +790,8 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): def test_collect_app_status_non_leader(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -810,8 +810,8 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): def test_collect_unit_status(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_unit_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -836,8 +836,8 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): def test_collect_unit_status_no_statuses(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_unit_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -857,8 +857,8 @@ def _on_collect_status(self, event: ops.CollectStatusEvent): def test_collect_app_and_unit_status(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_app_status) self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status) @@ -884,8 +884,8 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent): def test_add_status_type_error(request: pytest.FixtureRequest, fake_script: FakeScript): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any): - super().__init__(*args) + def __init__(self, framework: ops.Framework): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_status) def _on_collect_status(self, event: ops.CollectStatusEvent): @@ -911,11 +911,11 @@ def test_collect_status_priority( request: pytest.FixtureRequest, fake_script: FakeScript, statuses: typing.List[str], - expected: str + expected: str, ): class MyCharm(ops.CharmBase): - def __init__(self, *args: typing.Any, statuses: typing.List[str]): - super().__init__(*args) + def __init__(self, framework: ops.Framework, statuses: typing.List[str]): + super().__init__(framework) self.framework.observe(self.on.collect_app_status, self._on_collect_status) self.statuses = statuses diff --git a/test/test_helpers.py b/test/test_helpers.py index e1669b113..ba3534b14 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -117,9 +117,11 @@ def finalizer(): class FakeScript: - def __init__(self, - request: pytest.FixtureRequest, - path: typing.Optional[pathlib.Path] = None): + def __init__( + self, + request: pytest.FixtureRequest, + path: typing.Optional[pathlib.Path] = None, + ): if path is None: fake_script_path = tempfile.mkdtemp('-fake_script') self.path = pathlib.Path(fake_script_path) @@ -134,9 +136,7 @@ def cleanup(): else: self.path = path - def write(self, - name: str, - content: str): + def write(self, name: str, content: str): template_args: typing.Dict[str, str] = { 'name': name, 'path': self.path.as_posix(), @@ -149,9 +149,9 @@ def write(self, # RS 'record separator' (octal 036 in ASCII), FS 'file separator' (octal 034 in ASCII). f.write( '''#!/bin/sh - {{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt - {content}'''.format_map(template_args)) - os.chmod(str(path), 0o755) # type: ignore # noqa: S103 +{{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt +{content}'''.format_map(template_args)) + path.chmod(0o755) # noqa: S103 # TODO: this hardcodes the path to bash.exe, which works for now but might # need to be set via environ or something like that. path.with_suffix(".bat").write_text( # type: ignore @@ -165,7 +165,7 @@ def calls(self, clear: bool = False) -> typing.List[typing.List[str]]: # Newline and encoding forced to Linux-y defaults because on # windows they're written from git-bash. with calls_file.open('r+t', newline='\n', encoding='utf8') as f: - calls = [line.split('\x1e') for line in f.read().split('\x1c')[:-1]] + calls = [line.split('\036') for line in f.read().split('\034')[:-1]] if clear: f.truncate(0) return calls From 6bd8c52ba4235a6390194e0ca0c60bd7b6020fd9 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Wed, 24 Apr 2024 12:11:59 +0800 Subject: [PATCH 16/16] chore: refactor according to reviews --- test/test_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helpers.py b/test/test_helpers.py index ba3534b14..38e52e92a 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -151,7 +151,7 @@ def write(self, name: str, content: str): '''#!/bin/sh {{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt {content}'''.format_map(template_args)) - path.chmod(0o755) # noqa: S103 + path.chmod(0o755) # TODO: this hardcodes the path to bash.exe, which works for now but might # need to be set via environ or something like that. path.with_suffix(".bat").write_text( # type: ignore