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

Parameterized parse verification #244

Merged
merged 6 commits into from
Sep 18, 2018
Merged

Conversation

mvanaken
Copy link
Contributor

@mvanaken mvanaken commented Aug 3, 2018

During development of #241 and #242 this addition helped me a lot to check if the tests were in fact fully parsed. Decided to create a separate pull request for it.

Only had to improve the until tests, since there were 4 tests, that didn't parse until the end. For these tests I decreased the size of the stream.

Also adjusted some other descriptions within the until tests to correctly represent what the tests do.

@mvanaken mvanaken requested review from jvdb, rdvdijk and ccreeten August 3, 2018 18:57
@codecov
Copy link

codecov bot commented Aug 3, 2018

Codecov Report

Merging #244 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #244   +/-   ##
=======================================
  Coverage       100%   100%           
  Complexity     1016   1016           
=======================================
  Files            90     90           
  Lines          1381   1381           
  Branches        141    141           
=======================================
  Hits           1381   1381

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f432c33...2d56d00. Read the comment docs.

Copy link
Contributor

@jvdb jvdb left a comment

Choose a reason for hiding this comment

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

It's a really good idea to add this check to the standard implementation of ParameterizedParse! The fact that only the Until-tests were affected shows that it's something that we should have added a long time ago (or even should have been part of the original implementation).

My only worry is that this change affects the safety net-function of some of the Until-tests, so we should either make sure that's not the case, or decide on how to proceed.

… stops at the first match. Introduced overload methods with extra argument to indicate what the expected tail is of the stream after parsing the until token.
@jvdb jvdb merged commit d635955 into master Sep 18, 2018
@jvdb jvdb deleted the parameterized-parse-verification branch September 18, 2018 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants