Skip to content

tests: use inline_snapshot.Is on parametrized test - #945

Merged
ihrpr merged 5 commits into
mainfrom
fix-test-with-snapshot
Jul 14, 2025
Merged

tests: use inline_snapshot.Is on parametrized test#945
ihrpr merged 5 commits into
mainfrom
fix-test-with-snapshot

Conversation

@Kludex

@KludexKludex commented Jun 12, 2025

Copy link
Copy Markdown
Member

cc @agronholm

This PR just uses inline_snapshots properly on a parametrized test.

@KludexKludex mentioned this pull request Jun 12, 2025
9 tasks
@agronholm

Copy link
Copy Markdown

Looks like the tests are still failing though?

@agronholm

Copy link
Copy Markdown

That one failure looks like just a flaky test.

@Kludex

Kludex commented Jun 12, 2025

Copy link
Copy Markdown
MemberAuthor

Now the test is fixed... But it's failing correctly... 😅

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

@Viicos

Copy link
Copy Markdown
Contributor

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

In progress in pydantic/pydantic-core#1719, but anyway this will only be included in 2.12.

@agronholm

Copy link
Copy Markdown

Now the test is fixed... But it's failing correctly... 😅

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

@Kludex

Copy link
Copy Markdown
MemberAuthor

Now the test is fixed... But it's failing correctly... 😅
The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

Are you saying this is working as expected?

@agronholm

Copy link
Copy Markdown

Now the test is fixed... But it's failing correctly... 😅
The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

Are you saying this is working as expected?

I have no idea how the test is supposed to work, TBH. I just observed a lot of redirects happening in the test suite when the forward slash is missing from the end of the URI.

@Kludex

Copy link
Copy Markdown
MemberAuthor

It's a Starlette thing. There's another PR that fixes the redirect behavior.

@Kludex
Kludex requested a review from ihrprJune 13, 2025 08:38
@medaminezghal

Copy link
Copy Markdown
Contributor

@Kludex With pytest 8.4.0 I got this problem:

