qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
59 stars 18 forks source link

Wrong YAML block in TAP report #109

Closed jeberger closed 4 years ago

jeberger commented 5 years ago

YAML blocks in TAP reports may be wrong if one of the fields spans more than one line, which is often the case for stack traces. For example:

not ok 5 Voice > State
  ---
  message: "Test failed"
  severity: failed
  actual: 1
  expected: 2
  stack:     at function1 (/path/to/test.js:11:3)
    at function2 (/path/to/test.js:22:3)
  ...

The stack trace is not valid YAML. It should follow the YAML line folding rules.

Krinkle commented 4 years ago

There was indeed a bug with multi-line strings. The TAP reporter handled those incorrectly, it simply printed them as-is, without considering the YAML output format that we're inside of.

However, while the fix for this was, technically valid, I think it made two mistakes:

  1. There are multiple ways to format strings in YAML. The option we choose here is in my opinion not so easy to read (due to how it encodes line breaks as \\n). It would be better I think if we use a multi-line format for multi-line strings. Reading long sequences of escaped line breaks makes debugging difficult. This is part of why I have not yet upgraded to the latest js-reporters version in QUnit.

  2. I have read the TAP spec a few times, but do not find anywhere that suggests that the data.expected/actual must contain a string for display. In fact, it shows examples that demonstrate use of other native YAML value types such as objects and arrays (not doubly-wrapped JSON strings), and it also specifically states that "the [schema] format of the data structure represented by a YAML block has not been standardized".

I think what matters most is that other command-line programs that follow the TAP spec, can parse the js-reporters TAP output, and receive the actual/expected values in their correct type. That means they should either use neatly-formatted YAML (e.g. literal numbers, multi-line strings, and native objects), or JSON (which is allowed anywhere in YAML). However even when choosing JSON, it should be as a literal, not as a doubly-wrapped string. Consumers must not need to re-parse the JSON sub-portions. It's just handled naturally by any YAML parser.

The changes I'll be making: