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

Fixes #282 #283

Merged
merged 5 commits into from
Jul 8, 2019
Merged

Fixes #282 #283

merged 5 commits into from
Jul 8, 2019

Conversation

wavebeem
Copy link
Collaborator

@wavebeem wavebeem commented Jul 7, 2019

No description provided.

@coveralls
Copy link

coveralls commented Jul 7, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 5fa8882 on issues/282 into f71a738 on master.

@wavebeem wavebeem changed the title [WIP] try to test headless chrome try to test headless chrome Jul 8, 2019
@wavebeem wavebeem marked this pull request as ready for review July 8, 2019 00:45
@wavebeem wavebeem changed the title try to test headless chrome Fixes #282 Jul 8, 2019
@wavebeem wavebeem merged commit a8b67d1 into master Jul 8, 2019
@wavebeem wavebeem deleted the issues/282 branch July 8, 2019 01:00
@jneen
Copy link
Owner

jneen commented Jul 8, 2019

Can you point me to where the actual fix is? All I see in the diff are test changes.

@wavebeem
Copy link
Collaborator Author

wavebeem commented Jul 8, 2019

Ah yeah, it was just this line: https://github.com/jneen/parsimmon/pull/283/files#diff-d6843052e5b691e32d17eb1fb2508987L216

 function parseBufferFor(other, length) {
-  ensureBuffer();
   return new Parsimmon(function(input, i) {
+    ensureBuffer();
     if (i + length > input.length) {
       return makeFailure(i, length + " bytes for " + other);
     }

I'm waiting until the parse actually happens to report the error. This is needed because this was getting called by the setup of the int8LE and whatnot.

And yeah when I ws getting Karma working Mocha started getting mad that the tests were using mixed TDD/BDD syntax

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.

3 participants