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

Streaming validation #146

Merged
merged 72 commits into from
Oct 6, 2015
Merged

Streaming validation #146

merged 72 commits into from
Oct 6, 2015

Conversation

pezholio
Copy link
Contributor

Opening a PR for @quadrophobiac's work

@quadrophobiac
Copy link
Contributor

The streaming validator is typified by the specs added in context "with a single row"and context context "validation with multiple lines: "
Both of these spec contexts test various ways that the Validator_Stream object can be initialised and then it's various methods can be used to validate a CSV
the def validate method has been preserved in the interest of creating a validator that would be backwards compatible with the old code base, see line 43
Alternatively as the specs demonstrate each of the validate_metadata report_line_breaks and parse_contents methods can be invoked independently to generate errors and warnings
Both parse_contents and validate_metadata can be invoked with enumerable behaviour which was the aim with a method that could take a CSV line by line and report errors
see streaming_validator_spec.rb:130 and streaming_validator_spec.rb:157

@kwent
Copy link

kwent commented Sep 29, 2015

Pretty awesome ! It this included this feature ? #143

@pezholio
Copy link
Contributor Author

Yup, that's the plan!

@kwent
Copy link

kwent commented Sep 30, 2015

@pezholio Awesome ! Do we have an ETA of when this will be merged in master and the gem version updated ?

@pezholio
Copy link
Contributor Author

I'm working on it as we speak. Hoping to get it done for the end of the week 👍

@kwent
Copy link

kwent commented Sep 30, 2015

@pezholio Awesome ! Can't wait.

@quadrophobiac
Copy link
Contributor

These tests are now failing on a single feature features/validation_errors.feature:126 - error of :stray_quote returned instead of :line_breaks
This hinges on logic

      if !@csv_options[:row_sep].kind_of?(Symbol) && type == :unclosed_quote && !@input.match(@csv_options[:row_sep])
        build_linebreak_error

within validate.rb https://github.com/theodi/csvlint.rb/blob/19dc4244dabd90aa4789f08be42d3edd59546668/lib/csvlint/validate.rb#L290

In the case of the feature this fails because type is reported as unclosed_quote

changes to this logic (to recognise :stray_quote) will adversely affect https://github.com/theodi/csvlint.rb/blob/19dc4244dabd90aa4789f08be42d3edd59546668/spec/validator_spec.rb#L248

The string which the exception builder receives is "\"Foo\",\"Bar\",\"Baz\"\r\"Biff\",\"Baff\",\"Boff\"\r\"Qux\",\"Teaspoon\",\"Doge\"" - no obviously unclosed quote

pezholio and others added 21 commits October 6, 2015 14:56
I'm not sure what it was testing for
this method is required for the autogenerated features to test foreign key criteria. It has been included in the finish method
`csvw_validation_tests.feature:1046` and `csvw_validation_tests.feature:1075` were failing because link headers was left unpopulated due to empty hash assignment in Initialise (added to catch previously failing tests).
It has been added to the `validate_metadata` section where @headers is guaranteed to have been initialised to not nil
cosmetic, no change in execution
reinstate the Excel warning logic from PR 149
this feature file would not acknowledge manually amended code after a merge, therefore attempting preemptive amendment
caught an error!
@pezholio pezholio force-pushed the feature-streaming-validation branch from 4095f8f to 39db911 Compare October 6, 2015 14:03
pikesley pushed a commit that referenced this pull request Oct 6, 2015
@pikesley pikesley merged commit bfb2a41 into master Oct 6, 2015
@Floppy Floppy removed the in progress label Oct 6, 2015
@pikesley pikesley deleted the feature-streaming-validation branch October 6, 2015 15:26
@kwent
Copy link

kwent commented Oct 7, 2015

\o/ :+1: +:100:

@pezholio pezholio mentioned this pull request Oct 7, 2015
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.

5 participants