_________________________ ERROR at setup of TestOAuthClientProvider.test_generate_code_verifier __________________________
[gw4] linux -- Python 3.13.3 /home/medaminezghal/Documents/AUR_Packages/python-mcp/test/src/mcp-1.9.4/test-env/bin/python
cls = <class '_pytest.runner.CallInfo'>, func = <function call_and_report.<locals>.<lambda> at 0x7f3b3f42cf40>
when = 'setup', reraise = (<class '_pytest.outcomes.Exit'>, <class 'KeyboardInterrupt'>)
@classmethod
def from_call(
cls,
func: Callable[[], TResult],
when: Literal["collect", "setup", "call", "teardown"],
reraise: type[BaseException] | tuple[type[BaseException], ...] | None = None,
) -> CallInfo[TResult]:
"""Call func, wrapping the result in a CallInfo.
:param func:
The function to call. Called without arguments.
:type func: Callable[[], _pytest.runner.TResult]
:param when:
The phase in which the function is called.
:param reraise:
Exception or exceptions that shall propagate if raised by the
function, instead of being wrapped in the CallInfo.
"""
excinfo = None
instant = timing.Instant()
try:
> result: TResult | None = func()
^^^^^^
/usr/lib/python3.13/site-packages/_pytest/runner.py:344: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ /usr/lib/python3.13/site-packages/_pytest/runner.py:246: in <lambda>
lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_hooks.py:512: in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_manager.py:120: in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/logging.py:843: in pytest_runtest_setup
yield
/usr/lib/python3.13/site-packages/_pytest/capture.py:895: in pytest_runtest_setup
return (yield)
^^^^^
/usr/lib/python3.13/site-packages/_pytest/runner.py:164: in pytest_runtest_setup
item.session._setupstate.setup(item)
/usr/lib/python3.13/site-packages/_pytest/runner.py:514: in setup
col.setup()
/usr/lib/python3.13/site-packages/_pytest/python.py:1673: in setup
self._request._fillfixtures()
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:719: in _fillfixtures
item.funcargs[argname] = self.getfixturevalue(argname)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:548: in getfixturevalue
fixturedef = self._get_active_fixturedef(argname)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:639: in _get_active_fixturedef
fixturedef.execute(request=subrequest)
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:1127: in execute
result = ihook.pytest_fixture_setup(fixturedef=self, request=request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_hooks.py:512: in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_manager.py:120: in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:53: in run_old_style_hookwrapper
return result.get_result()
^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:38: in run_old_style_hookwrapper
res = yield
^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:53: in run_old_style_hookwrapper
return result.get_result()
^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:38: in run_old_style_hookwrapper
res = yield
^^^^^
/usr/lib/python3.13/site-packages/_pytest/setuponly.py:36: in pytest_fixture_setup
return (yield)
^^^^^
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ fixturedef = <FixtureDef argname='oauth_provider' scope='function' baseid='tests/client/test_auth.py'>
request = <SubRequest 'oauth_provider' for <Function test_generate_code_verifier>>
def pytest_fixture_setup(
fixturedef: FixtureDef[FixtureValue], request: SubRequest
) -> FixtureValue:
"""Execution of fixture setup."""
kwargs = {}
for argname in fixturedef.argnames:
kwargs[argname] = request.getfixturevalue(argname)
fixturefunc = resolve_fixture_function(fixturedef, request)
my_cache_key = fixturedef.cache_key(request)
if inspect.isasyncgenfunction(fixturefunc) or inspect.iscoroutinefunction(
fixturefunc
):
auto_str = " with autouse=True" if fixturedef._autouse else ""
> warnings.warn(
PytestRemovedIn9Warning(
f"{request.node.name!r} requested an async fixture "
f"{request.fixturename!r}{auto_str}, with no plugin or hook that "
"handled it. This is usually an error, as pytest does not natively "
"support it. "
"This will turn into an error in pytest 9.\n"
"See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture"
),
# no stacklevel will point at users code, so we just point here
stacklevel=1,
)
E pytest.PytestRemovedIn9Warning: 'test_generate_code_verifier' requested an async fixture 'oauth_provider', with no plugin or hook that handled it. This is usually an error, as pytest does not natively support it. This will turn into an error in pytest 9.
E See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:1181: PytestRemovedIn9Warning

I think it's better to edit it to make it compatible with newer pytest versions.

@dsp-ant
dsp-ant self-requested a review June 17, 2025 09:47
@agronholm

agronholm commented Jun 23, 2025

Copy link
Copy Markdown

@medaminezghal my Trio PR (#946) fixes that too. Right now this PR is the only thing in this code base standing between me and a fully passing test suite.

@agronholmagronholm mentioned this pull request Jun 23, 2025
9 tasks
@agronholm

Copy link
Copy Markdown

That SSE failure could be due to improperly nested async context managers (typically a stream closed before the task using it has finished)

Comment threadtests/client/test_auth.py Outdated
felixweinberger
felixweinberger previously approved these changes Jul 14, 2025

@felixweinbergerfelixweinberger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to unblock #945 - added a proposal making the test an xfail on top of this branch in #1138 if you want to take a look @Kludex?

@Kludex

Copy link
Copy Markdown
MemberAuthor

Looks good.

felixweinberger
felixweinberger previously approved these changes Jul 14, 2025
@felixweinberger

Copy link
Copy Markdown
Contributor

took the liberty of rebasing - looks like test_auth has changed significantly since this PR

@ihrpr
ihrpr merged commit 95b44fb into mainJul 14, 2025
13 checks passed
@ihrpr
ihrpr deleted the fix-test-with-snapshot branch July 14, 2025 20:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Kludex@agronholm@Viicos@medaminezghal@felixweinberger@ihrpr