Copy avoid_realization in run_conformance_test
#4536
Conversation
| @contextmanager | ||
| def with_register_backend(name, provider_cls): | ||
| try: | ||
| AVAILABLE_PROVIDERS[name] = provider_cls | ||
| yield | ||
| finally: | ||
| del AVAILABLE_PROVIDERS[name] | ||
|
|
||
|
|
There was a problem hiding this comment.
I'm naming this with the assumption that we'll migrate AVAILABLE_PROVIDERS to def register_backend(name, class_or_string) at some point
|
Nice! I don't have an informed opinion on the code changes, but I'll kick off a 50-repeat run of the reproducer on both windows and ubuntu. It usually failed at about 5 repeats, so if those pass I'll consider it solved.
Yes, most definitely! If it starts happening again we want to know. |
|
The ubuntu runner got a probably unrelated error after ~20 repeats, https://github.com/HypothesisWorks/hypothesis/actions/runs/17525804945/job/49776013140 I'll switch to windows runner and remove Aside: Consider migrating the currently windows-specific CI job configuration to tox and switch f.x. one of the older python job to run the same config? |
Agree this is unrelated - crosshair hitting timeouts on a particularly slow CI run, perhaps.
I don't follow, what's the benefit of this? Is there some crosshair test or config which we're only running on windows, despite the |
The benefit for this issue (in hindsight) would have been to quickly identify the trigger as being the combination of tests rather than the platform. That may never happen again of course, but the windows runner differs also in dependency versions. Duplicating the setup to another platform would make any error that happens only on windows more likely to be windows-specific. But: from a quick glance I don't see that [edit: Sample toxified build matrix for windows+osx at the repurposed #4510) |
Right, this seems to be the biggest problem. Probably we should add a new linux CI entry to test |
Yes. Technically, something like |
|
@Zac-HD just a quick check you're ok with the api of allowing provider class values in |
I'd prefer to keep it string-only, to encourage designs which only pay for importing the backend if that backend is actually used. (we do this for our Pytest plugin, for example) But honestly writing a paragraph or two of docs about why and how to do that probably matters more, so if you want to go with docs + class support I won't object. |
|
I already wrote such a paragraph yep, encouraging string by default |
| #: If you maintain a third-party backend, we strongly recommend providing an | ||
| #: absolute importable path, rather than a |PrimitiveProvider| class. Hypothesis | ||
| #: will defer importing class paths until required, which improves startup times | ||
| #: for tests which don't request the backend. |
There was a problem hiding this comment.
I already wrote such a paragraph yep, encouraging string by default
Nice! I think it's worth expanding on this though: "We strongly encourage plugin authors to ensure that import hypothesis does not automatically import your package, e.g. by registering a standalone _mypkg_hypothesis_plugin module rather than mypgk._hypothesis_plugin."
There was a problem hiding this comment.
I think the plugin example is confusing - it can be in a subdir as long as you're careful about imports - so I just said as much more explicitly
|
Finally dug into why the crosshair tests have been failing on this and other PRs, and it's because of our new CI detection. The crosshair profile previously inherited from the |
|
Looks like this needs a codeowner approval @Zac-HD |
(pending confirmation)closes #4442. Thanks to @pschanely and especially @jobh for getting to the bottom of this!