-
Notifications
You must be signed in to change notification settings - Fork 2k
tests: add lowest dependency tests #1067
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
Conversation
@@ -45,4 +45,7 @@ jobs: | |||
|
|||
- name: Run pytest | |||
run: uv run --frozen --no-sync pytest | |||
continue-on-error: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be set, should it be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's needed, but I figured out why it's here: so if other tests on the matrix fail, it will continue to run.
@@ -26,6 +26,7 @@ jobs: | |||
test: | |||
runs-on: ${{ matrix.os }} | |||
timeout-minutes: 10 | |||
continue-on-error: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put it up because it makes more sense since it's a job configuration, not a step one.
@@ -25,7 +25,7 @@ dependencies = [ | |||
"anyio>=4.5", | |||
"httpx>=0.27", | |||
"httpx-sse>=0.4", | |||
"pydantic>=2.7.2,<3.0.0", | |||
"pydantic>=2.8.0,<3.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2.7.2 resolves one of the tests wrong, and I found that pushing up 1 minor is not that damaging - pushing it to 2.11+ would be too much right now.
2.8.0 is from July 2024.
@@ -58,6 +58,7 @@ dev = [ | |||
"pytest-examples>=0.0.14", | |||
"pytest-pretty>=1.2.0", | |||
"inline-snapshot>=0.23.0", | |||
"dirty-equals>=0.9.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This goes with the same line as inline-snapshot
s. It's a utility package that helps a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessary - but it did make the test cleaner. I can remove it if requested.
|
||
|
||
def main(): | ||
anyio.run(test_messages_are_executed_concurrently) | ||
|
||
|
||
if __name__ == "__main__": | ||
import logging | ||
|
||
logging.basicConfig(level=logging.DEBUG) | ||
|
||
main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't have been merged - it seems it was here for debugging purposes.
tests/issues/test_188_concurrency.py
Outdated
@pytest.mark.filterwarnings( | ||
"ignore:coroutine 'test_messages_are_executed_concurrently.<locals>.slow_resource' was never awaited:RuntimeWarning" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find the reason or package that triggered this - I tried bumping anyio, but that didn't seem to be the reason - but this seems an issue only with the test suite, so it doesn't have any impact.
"title": "func_dict_anyDictOutput", | ||
} | ||
|
||
assert meta.output_schema == IsPartialDict(type="object", title="func_dict_anyDictOutput") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that the output_schema
has AT LEAST the keys I've added inside the IsPartialDict
.
@@ -43,7 +43,6 @@ ws = ["websockets>=15.0.1"] | |||
mcp = "mcp.cli:app [cli]" | |||
|
|||
[tool.uv] | |||
resolution = "lowest-direct" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, we actually want the resolution to be the "highest" - which is the default. Because we have a test job in the pipeline that sets this resolution.
This way we can have the highest resolution on the lockfile, and actually test both highest and lowest-direct.
8e569cb
to
45187fc
Compare
@@ -36,14 +36,13 @@ dependencies = [ | |||
|
|||
[project.optional-dependencies] | |||
rich = ["rich>=13.9.4"] | |||
cli = ["typer>=0.12.4", "python-dotenv>=1.0.0"] | |||
cli = ["typer>=0.16.0", "python-dotenv>=1.0.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to bump typer because click broke it recently.
I don't think bumping this package is an issue given that people can usually easily bump this without coding changes on their applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pydantic or starlette would be more concerning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
No description provided.