yaml / yaml-test-suite

Comprehensive, language independent Test Suite for YAML
MIT License
181 stars 62 forks source link

Use a program to generate the in-json automatically from test-event #20

Closed rogpeppe closed 6 years ago

rogpeppe commented 6 years ago

Fixes issue #17.

I've left 2JQS there even though the YAML appears to be invalid, but there's only a single key in the JSON output.

All the JSON has been passed through jq for consistency, and sections arranged in consistent order. JSON which would be derived from data with null keys or with binary tags has been omitted.

perlpunk commented 6 years ago

for multi-document YAML files we decided to just put all JSON objects in the in.json, after each other. jq can process it, and if one wants to load it, it's not too complicated to split it into several JSON inputs.

rogpeppe commented 6 years ago

for multi-document YAML files we decided to just put all JSON objects in the in.json, after each other. jq can process it, and if one wants to load it, it's not too complicated to split it into several JSON inputs.

I just made that change (as a separate commit so it's easier to see).

rogpeppe commented 6 years ago

I like the cleanup in general, but I don't want the in-json moved after the out-yaml. I'd like to keep the old ordering. Can you change that back?

Oh, sorry, the old ordering was inconsistent - I just chose the order that the majority of the tests used (of the 155 tests that included both out-yaml and in-json, 132 put in-json after the out-yaml). Happy to move in-json before out-yaml in all of those too, or is there a reason for the ordering being different in some tests?

The order I'm using now (adjusted to put in-json before out-yaml) is:

Does that seem right?

I've created PR #21 (hopefully uncontroversial) so that the diffs in this PR will better reflect just the semantic JSON changes when that lands.

rogpeppe commented 6 years ago

I've updated the PR to do that (again in a separate commit, so the consequent changes are clear).

rogpeppe commented 6 years ago

I've just rebased this branch based on PR #21, which makes the semantic changes considerably clearer. Look at the second commit to see the actual changes.

ingydotnet commented 6 years ago

So you just generated in-json sections for tests where they were missing, correct?

The point ordering seems good to me.

The FRK4 comment is a TestML comment.

rogpeppe commented 6 years ago

I generated in-json for all tests that I could do it for. You're seeing the changes that resulted (i.e. the tests with existing in-json sections were correct).

ingydotnet commented 6 years ago

OK. I approve this if it passes the matrix and @perlpunk approves.

perlpunk commented 6 years ago

Some tests don't have in-json on purpose. I have to go over all tests and remove those which shouldn't have one.

perlpunk commented 6 years ago

@rogpeppe Could you run this again on master? I have merged #21. Also, tests with local tags like !circle need to be ignored, and probably !!set.

perlpunk commented 6 years ago

@rogpeppe we decided to do development in the devel branch, so that we can easily commit things, test them and later merge things to master. Could you run this against devel?

rogpeppe commented 6 years ago

Could you run this against devel?

Done, and also retargeted the PR against devel.

perlpunk commented 6 years ago

merged, thanks! (I removed two changes and amended the commit)