Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add type hinting to test_lib #1012

Merged
merged 7 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ops/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import os
import re
import sys
import typing
import warnings
from ast import literal_eval
from importlib.machinery import ModuleSpec
Expand Down Expand Up @@ -117,7 +118,7 @@ def autoimport():
versions.sort(reverse=True)


def _find_all_specs(path):
def _find_all_specs(path: typing.Iterable[str]) -> typing.Iterator[ModuleSpec]:
for sys_dir in path:
if sys_dir == "":
sys_dir = "."
Expand Down Expand Up @@ -192,7 +193,7 @@ def __str__(self):
return f"got {_join_and(sorted(got))}, but missing {_join_and(sorted(exp - got))}"


def _parse_lib(spec):
def _parse_lib(spec: ModuleSpec) -> typing.Optional["_Lib"]:
if spec.origin is None:
# "can't happen"
logger.warning("No origin for %r (no idea why; please report)", spec.name)
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ include = ["ops/*.py", "ops/_private/*.py",
"test/test_infra.py",
"test/test_jujuversion.py",
"test/test_log.py",
"test/test_lib.py",
]
pythonVersion = "3.8" # check no python > 3.8 features are used
pythonPlatform = "All"
Expand Down
1 change: 0 additions & 1 deletion requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
isort==5.11.4
logassert==7
autopep8==1.6.0
flake8==4.0.1
flake8-docstrings==1.6.0
Expand Down
51 changes: 25 additions & 26 deletions test/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
from test.test_helpers import BaseTestCase, fake_script
from unittest.mock import patch

import logassert

import ops
from ops.framework import _BREAKPOINT_WELCOME_MESSAGE, _event_regex
from ops.storage import NoSnapshotError, SQLiteStorage
Expand All @@ -41,13 +39,14 @@ def setUp(self):
patcher = patch('ops.storage.SQLiteStorage.DB_LOCK_TIMEOUT', datetime.timedelta(0))
patcher.start()
self.addCleanup(patcher.stop)
logassert.setup(self, 'ops')

def test_deprecated_init(self):
# For 0.7, this still works, but it is deprecated.
framework = ops.Framework(':memory:', None, None, None)
self.assertLoggedWarning(
"deprecated: Framework now takes a Storage not a path")
with self.assertLogs(level="WARNING") as cm:
framework = ops.Framework(':memory:', None, None, None)
self.assertIn(
"WARNING:ops.framework:deprecated: Framework now takes a Storage not a path",
cm.output)
self.assertIsInstance(framework._storage, SQLiteStorage)

def test_handle_path(self):
Expand Down Expand Up @@ -356,12 +355,7 @@ def _on_foo(self, event):
obs = MyObserver(framework, "1")

framework.observe(pub.foo, obs._on_foo)

self.assertNotLogged("Deferring")
pub.foo.emit(1)
self.assertLogged("Deferring <MyEvent via MyNotifier[1]/foo[1]>.")
self.assertNotLogged("Re-emitting")

framework.reemit()

# Two things being checked here:
Expand All @@ -375,7 +369,6 @@ def _on_foo(self, event):
# we'd get a foo=3).
#
self.assertEqual(obs.seen, ["on_foo:foo=2", "on_foo:foo=2"])
self.assertLoggedDebug("Re-emitting deferred event <MyEvent via MyNotifier[1]/foo[1]>.")

def test_weak_observer(self):
framework = self.create_framework()
Expand Down Expand Up @@ -1487,21 +1480,24 @@ def callback_method(self, event):
@patch('sys.stderr', new_callable=io.StringIO)
class BreakpointTests(BaseTestCase):

def setUp(self):
super().setUp()
logassert.setup(self, 'ops')

def test_ignored(self, fake_stderr):
# It doesn't do anything really unless proper environment is there.
with patch.dict(os.environ):
os.environ.pop('JUJU_DEBUG_AT', None)
framework = self.create_framework()

with patch('pdb.Pdb.set_trace') as mock:
framework.breakpoint()
# We want to verify that there are *no* logs at warning level.
# However, assertNoLogs is Python 3.10+.
try:
with self.assertLogs(level="WARNING"):
framework.breakpoint()
except AssertionError:
pass
else:
self.fail("No warning logs should be generated")
self.assertEqual(mock.call_count, 0)
self.assertEqual(fake_stderr.getvalue(), "")
self.assertNotLoggedWarning("Breakpoint", "skipped")

def test_pdb_properly_called(self, fake_stderr):
# The debugger needs to leave the user in the frame where the breakpoint is executed,
Expand Down Expand Up @@ -1658,17 +1654,20 @@ def test_named_indicated_specifically(self, fake_stderr):

def test_named_indicated_unnamed(self, fake_stderr):
# Some breakpoint was indicated, but the framework call was unnamed
self.check_trace_set('some-breakpoint', None, 0)
self.assertLoggedWarning(
"Breakpoint None skipped",
"not found in the requested breakpoints: {'some-breakpoint'}")
with self.assertLogs(level="WARNING") as cm:
self.check_trace_set('some-breakpoint', None, 0)
self.assertEqual(cm.output, [
"WARNING:ops.framework:Breakpoint None skipped "
"(not found in the requested breakpoints: {'some-breakpoint'})"
])

def test_named_indicated_somethingelse(self, fake_stderr):
# Some breakpoint was indicated, but the framework call was with a different name
self.check_trace_set('some-breakpoint', 'other-name', 0)
self.assertLoggedWarning(
"Breakpoint 'other-name' skipped",
"not found in the requested breakpoints: {'some-breakpoint'}")
with self.assertLogs(level="WARNING") as cm:
self.check_trace_set('some-breakpoint', 'other-name', 0)
self.assertEqual(cm.output, [
"WARNING:ops.framework:Breakpoint 'other-name' skipped "
"(not found in the requested breakpoints: {'some-breakpoint'})"])

def test_named_indicated_ingroup(self, fake_stderr):
# A multiple breakpoint was indicated, and the framework call used a name among those.
Expand Down
Loading
Loading