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

Fix issue where entire (very long) input is passed to bitSeq. #232

Merged
merged 2 commits into from
Mar 22, 2018

Conversation

theqabalist
Copy link
Collaborator

So I need help writing a test that actually creates the problem I have fixed. In the binary parser I have constructed, when the parser runs, I am getting the entire input to bitSeq, when it's supposed to just operate on a slice of the input (when embedded as part of a large parsing context). I have fixed this issue, but I cannot seem to replicate it in a small test to prove it's an issue.

Can someone help me construct a proper test that will fail without this change?

@coveralls
Copy link

coveralls commented Mar 21, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 8eb1966 on theqabalist:fix/bit-seq-wrong into c1e28fb on jneen:master.

@theqabalist
Copy link
Collaborator Author

theqabalist commented Mar 22, 2018

I was actually just being [redacted] yesterday and didn't think through my input buffer properly when constructing the test. I have a properly failing test case now to prove the problem.

@wavebeem
Copy link
Collaborator

Looks good. Glad you were able to figure it out. I didn't have a chance to look at it yet but it all looks sorted now.

(p.s. please consider not using exclusive language next time)

@wavebeem wavebeem merged commit 1821f52 into jneen:master Mar 22, 2018
@wavebeem
Copy link
Collaborator

I forgot to mention that I released this as 1.7.1 shortly after merging the PR

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.

None yet

3 participants