-
Notifications
You must be signed in to change notification settings - Fork 589
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
return_value and not_a_test_method health checks strict error fix #3581
Conversation
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.
Comments below; you'll also need to add your name to AUTHORS.rst
and create a hypothesis-python/RELEASE.rst
file - see recent PRs for examples. Finally, we'll want to test that accessing the deprecated attributes... emits a HypothesisDeprecationWarning, which you can do with our @checks_deprecated_behaviour
decorator.
OK... this is looking like a bit of a tarpit, because |
OK, let's give up on deprecating the attribute access - that would have been nicer, but turns out to be way harder than expected. Instead, we'll |
So I got |
Ugh, maybe we should monkeypatch? something like class HealthCheck: ...
HealthCheck.__iter__ = lambda: iter(HealthCheck.all()) |
This looks good besides a few (unrelated?) tests. Let me know if there's any issue with adding in a metaclass to do this as well |
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.
Looks great! I think we're done here, with the only remaining blocker that infuriating test hang 😢
@classmethod | ||
def all(cls) -> List["HealthCheck"]: | ||
# Skipping of deprecated attributes is handled in HealthCheckMeta.__iter__ | ||
# TODO: note_deprecation() and write a codemod for HC.all() -> list(HC) | ||
return list(HealthCheck) |
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.
Want to take this on as a follow-up PR?
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.
Sure! I can work on this after the cacheing :)
This addresses parts of issue #3568.
Currently, some of the tests fail because (I believe)
note_deprecation
causes an undefined error with Pytest before their expectedFailedHealthCheck
message in tests that explicitly test forreturn_value
(andnot_a_test_method
). An example of this is thetest_stateful_returnvalue_healthcheck
test.Example below
Edit: I can solve the majority of tests above by adding in a catch_warning, though I'm not sure whether or not editing the tests themselves would be advised