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

Standardise core functionality within the TAP YAML format. #9

Closed
8 tasks
jonathanKingston opened this issue Jan 18, 2015 · 18 comments
Closed
8 tasks

Comments

@jonathanKingston
Copy link
Member

As brought up by #1 I think we should work upon standardising the TAP format itself.

I think this could start with adding normal language style exception information, above that we can add further information.

Common properties of errors are:

  • Error message
  • Error name
  • Description
  • Error number
  • File Name
  • Line Number
  • Column Number
  • Stack trace

Much like language exceptions it might also be worth adding in the ability to type the errors.

@jonathanKingston
Copy link
Member Author

@kinow
Copy link
Member

kinow commented Jan 19, 2015

Thanks for the ref link. I remembered reading similar proposal somewhere. In tap4j we use similar structure, and in functional tests with Selenium, usually I add extra entries under the extensions key.

@kinow
Copy link
Member

kinow commented Jan 19, 2015

I think we probably can leave few suggestions or models. Like two or three examples of use, that can be followed or not by implementations. It will be quite hard to define a good set of properties that can be applied to most implementations, I think.

@Leont
Copy link

Leont commented Jan 19, 2015

I think I rather like the idea of having several profiles for structured diagnostics, so that various use-cases can be adequately transmitted.

@Ovid
Copy link

Ovid commented Jan 21, 2015

A few thoughts on this.

There was a point where we thought "an official set of keys" and user extensibility, like the HTTP X- headers (though they've been deprecated). Perhaps having a safe "notes" key were you can dump anything that's not official?

Also, I have concerns about YAML. The YAML spec is huge and as a result, people keep hitting subtle problems with YAML not working the way they expected it to, largely because the spec is so huge that implementations keep getting it wrong.

Here are just a few issues people have had (I could find tons of other links):

That last issue is very interesting because it talks about a well-known Rails security hole cause by untrusted YAML input. Since one of the ideas behind subtests would be to allow us to aggregate TAP from multiple sources, we could be injecting a known security issue into TAP. That would be unfortunate.

One possibly solution is to restrict YAML to a subset that some refer to as YAMLish. It parses as YAML, but is very restricted in what it provides, has a defined spec, and is easy to implement. That could greatly improve the interoperability of YAP between languages. The issue here is that people will ignore it and go with YAML anyway.

Switching to JSON or another format may be an option.

I don't know what the right answer is.

@kinow
Copy link
Member

kinow commented Jan 21, 2015

That last issue is very interesting because it talks about a well-known Rails security hole cause by untrusted YAML input. Since one of the ideas behind subtests would be to allow us to aggregate TAP from multiple sources, we could be injecting a known security issue into TAP. That would be unfortunate.

Reading the three cases, they all sound like problems in the implementations, not in the specification itself - though as you noted it is indeed huge. I tried reading the whole SnakeYAML code some tie ago, but there were some many verifications for special characters and special conditions (probably due to something in the spec) that I gave up.

One possibly solution is to restrict YAML to a subset that some refer to as YAMLish. It parses as YAML, but is very restricted in what it provides, has a defined spec, and is easy to implement. That could greatly improve the interoperability of YAP between languages. The issue here is that people will ignore it and go with YAML anyway.

+1

Switching to JSON or another format may be an option.

I also don't know the right answer. My arguments for using YAML over JSON were always that YAML supports comments and JSON not, and that JSON was a subset of YAML, so in theory you can represent the same data you have in JSON as YAML (though I never tried that). There few other points here.

From a implementation developer point of view, it doesn't make much a difference for me, since if we ditched YAML for JSON, I'd simply replace SnakeYAML dependency by Jackson, and update some code in the parser/consumer.

@jonathanKingston
Copy link
Member Author

@Ovid yeah YAML is pretty feature packed and has lots of useful features, however it is far too rich for what we actually want it for. The YAMLish format was always what I was suggesting.
The only slight advantage to using the JSON subset of YAML means as @kinow said it could be compiled backwards into native documents and JSON and that YAMLish is slightly easier to read especially when it comes to stack traces or chunks of text. However for a lot of the motivation on that page, a lot has changed in 7 years - there are JSON editors and wouldn't need the --- and ...

I would prefer to either:

  • Continue using YAMLish (get a better page on how the language is defined also)
  • Allow for JSON as well

The keys under the YAMLish page are pretty sensible as a starting place

@Leont
Copy link

Leont commented Jan 21, 2015

One possibly solution is to restrict YAML to a subset that some refer to as YAMLish. It parses as YAML, but is very restricted in what it provides, has a defined spec, and is easy to implement. That could greatly improve the interoperability of YAP between languages.

I'd favor that over full YAML. Quite frankly it's still a bit tricky to parse (I'm writing such a parser right now), but far easier than full YAML. JSON is much easier to deal with, but much less human friendly. Anything else is not really viable I suspect.

The issue here is that people will ignore it and go with YAML anyway.

Possibly. On the parsing side that shouldn't be an issue, it's the client side where we should be strict.

@kinow
Copy link
Member

kinow commented Jan 22, 2015

@Ovid, @Leont, @jonathanKingston all +1 for YAMLish? :-) If so I believe we can move forward and discuss the possible specs for the diagnostics keys.

@Leont
Copy link

Leont commented Jan 22, 2015

all +1 for YAMLish? :-) If so I believe we can move forward and discuss the possible specs for the diagnostics keys.

