-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add pytest-examples and make examples in docs testable #56
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
=======================================
Coverage 95.37% 95.37%
=======================================
Files 5 5
Lines 303 303
Branches 69 69
=======================================
Hits 289 289
Misses 12 12
Partials 2 2
☔ View full report in Codecov by Sentry. |
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 see the test code using pytest-examples - I guess you need to add that file?
docs/index.md
Outdated
auth_key: str = Field('', validation_alias='my_auth_key') | ||
api_key: str = Field('', validation_alias='my_api_key') |
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 think the idea here was that these fields were required - then set via env vars. Probably best if you can set the appropriate env vars in the test so this works without modifiation.
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.
Done
docs/index.md
Outdated
# Set environment variables | ||
os.environ['V0'] = '0' | ||
os.environ['SUB_MODEL'] = '{"v1": "json-1", "v2": "json-2"}' | ||
os.environ['SUB_MODEL__V2'] = 'nested-2' | ||
os.environ['SUB_MODEL__V3'] = '3' | ||
os.environ['SUB_MODEL__DEEP__V4'] = 'v4' |
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.
maybe you can see these in the test?
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.
Sorry, didn't get this comment. Do you mean to convert them to a test function here?
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.
you can move all this logic into test setup.
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.
e.g. set a bunch of env variables before running all or specific tests, then remove them after.
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've added a fixture
docs/index.md
Outdated
|
||
print(Settings().model_dump()) | ||
|
||
pprint(Settings().model_dump()) |
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 think you should just use print
, pytest-examples should take care of formatting the output with black
.
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.
Didn't work with print
.
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― test_docs_examples[docs/index.md:14-72] ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Print output changed code:
--- before
+++ after
@@ -56,30 +56,21 @@
model_config = ConfigDict(env_prefix='my_prefix_') # defaults to no prefix, i.e. ""
print(Settings().model_dump())
"""
-{
- 'auth_key': '',
- 'api_key': '',
- 'redis_dsn': Url('redis://user:pass@localhost:6379/1'),
- 'pg_dsn': Url('postgres://user:pass@localhost:5432/foobar'),
- 'amqp_dsn': Url('amqp://user:pass@localhost:5672/'),
- 'special_function': <built-in function cos>,
- 'domains': set(),
- 'more_settings': {'foo': 'bar', 'apple': 1},
-}
+{'auth_key': '', 'api_key': '', 'redis_dsn': Url('redis://user:pass@localhost:6379/1'), 'pg_dsn': Url('postgres://user:pass@localhost:5432/foobar'), 'amqp_dsn': Url('amqp://user:pass@localhost:5672/'), 'special_function': <built-in function cos>, 'domains': set(), 'more_settings': {'foo': 'bar', 'apple': 1}}
"""
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.
Ok, I've replaced it with print
but it doesn't work with formatted output.
we can ignore it for now.
4b35d9f
to
926059c
Compare
e3b3e50
to
553dce5
Compare
553dce5
to
b9348bd
Compare
45cdc87
to
cfec546
Compare
6dfb0c8
to
7ad45ef
Compare
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.
otherwise LGTM, I committed to fix the issue with formatting the first example.
docs/index.md
Outdated
# Set environment variables | ||
os.environ['V0'] = '0' | ||
os.environ['SUB_MODEL'] = '{"v1": "json-1", "v2": "json-2"}' | ||
os.environ['SUB_MODEL__V2'] = 'nested-2' | ||
os.environ['SUB_MODEL__V3'] = '3' | ||
os.environ['SUB_MODEL__DEEP__V4'] = 'v4' |
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.
you can move all this logic into test setup.
docs/index.md
Outdated
# Set environment variables | ||
os.environ['V0'] = '0' | ||
os.environ['SUB_MODEL'] = '{"v1": "json-1", "v2": "json-2"}' | ||
os.environ['SUB_MODEL__V2'] = 'nested-2' | ||
os.environ['SUB_MODEL__V3'] = '3' | ||
os.environ['SUB_MODEL__DEEP__V4'] = 'v4' |
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.
e.g. set a bunch of env variables before running all or specific tests, then remove them after.
@samuelcolvin It's ready |
Thanks so much. |
Fixes #33