Skip to content

Conversation

Liam-DeVoe
Copy link
Member

@Liam-DeVoe Liam-DeVoe commented Sep 7, 2025

(pending confirmation) closes #4442. Thanks to @pschanely and especially @jobh for getting to the bottom of this!

Comment on lines +310 to +318
@contextmanager
def with_register_backend(name, provider_cls):
try:
AVAILABLE_PROVIDERS[name] = provider_cls
yield
finally:
del AVAILABLE_PROVIDERS[name]


Copy link
Member Author

Choose a reason for hiding this comment

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

I'm naming this with the assumption that we'll migrate AVAILABLE_PROVIDERS to def register_backend(name, class_or_string) at some point

@jobh
Copy link
Contributor

jobh commented Sep 7, 2025

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.

(probably?) remove the cache rebuild added in #4518

Yes, most definitely! If it starts happening again we want to know.

@jobh
Copy link
Contributor

jobh commented Sep 7, 2025

The ubuntu runner got a probably unrelated error after ~20 repeats, https://github.com/HypothesisWorks/hypothesis/actions/runs/17525804945/job/49776013140

FAILED hypothesis-python/tests/crosshair/test_crosshair.py::test_crosshair_works_for_all_verbosities[3-21-50] - Failed: DID NOT RAISE <class 'AssertionError'>

I'll switch to windows runner and remove -x to complete.

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?

Copy link
Contributor

@jobh jobh left a comment

Choose a reason for hiding this comment

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

@Liam-DeVoe
Copy link
Member Author

The ubuntu runner got a probably unrelated error after ~20 repeats, https://github.com/HypothesisWorks/hypothesis/actions/runs/17525804945/job/49776013140

Agree this is unrelated - crosshair hitting timeouts on a particularly slow CI run, perhaps.

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?

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 check-crosshair-custom jobs on linux? (I don't know our windows CI particularly well)

@jobh
Copy link
Contributor

jobh commented Sep 8, 2025

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?

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 check-crosshair-custom jobs on linux? (I don't know our windows CI particularly well)

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 tests/crosshair is run anywhere else currently (as opposed to ~everything else under the crosshair profile)?

[edit: Sample toxified build matrix for windows+osx at the repurposed #4510)

@Liam-DeVoe
Copy link
Member Author

But: from a quick glance I don't see that tests/crosshair is run anywhere else currently (as opposed to ~everything else under the crosshair profile)?

Right, this seems to be the biggest problem. Probably we should add a new linux CI entry to test /tests/crosshair and any other missed test dirs that were previously windows-only? (or perhaps that's what you were proposing). Regardless it does seem good to move more things to tox.

@jobh
Copy link
Contributor

jobh commented Sep 9, 2025

But: from a quick glance I don't see that tests/crosshair is run anywhere else currently (as opposed to ~everything else under the crosshair profile)?

Right, this seems to be the biggest problem. Probably we should add a new linux CI entry to test /tests/crosshair and any other missed test dirs that were previously windows-only? (or perhaps that's what you were proposing). Regardless it does seem good to move more things to tox.

Yes. Technically, something like tests/numpy was also needed to create the prerequisite cache pressure, but that's going into the weeds. I created a new issue to follow up.

Sign up for free to 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.

Identical objects sometimes hashing differently during crosshair tests
2 participants