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

Fixed failing tests due to range error #170

Merged
merged 1 commit into from
Oct 13, 2021
Merged

Fixed failing tests due to range error #170

merged 1 commit into from
Oct 13, 2021

Conversation

bmuddha
Copy link
Contributor

@bmuddha bmuddha commented Oct 12, 2021

The issue was that helper account method generated AccountData
values with len() ranging from 1, in case if len turned out to be 1,
then valid_ranges couldn't generate range, since it also tried to
build range starting from value 1 and ending with whatever is the len() of
AccountData, 1..1 in case which triggered error.

As a solution, the minimum AccountData length was set to 2, since
there's no logic requirements for it to start from 1.

Closes #159

@polachok polachok requested a review from 00nktk October 12, 2021 19:01
Copy link
Collaborator

@00nktk 00nktk left a comment

Choose a reason for hiding this comment

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

Yeah, that fixes the issue, but we still want to test 1 byte accounts since that can be a tricky corner case.
I believe we can filter out empty ranges the same way it's done for offsets (see fn offset on 43, it returns one-item strategy for a 1 byte account).

I think the best way is to refactor offsets function so we can use it to generate strategies for both offsets and slice lengths.

The issue was that helper `account` method generated `AccountData`
values with `len()` ranging from 1, in case if len turned out to be 1,
then `valid_ranges` couldn't generate range, since it also tried to
build range starting from value 1 and ending with whatever is the `len()` of
`AccountData`, 1..1 in case which triggered error.

As a solution, the minimum `AccountData` length was set to 2, since
there's no logic requirements for it to start from 1.

Reworked offsets function in tests to support AccounData of len < 2
@bmuddha
Copy link
Contributor Author

bmuddha commented Oct 13, 2021

I think the best way is to refactor offsets function so we can use it to generate strategies for both offsets and slice lengths.

I've rewrote the function to generate both offsets and slice length, with special handling of AccountData::len() being 1, will that do?

@polachok polachok merged commit c60de02 into bestarch-ae:master Oct 13, 2021
@bmuddha bmuddha deleted the fix-test branch October 14, 2021 15:31
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.

filter::tests::tree_matches_overall can randomly fail
3 participants