Reorganize tests - #89

tests/decoration/test_method_function_decorator_async.py (1)

406-408: 🧹 Nitpick | 🔵 Trivial

Remove these PR-thread TODOs from the test body.

These comments leave the assertion strategy undecided and couple the suite to this PR discussion. Either keep these Promise-type checks as-is or move follow-up to an issue, but please drop the inline TODOs.

🧹 Suggested cleanup

     assert isinstance(MathUtils.triple, promising.PromisingFunction)

-    # TODO Apply these elaborate checks to other tests as well, or get rid of
-    #  them ?
-    #  https://github.com/teremterem/Promising/pull/89#discussion_r3008493442
     class_result = MathUtils.triple(5)
     instance_result = MathUtils().triple(5)
     assert isinstance(class_result, promising.Promise)
     assert isinstance(instance_result, promising.Promise)
     assert await class_result == 15
@@
     assert isinstance(Factory.create_name.__func__, promising.PromisingFunction)

-    # TODO Apply these elaborate checks to other tests as well, or get rid of
-    #  them ?
-    #  https://github.com/teremterem/Promising/pull/89#discussion_r3008493442
     class_result = Factory.create_name()
     instance_result = Factory().create_name()
     assert isinstance(class_result, promising.Promise)
     assert isinstance(instance_result, promising.Promise)
     assert await class_result == "Factory"

Also applies to: 431-433

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/decoration/test_method_function_decorator_async.py` around lines 406 -
408, Remove the inline PR-thread TODO comments from the test body in
tests/decoration/test_method_function_decorator_async.py (the comment block
starting with "# TODO Apply these elaborate checks..." around the region that
also has a duplicate at lines ~431-433); either keep the current Promise-type
checks as-is or open a separate issue for follow-up, but delete these inline
TODO comment lines so the test suite is not coupled to the PR discussion.

tests/decoration/test_method_context_decorator_sync.py (2)

79-81: 🧹 Nitpick | 🔵 Trivial

Remove the PR-thread TODO from the committed test.

This leaves the assertion strategy undecided and ties the suite to a review discussion. Keep the assertion if you want the coverage, but drop the inline TODO block.

🧹 Suggested cleanup

     # Verify context is properly deactivated after exception
-    # TODO Apply this check to other tests as well ? Make it more elaborate ?
-    #  Get rid of it ?
-    #  https://github.com/teremterem/Promising/pull/89#discussion_r3008493421
     assert promising.get_active_context(raise_if_none=False) is None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/decoration/test_method_context_decorator_sync.py` around lines 79 - 81,
Remove the inline PR-thread TODO comment block (the three commented lines
starting with "# TODO Apply this check..." through the GitHub PR link) from the
committed test file so the test suite no longer contains review discussion;
retain the actual assertion(s) in the test if coverage is desired, but delete
the TODO lines and any duplicate comment to keep the test file clean.

---

6-6: 🧹 Nitpick | 🔵 Trivial

Use regular def for these sync-only tests.

Every test in this module is declared async def, but the bodies are fully synchronous. That adds unnecessary async-test/plugin coupling to a file whose purpose is to exercise sync decorators.

Run this to confirm there is no async-only syntax in the module before converting them mechanically:

#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path

path = Path("tests/decoration/test_method_context_decorator_sync.py")
tree = ast.parse(path.read_text())

for node in tree.body:
    if isinstance(node, ast.AsyncFunctionDef):
        has_async_syntax = any(
            isinstance(n, (ast.Await, ast.AsyncFor, ast.AsyncWith))
            for n in ast.walk(node)
        )
        print(f"Line {node.lineno}: {node.name} -> async-only-syntax={has_async_syntax}")
PY

If every row prints async-only-syntax=False, these can be switched to regular def tests without changing the test bodies.

Also applies to: 21-21, 45-45, 64-64, 85-85, 94-94, 111-111, 130-130, 147-147, 167-167, 185-185, 204-204, 220-220, 237-237, 255-255, 265-265

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/decoration/test_method_context_decorator_sync.py` at line 6, Convert
sync-only test coroutines in this module from "async def" to regular "def": for
each AsyncFunctionDef such as test_instance_method_activates_context, change its
signature from "async def ..." to "def ..." (and do the same for the other async
tests in this file) after first running the provided AST-check script to ensure
no async-only syntax (Await/AsyncFor/AsyncWith) is present; this keeps test
bodies unchanged and removes unnecessary async-test/plugin coupling while
preserving existing function names and behavior.

tests/decoration/test_method_context_decorator_async.py (1)

79-81: 🧹 Nitpick | 🔵 Trivial

Remove the PR-thread TODO from the committed test.

This leaves the assertion strategy undecided and couples the suite to a review discussion. Keep the assertion or track follow-up elsewhere, but please drop the inline TODO block.

🧹 Suggested cleanup

     # Verify context is properly deactivated after exception
-    # TODO Apply this check to other tests as well ? Make it more elaborate ?
-    #  Get rid of it ?
-    #  https://github.com/teremterem/Promising/pull/89#discussion_r3008493421
     assert promising.get_active_context(raise_if_none=False) is None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/decoration/test_method_context_decorator_async.py` around lines 79 -
81, Remove the PR-thread TODO comment block in
tests/decoration/test_method_context_decorator_async.py (the three-line TODO
starting with "# TODO Apply this check to other tests as well ?") so the
committed test no longer references a review discussion; either delete those
lines entirely or replace them with a short, concrete comment or an issue/PR
reference if you want to track follow-up (no inline discussion notes).