Add async method mock #148

MetalHurlant · 2023-11-01T20:36:03Z

No description provided.

MetalHurlant · 2023-11-01T20:36:38Z

Not sure this is the best way to do it. please let me know if you have comments

ollipa · 2023-11-03T09:04:43Z

@MetalHurlant, thanks for the PR! Some comments:

I think the sync_ and async_ methods with the underscore don't fit so well in the API. How about calling them make_sync and make_async?
Dependencies are probably missing pytest-asyncio so tests in CI are failing.
Test coverage should be increased to test that everything works with different types of methods and spying too. You can see some examples in my earlier attempt to add async support: 626ca53

Anyway, this looks like a simple an pragmatic way to add async support to flexmock 👍

@christophe-riolo, @adarshk7 what do you think?

MetalHurlant · 2023-11-03T11:55:30Z

Thanks for the comments. I will update the PR.

tests/features/mocking.py

christophe-riolo · 2023-11-09T08:31:07Z

I had a quick glance it looks like a nice change, I'll look more in detail asap !

christophe-riolo

Thank you very much for your contribution 🙏
I have a couple of questions and suggestions, but overall I like the way you have designed this addition 👍

src/flexmock/_api.py

christophe-riolo · 2023-11-13T10:43:18Z

src/flexmock/_api.py

@@ -778,6 +797,14 @@ def _match_args(self, given_args: Any) -> bool:
                return False
        return True

+    def _verify_not_async_spy(self):
+        """Check if trying to assert the output of an async call."""
+        is_spy = self._replace_with is self.__dict__.get("_original")


I am thinking, this could potentially be extracted in a property, maybe outside of this PR what do you think @ollipa ?

src/flexmock/_api.py

christophe-riolo · 2023-11-13T11:19:25Z

src/flexmock/_api.py

@@ -872,6 +899,7 @@ def and_return(self, *values: Any) -> "Expectation":
            >>> plane.passenger_count()
            3
        """
+        self._verify_not_async_spy()


Do we really need to forbid this ? It feels like we could check the result of the spied function in an asynchronous way. It could require more work that could be on us @ollipa

We could forbid it now and allow it in the future when we make an implementation for it.

As olippa said. For now it would not work properly so I preferred raising a clear error.

Yes, that's what I was suggesting, but so we could create an issue for that 👍

tests/features/mocking.py

tests/features/mocking_async.py

codecov-commenter · 2023-12-03T08:22:20Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (58a2eb8) 100.00% compared to head (0757ea1) 99.53%.
Report is 2 commits behind head on master.

Files	Patch %	Lines
src/flexmock/_api.py	86.20%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##            master     #148      +/-   ##
===========================================
- Coverage   100.00%   99.53%   -0.47%     
===========================================
  Files            4        4              
  Lines          834      863      +29     
===========================================
+ Hits           834      859      +25     
- Misses           0        4       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ollipa

I made a few comments comments but overall looks very good!

ollipa · 2023-12-03T08:23:47Z

.github/workflows/ci.yml

            "pypy-3.8",
            "pypy-3.9",
-            "3.7",


It seems that Pypy has released 3.10 version. You can include that here.

ollipa · 2023-12-03T08:24:25Z

.github/workflows/ci.yml

@@ -57,7 +55,7 @@ jobs:
      - name: Set up Python
        uses: actions/setup-python@v4
        with:
-          python-version: "3.7"
+          python-version: "3.11"


The tests are meant to be executed with the lowest Python version we support. Otherwise

Suggested change

python-version: "3.11"

python-version: "3.8"

ollipa · 2023-12-03T08:24:35Z

.github/workflows/ci.yml

@@ -89,7 +87,7 @@ jobs:
      - name: Set up Python
        uses: actions/setup-python@v4
        with:
-          python-version: "3.7"
+          python-version: "3.11"


Suggested change

python-version: "3.11"

python-version: "3.8"

ollipa · 2023-12-03T08:24:50Z

.github/workflows/coverage.yml

@@ -21,7 +21,7 @@ jobs:
      - name: Set up Python
        uses: actions/setup-python@v4
        with:
-          python-version: "3.7"
+          python-version: "3.11"


Suggested change

python-version: "3.11"

python-version: "3.8"

ollipa · 2023-12-03T08:25:13Z

docs/index.md

 - Python 3.8
 - Python 3.9
 - Python 3.10
+- Python 3.11


You can also add Python 3.12 here.

ollipa · 2023-12-03T08:36:27Z

src/flexmock/_api.py

@@ -28,6 +28,24 @@
 RE_TYPE = type(re.compile(""))


+async def future_raise(anything: type[BaseException]) -> None:


Using type directly requires Python 3.9.

Suggested change

async def future_raise(anything: type[BaseException]) -> None:

async def future_raise(anything: Type[BaseException]) -> None:

ollipa · 2023-12-03T08:39:00Z

src/flexmock/_api.py

@@ -28,6 +28,24 @@
 RE_TYPE = type(re.compile(""))


+async def future_raise(anything: type[BaseException]) -> None:


I guess we don't need these functions to be part of the public API?

Suggested change

async def future_raise(anything: type[BaseException]) -> None:

async def _future_raise(anything: type[BaseException]) -> None:

ollipa · 2023-12-03T08:39:10Z

src/flexmock/_api.py

+    raise anything
+
+
+async def future(anything: Any) -> Any:


Suggested change

async def future(anything: Any) -> Any:

async def _future(anything: Any) -> Any:

ollipa · 2023-12-03T08:44:21Z

src/flexmock/_api.py

+        """Set the return values of the expectation to coroutines
+        Need to be set before the return value is set


