Skip to content

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

Merged
Zac-HD merged 1 commit intoHypothesisWorks:masterfrom
Zac-HD:fix-explain-phase
Oct 1, 2023
Merged

Remove explain-phase assertion which is in fact reachable#3758
Zac-HD merged 1 commit intoHypothesisWorks:masterfrom
Zac-HD:fix-explain-phase

Conversation

@Zac-HD
Copy link
Copy Markdown
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
@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