The main issue with YAMLish right now is that we'd have to spec it ourselves.

@Ovid
Copy link

Ovid commented Jan 22, 2015

The main issue with YAMLish right now is that we'd have to spec it ourselves.

Here you go :)

@jonathanKingston
Copy link
Member Author

@Ovid that is certainly a start.

I think it would be nice to define it as a spec clear of needing understanding of YAML, I get the feeling it actually looks a lot more complex to most than it actually is.

@Leont
Copy link

Leont commented Jan 26, 2015

I think it would be nice to define it as a spec clear of needing understanding of YAML, I get the feeling it actually looks a lot more complex to most than it actually is.

Yeah, that

@jonathanKingston
Copy link
Member Author

There is always standardising a fresh format:
https://github.com/jonathanKingston/jetson

I worked on this mostly for fun and that it cures my issues with JSON and gives YAMLish behaviour to JSON using ECMA6 standard string templates.

not ok 12 - item name
{
  trace: `/dir/filename.js .....
/dir/filename.js .....
/dir/filename.js .....
`,
  // This explains a thing
  errorCode: 12,
  etc: 'thing'
}

It is alpha level code however I thought it would be worth mentioning here as JSON has been mentioned.

The above in JSON would either be:

not ok 12 - item name
{
  trace: '/dir/filename.js .....\n/dir/filename.js .....\n/dir/filename.js .....',
  errorCode: 12,
  _comment1: 'This explains a thing',
  etc: 'thing'
}

or:

not ok 12 - item name
{
  trace: [
    '/dir/filename.js .....',
    '/dir/filename.js .....',
    'dir/filename.js .....'
  ],
  _comment1: 'This explains a thing',
  errorCode: 12,
  etc: 'thing'
}

@beatgammit
Copy link

@jonathanKingston

I like JSON, but I would prefer to use the JSON standard instead of something else (e.g. " around keys). It's a bit difficult to find parsers/producers in every language for anything non-standard.

Perhaps if this is to be done, it should be a pragma like the TAP version? I can imagine it being easier to find a JSON producer than a YAMLish producer, so it would be nice to have that as an option. If this is to be done, I think there should be a guideline that says JSON SHOULD be pretty-printed so it's easier to read for humans.

@jonathanKingston
Copy link
Member Author

@beatgammit - Yeah pretty print doesn't help much here for multiline strings which is very common in the YAML.

I agree non standard formats are a pain, but then JSON is going to struggle on big log outputs, YAMLish parsers are pretty limited in number and YAML parsers are too feature packed with issues.

Jetson on the other hand is just valid ECMA6 JSON making it as easy to parse as YAMLish with a familiar touch of JSON.

Pretty print SHOULD suggestion is a good shout if it comes into play.

@ebo
Copy link

ebo commented Feb 15, 2019

I will have to read through the details of this thread, but I came at it from another direction.

Thinking about this abstractly, each yaml block is attached to something (typically a test or diagnostic line) -- which provides a scope. Some of the standard properties actually come from different scopes -- such as error name/number/line and file_name. Assuming that the Plan is defined in a single file (mine are at least), then its context scope is the entire plan and not just individual lines. So is it reasonable that you would have multi-level context for any or all of these variables?

Earlier today I reached out to the tap-l list (which bounced BTW) to ask if it is valid to attach a yaml block to the plan -- example:

TAP version 13
1..N
   ---
   name: 'The Grand Plan'
   run_time: 100us
   submitted: 2019/01/01
   accepted: 2019/02/14
   ...
ok 1 Description # Directive
# Diagnostic

I am interested in using yaml block as a metadat

@isaacs
Copy link
Contributor

isaacs commented Mar 4, 2022

The nice thing about using YAML (not tiny, not ish, the godawful giant ass spec we all hate) is that you probably don't have to write a parser for it. Godawful gigantic though it is, most languages already have a specification and a handful of decent parsers for it. I found rather quickly with node-tap that any value in having a smaller "yamlish" diagnostic language was quickly outweighed by the costs of dealing with users surprised that perfectly valid YAML wasn't parsed how they expected.

English is spoken the world over. JavaScript is in every web browser and on every server. YAML is used to configure everything. The worst sloppy convenient mishmash of a thing usually wins, we should just accept it 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants