Skip to content

Conversation

novemberborn
Copy link
Member

  • Properly print multiline strings (assuming \n is the line separator)
  • Print additional name & message properties
  • Determine 'at' value similar to serialize-error.js. Recompute since here we want the full line
  • Print unhandled errors using the same logic
  • Strip ANSI from actual and expected values

output.push(` name: ${yamlBlockPropertyValue(error.name)}`);
}
if (includeMessage && error.message) {
output.push(` message: ${yamlBlockPropertyValue(error.message)}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to handle quotes.

For example:

message: 'hello' this is not valid YAML

Wouldn't it be better to just use a real YAML stringifier? Like js-yaml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, not sure why I thought I could get away with the manual approach 😉

* Properly dump errors
* Only print actual and expected values if they're present, and strings.
This assumes they're serialized in the test workers
* Strip ANSI from actual and expected values
* Print additional name & message properties
* Determine 'at' value similar to serialize-error.js. Recompute since
here we want the full line
* Print unhandled errors using the same logic
@novemberborn novemberborn force-pushed the improve-tap-reporter branch from 8c34963 to 15eeb54 Compare March 9, 2017 11:41
@novemberborn
Copy link
Member Author

Updated.

@novemberborn novemberborn mentioned this pull request Mar 9, 2017
@sindresorhus
Copy link
Member

Looks good :)

I wish TAP had some kind of testsuite we could run our implementation at to check for conformance.

@sindresorhus sindresorhus merged commit 3279336 into master Mar 11, 2017
@novemberborn novemberborn deleted the improve-tap-reporter branch March 12, 2017 14:36
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 this pull request may close these issues.

2 participants