Skip to content
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

Remove explain-phase assertion which is in fact reachable #3758

Merged
merged 1 commit into from
Oct 1, 2023

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Oct 1, 2023

Fixes #3755, by deleting some defensive code which I wrote to defend against implementation bugs and thought was unreachable in normal use. Obviously that turned out to be wrong! So, how can you reach that?

The relevant loop is replacing an infix of our test buffer, for example taking AA XX BB and trying AA YY BB. However, the length can vary by contents, so the test might try to read off the end of the buffer: AA YYB B-. In this case, we'll try fixing it up as AA YYB BB - and my incorrect assert was that we'd have exactly this buffer from the rerun. However, it's possible for the infix to first overflow, and then have part of the infix dropped by the canonicalisation logic (e.g. because of a filter) - so the final buffer is actually (e.g.) AA YB BB. This is in fact working as intended, so deleting the oversensitive check is a complete solution 😅

@Zac-HD Zac-HD added the bug something is clearly wrong here label Oct 1, 2023
@Zac-HD Zac-HD requested a review from DRMacIver as a code owner October 1, 2023 05:24
@Zac-HD Zac-HD merged commit ffda79f into HypothesisWorks:master Oct 1, 2023
46 checks passed
@Zac-HD Zac-HD deleted the fix-explain-phase branch October 1, 2023 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is clearly wrong here
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NotImplementedError: This should never happen in a test chaining st.datetimes()
1 participant