Suggested change

"""Set the return values of the expectation to coroutines

Need to be set before the return value is set

"""Set the return value to be a coroutine.

Needs to be set before the return value is set.

ollipa · 2023-12-03T08:47:03Z

src/flexmock/_api.py

+
+    def make_sync(self) -> "Expectation":
+        """Make the mocked method synchronous.
+        Need to be set before the return value is set


Suggested change

Need to be set before the return value is set

Needs to be set before the return value is set.

ollipa · 2023-12-03T08:54:43Z

@MetalHurlant, if you rebase your changes you can remove docs/requirements.txt. That file is not needed anymore.

MetalHurlant force-pushed the async_method_mock branch from 55f1d0a to d101207 Compare November 1, 2023 22:12

Add async method mock

1adcccd

MetalHurlant force-pushed the async_method_mock branch from d101207 to 1adcccd Compare November 2, 2023 04:17

Fix multiple call and extend tests

55fb361

MetalHurlant commented Nov 3, 2023

View reviewed changes

tests/features/mocking.py Outdated Show resolved Hide resolved

Fix multiple call and extend tests

4e126e9

christophe-riolo reviewed Nov 13, 2023

View reviewed changes

tests/features/mocking_async.py Show resolved Hide resolved

christophe-riolo reviewed Nov 13, 2023

View reviewed changes

tests/features/mocking_async.py Outdated Show resolved Hide resolved

Address some comments

0757ea1

ollipa reviewed Dec 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add async method mock #148

Add async method mock #148

MetalHurlant commented Nov 1, 2023

MetalHurlant commented Nov 1, 2023

ollipa commented Nov 3, 2023

MetalHurlant commented Nov 3, 2023

christophe-riolo commented Nov 9, 2023

christophe-riolo left a comment

christophe-riolo Nov 13, 2023

christophe-riolo Nov 13, 2023

ollipa Nov 19, 2023

MetalHurlant Nov 19, 2023

christophe-riolo Nov 20, 2023

codecov-commenter commented Dec 3, 2023

ollipa left a comment

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa Dec 3, 2023

ollipa commented Dec 3, 2023

		@@ -28,6 +28,24 @@
		RE_TYPE = type(re.compile(""))


		async def future_raise(anything: type[BaseException]) -> None:

	async def future_raise(anything: type[BaseException]) -> None:
	async def _future_raise(anything: type[BaseException]) -> None:

	async def future(anything: Any) -> Any:
	async def _future(anything: Any) -> Any:

		"""Set the return values of the expectation to coroutines
		Need to be set before the return value is set

	Need to be set before the return value is set
	Needs to be set before the return value is set.

Add async method mock #148

Are you sure you want to change the base?

Add async method mock #148

Conversation

MetalHurlant commented Nov 1, 2023

MetalHurlant commented Nov 1, 2023

ollipa commented Nov 3, 2023

MetalHurlant commented Nov 3, 2023

christophe-riolo commented Nov 9, 2023

christophe-riolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 3, 2023

Codecov Report

ollipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ollipa commented Dec 3, 2023