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

in.json should be converted to canonical format #19

Open
perlpunk opened this issue Jan 3, 2018 · 3 comments · May be fixed by #139
Open

in.json should be converted to canonical format #19

perlpunk opened this issue Jan 3, 2018 · 3 comments · May be fixed by #139

Comments

@perlpunk
Copy link
Member

perlpunk commented Jan 3, 2018

To be able to reliably compare JSON the in.json should all be converted to the default jq output.
To ensure that also future additions have the correct format, we would need a script that automatically converts in-json in a tml file.

@ingydotnet
Copy link
Member

As I mentioned in the #20 review, in-json was not meant to be a raw expected result string (that would be called out-json), but rather a loaded JSON input that would be compared either in memory or by a reserialization into any mature/trusted format.

That said, I support the change if it helps people use the data in other useful ways and does not break any existing testing.

@perlpunk
Copy link
Member Author

perlpunk commented Jan 4, 2018

  • Why should this change break any existing testing? I will let the test matrix run before merging it to master
  • if we generate in.json half-automatically, a canonical format will guarantee that we only get diffs for real changes

@eemeli
Copy link
Member

eemeli commented Jul 1, 2018

With the changes brought in by PR #20, the in.json files representing streams with multiple documents can no longer be easily parsed in a JS environment. In fact jq might be one of the only tools that can parse their format without complaint.

As is, the files are pretty close to line-delimited JSON, for which tools do exist. Therefore, would it be possible to add a step that would use something like jq -c in their generation?

I encountered this problem while updating the tests of my JS YAML library, which can in fact read the in.json format, but complains about it:

import fs from 'fs'
import YAML from 'yaml'
const src = fs.readFileSync('./U9NS/in.json', 'utf8')
const doc = YAML.parseDocuments(src)[0]

doc.errors
// [ [YAMLSyntaxError: Document is not valid YAML (bad indentation?)] ]

doc.contents[0].toJSON()
// { time: '20:03:20',
//   player: 'Sammy Sosa',
//   action: 'strike (miss)' }

doc.contents[1].toJSON()
// { time: '20:03:47',
//   player: 'Sammy Sosa',
//   action: 'grand slam' }

eemeli added a commit to eemeli/yaml-test-suite that referenced this issue Jul 1, 2018
eemeli added a commit to eemeli/yaml-test-suite that referenced this issue May 9, 2020
eemeli added a commit to eemeli/yaml-test-suite that referenced this issue Nov 28, 2021
eemeli added a commit to eemeli/yaml-test-suite that referenced this issue Mar 11, 2022
@eemeli eemeli linked a pull request Jun 8, 2024 that will close this issue
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 a pull request may close this issue.

3 participants