-
Notifications
You must be signed in to change notification settings - Fork 167
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
schema fixtures: testmarkify #144
Draft
warpfork
wants to merge
6
commits into
master
Choose a base branch
from
schemas-tests-testmarkify
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…pdating them. I've added a lot of prose explainer interspersed between data hunks. I chose to combine a bunch of the basic things (especially, scalar kinds like int and float) into one file. This might be stylistic (and I could be convinced to split it back out), but made sense to me, and felt relatively reasonable-looking in testmark (because it's fairly normative to start hunk names in testmark with a test name prefix and have several fixtures in the same file that way), so I gave it a spin. I've ditched the actual-vs-expected distinction in example blocks. In almost all cases, they were equal. The only places where they differed where where there were comments speculating if various coersions like string-to-int or float-to-int were supposed to happen. In all cases, the answer to those questions is "no". So that simplifies things nicely. (Okay, there's one more exception around json floats, but... I let this be lost; I think that's testing json codec normalization, not schemas; it doesn't belong here.) I renamed "blocks" to "match" and "badBlocks" to "nomatch". We could bikeshed these to something else (I'm not super attached to my picks), or revert to those previous terms; I just felt a little strange at the word "block" floating around. I'm trying to avoid tackling this too much, at the moment but it may be worth a brief mention that some of the fixtures for data that matches feel like they might be probing tangents. For example, a bunch of the float fixtures seem like they're sanity checking the json float parser as much as they're saying anything about the schema matching. I'm not sure these are optimally useful. But they also don't hurt, so I've (mostly) kept them. I made some target-of-opportunity fixes, such as replacing fixtures about null with updated fixtures about unit (which follows #136 ). I put all this in a dir called "fixtures" because that's what the similar dir is called in the selector specs, and in most of the codec specs that have them. (We're not 100% consistent on this either, in fairness. For the CAR specs, the dir is called "fixture" -- no "s". Ah well. Aiming towards the center of gravity.) So far this diff just covers a few of the files; I've removed the content I've ported, but there's a lot to go.
Update several things: - split them apart better! No reason to put several of these into one schema now. (We don't get forced to make new files for new schemas, anymore, so there's less barrier vs more smaller ones.) - the representation syntax for ints had quote marks; we've moved towards thinking that was wrong approach, as of #137 , so I've updated to use bare integers accordingly. - as usual: more docs prose and explainer material interspersed.
2022-04-05 conversation: putting this and ipld/go-ipld-prime#381 back to Todo, waiting on someone to have the bandwidth to complete all the fixtures. @rvagg is interested in picking it up if/when he frees up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Porting schema fixtures from yaml to testmark format. Reviewing with a fine-tooth comb as I go, and interspersing documentation, since the format permits it.
I'm aiming for a documentation level that's useful while terse for a reader that's implementing schema systems, and dry but survivable for a reader that's a user just wanting to see more detailed examples of how to use the DSL (or DMT) and what it matches.
Some of the impetus is covered in #142 (comment) and other comments in that PR.
There aren't many changes to the logical content, but where there are, the commit messages per commit will describe it.
The new ported content is going into a dir called "fixtures" because that's what the similar dir is called in the selector specs, and in most of the codec specs that have them. (We're not 100% consistent on this either, in fairness. For the CAR specs, the dir is called "fixture" -- no "s". Ah well. Aiming towards the center of gravity.)
The new index file for the directory contains a brief explanation of the format and how the data hunks are labelled, so that might be a good place to start reading from.
This is still a work in progress. I'm removing the old yaml files as I port them, so files remaining with that extension can be seen as the progress bar.