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

[js refactor] mark invalid/valid #13

Closed
mapsam opened this issue Aug 11, 2017 · 11 comments
Closed

[js refactor] mark invalid/valid #13

mapsam opened this issue Aug 11, 2017 · 11 comments

Comments

@mapsam
Copy link
Contributor

mapsam commented Aug 11, 2017

We should mark fixtures as valid or invalid (or possibly some sort of enum for semi-invalids #10) so downstream applications can assert based on success/lack thereof of parsing particular fixtures.

cc @springmeyer

@mapsam mapsam mentioned this issue Aug 11, 2017
3 tasks
@springmeyer
Copy link
Contributor

👍 on going with a Boolean right now of is_valid.

@joto
Copy link
Contributor

joto commented Aug 12, 2017

There are several different (maybe overlapping) "categories" I see here:

  • is valid/invalid in version 1
  • is valid/invalid in version 2
  • recoverable error: is invalid but reader should keep going (to next feature/next layer), for instance invalid geometry
  • fatal error: is invalid and reader should stop reading from this tile (or even from the source), for instance when somebody clearly is trying to provoke a buffer overrun
  • some way of saying where the problem (if any) is: in the geometry, in the tags, or more general

There are probably more.

@mapsam
Copy link
Contributor Author

mapsam commented Aug 14, 2017

@joto since we'll likely be using these fixtures outside of a javascript library, how would you prefer to know which fixtures are valid/invalid/semi? Trying to figure out how writing tests around these fixtures can be most helpful.

For example, would you prefer some sort of directory structure like:

fixtures/
  > valid/
  > invalid/
  > semi-invalid/
  > etc/ 

Or would you be happy just parsing the information from the info.json file?

@joto
Copy link
Contributor

joto commented Aug 14, 2017

I am happy with parsing this from a JSON file or I'll just take the info from there and hardcode it in some script or so.

Directory structure is unlikely to be enough because that can only encode one "category" (or would become overly complex with nested dirs or so).

@springmeyer
Copy link
Contributor

👍 on putting in the JSON file. This would also make it easier to update when we add "categories", and less noisy git diffs.

@mapsam
Copy link
Contributor Author

mapsam commented Aug 15, 2017

Excellent @joto @springmeyer - easier for me 😉

@mapsam
Copy link
Contributor Author

mapsam commented Aug 15, 2017

How about this description added to the README?


Validity: each fixture includes information about whether they are valid according to the specification versions and possible error outcomes if they are invalid. If any of the fixtures are invalid, they must include an error field describing how to recover (or not) from the error. These can be found in the validity field of the fixture and info.json files. The following checks:

  • v1 (Boolean): is this fixture valid according to Version 1.x of the Mapbox Vector Tile spec
  • v2 (Boolean): is this fixture valid according to Version 2.x of the Mapbox Vector Tile spec
  • error (String): describes if the encoder/decoder should recover from this error or stop completely. Values are
    • recoverable: should the encoder/decoder continue move on and continue its work? For instance, if invalid geometry is found, can the encoder safely move to the next feature?
    • fatal: the encoder should completely stop its process
    • ... more

@joto
Copy link
Contributor

joto commented Aug 15, 2017

@mapsam Looks good to me. We'll see how well it works once we are testing our tests and can always introduce more categories or so.

@mapsam
Copy link
Contributor Author

mapsam commented Aug 15, 2017

👍 indeed - thanks for 👀 @joto!

@mapsam
Copy link
Contributor Author

mapsam commented Aug 15, 2017

First pass completed in 9daefde

@springmeyer
Copy link
Contributor

👍

@mapsam mapsam closed this as completed Aug 15, 2017
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

No branches or pull requests

3 participants