From 3e356ac7154290c3d7a542981810fad1b3af1139 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 29 Jul 2022 16:00:00 +0200 Subject: [PATCH] fix Trio100 bug, add test_messages_documented and test_trio_tests, clean up tests --- README.md | 7 ++-- flake8_trio.py | 3 +- tests/test_changelog_and_version.py | 25 +++++++++++++- tests/test_flake8_trio.py | 13 ++++---- tests/test_trio_tests.py | 51 +++++++++++++++++++++++++++++ tests/trio100.py | 7 ++-- tests/trio100_py39.py | 6 ++-- tests/trio102.py | 8 ++--- tests/trio105.py | 35 ++++++++++---------- tests/trio106.py | 6 ++-- 10 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 tests/test_trio_tests.py diff --git a/README.md b/README.md index 33f8546..78f01ee 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,8 @@ pip install flake8-trio - **TRIO100**: a `with trio.fail_after(...):` or `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. -- **TRIO101** `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. -- **TRIO102** it's unsafe to await inside `finally:` unless you use a shielded +- **TRIO101**: `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. +- **TRIO102**: it's unsafe to await inside `finally:` unless you use a shielded cancel scope with a timeout" -- **TRIO105** Calling a trio async function without immediately `await`ing it. +- **TRIO105**: Calling a trio async function without immediately `await`ing it. +- **TRIO106**: trio must be imported with `import trio` for the linter to work diff --git a/flake8_trio.py b/flake8_trio.py index dbcadc6..7c70f0b 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -150,7 +150,8 @@ def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]) -> None: for item in (i.context_expr for i in node.items): call = get_trio_scope(item, *cancel_scope_names) if call and not any( - isinstance(x, checkpoint_node_types) for x in ast.walk(node) + isinstance(x, checkpoint_node_types) and x != node + for x in ast.walk(node) ): self.problems.append( make_error(TRIO100, item.lineno, item.col_offset, call) diff --git a/tests/test_changelog_and_version.py b/tests/test_changelog_and_version.py index 7e02ed6..e83c8bb 100644 --- a/tests/test_changelog_and_version.py +++ b/tests/test_changelog_and_version.py @@ -1,7 +1,8 @@ """Tests for flake8-trio package metadata.""" import re +import unittest from pathlib import Path -from typing import Iterable, NamedTuple +from typing import Dict, Iterable, NamedTuple, Set import flake8_trio @@ -43,3 +44,25 @@ def test_version_increments_are_correct(): assert prev[:2] < current[:2], msg else: assert current == prev._replace(patch=prev.patch + 1), msg + + +class test_messages_documented(unittest.TestCase): + def runTest(self): + documented_errors: Dict[str, Set[str]] = {} + for filename in ( + "CHANGELOG.md", + "README.md", + "flake8_trio.py", + "tests/test_flake8_trio.py", + ): + with open(Path(__file__).parent.parent / filename) as f: + lines = f.readlines() + documented_errors[filename] = set() + for line in lines: + for error_msg in re.findall(r"TRIO\d\d\d", line): + documented_errors[filename].add(error_msg) + + file_items = list(documented_errors.items()) + first = file_items.pop() + for file, documented in file_items: + self.assertSetEqual(first[1], documented, msg=(first[0], file)) diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 3862a51..d24fa82 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -29,18 +29,19 @@ def assert_expected_errors(self, test_file: str, *expected: Error) -> None: filename = Path(__file__).absolute().parent / test_file plugin = Plugin.from_filename(str(filename)) errors = tuple(plugin.run()) - self.assertEqual(errors, expected) + self.assertTupleEqual(errors, expected) def test_tree(self): plugin = Plugin(ast.parse("")) errors = list(plugin.run()) - self.assertEqual(errors, []) + self.assertSequenceEqual(errors, []) def test_trio100(self): self.assert_expected_errors( "trio100.py", make_error(TRIO100, 3, 5, "trio.move_on_after"), - make_error(TRIO100, 23, 9, "trio.fail_after"), + make_error(TRIO100, 8, 15, "trio.fail_after"), + make_error(TRIO100, 26, 9, "trio.fail_after"), ) @unittest.skipIf(sys.version_info < (3, 9), "requires 3.9+") @@ -100,11 +101,11 @@ def test_trio105(self): make_error(TRIO105, 36, 4, "sleep"), make_error(TRIO105, 37, 4, "sleep_forever"), make_error(TRIO105, 38, 4, "sleep_until"), - make_error(TRIO105, 45, 15, "open_file"), - make_error(TRIO105, 50, 8, "open_file"), + make_error(TRIO105, 44, 15, "open_file"), + make_error(TRIO105, 49, 8, "open_file"), ) - self.assertEqual( + self.assertSetEqual( set(trio_async_functions), { o[0] diff --git a/tests/test_trio_tests.py b/tests/test_trio_tests.py new file mode 100644 index 0000000..41db3db --- /dev/null +++ b/tests/test_trio_tests.py @@ -0,0 +1,51 @@ +import inspect +import os +import os.path +import re +import unittest +from types import FunctionType +from typing import Dict, Optional, Set, Tuple + +from test_flake8_trio import Flake8TrioTestCase + + +class TestTrioTests(unittest.TestCase): + def runTest(self): + # get files + trio_tests: Dict[str, Tuple[str, Optional[FunctionType]]] = { + os.path.splitext(f)[0]: (f, None) + for f in os.listdir("tests") + if re.match(r"^trio.*.py", f) + } + + # get functions + for o in inspect.getmembers(Flake8TrioTestCase): + if inspect.isfunction(o[1]) and re.match(r"^test_trio\d\d\d", o[0]): + key = o[0][5:] + + self.assertIn(key, trio_tests) + self.assertIsNone(trio_tests[key][1], msg=key) + + trio_tests[key] = (trio_tests[key][0], o[1]) + + for test, (filename, func) in sorted(trio_tests.items()): + self.assertIsNotNone(func, msg=test) + assert func is not None # for type checkers + + with open(os.path.join("tests", filename), encoding="utf-8") as file: + file_error_lines = { + lineno + 1 + for lineno, line in enumerate(file) + if re.search(r"# *error", line, flags=re.I) + } + + func_error_lines: Set[int] = set() + for line in inspect.getsourcelines(func)[0]: + m = re.search(r"(?<=make_error\(TRIO\d\d\d, )\d*", line) + if not m: + continue + lineno = int(m.group()) + self.assertNotIn(lineno, func_error_lines, msg=test) + func_error_lines.add(lineno) + + self.assertSetEqual(file_error_lines, func_error_lines, msg=test) diff --git a/tests/trio100.py b/tests/trio100.py index f305434..41a49ac 100644 --- a/tests/trio100.py +++ b/tests/trio100.py @@ -1,10 +1,13 @@ import trio -with trio.move_on_after(10): +with trio.move_on_after(10): # error pass async def function_name(): + async with trio.fail_after(10): # error + pass + with trio.move_on_after(10): await trio.sleep(1) @@ -20,7 +23,7 @@ async def function_name(): with open("filename") as _: pass - with trio.fail_after(10): + with trio.fail_after(10): # error pass send_channel, receive_channel = trio.open_memory_channel(0) diff --git a/tests/trio100_py39.py b/tests/trio100_py39.py index 50ad2d4..a033e63 100644 --- a/tests/trio100_py39.py +++ b/tests/trio100_py39.py @@ -4,13 +4,13 @@ async def function_name(): with ( open("veryverylongfilenamesoshedsplitsthisintotwolines") as _, - trio.fail_after(10), + trio.fail_after(10), # error ): pass with ( - trio.fail_after(5), + trio.fail_after(5), # error open("veryverylongfilenamesoshedsplitsthisintotwolines") as _, - trio.move_on_after(5), + trio.move_on_after(5), # error ): pass diff --git a/tests/trio102.py b/tests/trio102.py index f92cca1..2111f28 100644 --- a/tests/trio102.py +++ b/tests/trio102.py @@ -67,7 +67,7 @@ async def foo(): pass finally: with open("bar"): - await foo() # safe + await foo() # error with open("bar"): pass with trio.move_on_after(): @@ -81,11 +81,11 @@ async def foo(): with trio.CancelScope(deadline=30): await foo() # error with trio.CancelScope(deadline=30, shield=(1 == 1)): - await foo() # safe in theory, but deemed error + await foo() # error: though safe in theory myvar = True with trio.open_nursery(10) as s: s.shield = myvar - await foo() # safe in theory, but deemed error + await foo() # error: though safe in theory with trio.CancelScope(deadline=30, shield=True): with trio.move_on_after(30): await foo() # safe @@ -120,4 +120,4 @@ async def foo3(): await foo() # safe with trio.fail_after(5), trio.move_on_after(30) as s: s.shield = True - await foo() # safe in theory, but we don't bother parsing + await foo() # error: safe in theory, but we don't bother parsing diff --git a/tests/trio105.py b/tests/trio105.py index d121361..425a8b2 100644 --- a/tests/trio105.py +++ b/tests/trio105.py @@ -21,31 +21,30 @@ async def foo(): await trio.sleep_forever() await trio.sleep_until() - # errors - trio.aclose_forcefully() - trio.open_file() - trio.open_ssl_over_tcp_listeners() - trio.open_ssl_over_tcp_stream() - trio.open_tcp_listeners() - trio.open_tcp_stream() - trio.open_unix_socket() - trio.run_process() - trio.serve_listeners() - trio.serve_ssl_over_tcp() - trio.serve_tcp() - trio.sleep() - trio.sleep_forever() - trio.sleep_until() + # all async functions + trio.aclose_forcefully() # error + trio.open_file() # error + trio.open_ssl_over_tcp_listeners() # error + trio.open_ssl_over_tcp_stream() # error + trio.open_tcp_listeners() # error + trio.open_tcp_stream() # error + trio.open_unix_socket() # error + trio.run_process() # error + trio.serve_listeners() # error + trio.serve_ssl_over_tcp() # error + trio.serve_tcp() # error + trio.sleep() # error + trio.sleep_forever() # error + trio.sleep_until() # error # safe async with await trio.open_file() as f: pass - # error - async with trio.open_file() as f: + async with trio.open_file() as f: # error pass # safe in theory, but deemed sufficiently poor style that parsing # it isn't supported - k = trio.open_file() + k = trio.open_file() # error await k diff --git a/tests/trio106.py b/tests/trio106.py index 2da0ab9..f5ab448 100644 --- a/tests/trio106.py +++ b/tests/trio106.py @@ -1,9 +1,9 @@ import importlib import trio -import trio as foo -from trio import * # noqa -from trio import blah, open_file as foo # noqa +import trio as foo # error +from trio import * # noqa # error +from trio import blah, open_file as foo # noqa # error # Note that our tests exercise the Visitor classes, without going through the noqa filter later in flake8 - so these suppressions are picked up by our project-wide linter stack but not the tests.