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

fix(reporter): Use multiline strings to escape YAML #77

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

lkipke
Copy link
Contributor

@lkipke lkipke commented Mar 5, 2021

Summary

Closes #72. This converts all of our expected/actual values to be YAML multiline strings, which has the effect of escaping any YAML special characters that may appear in strings/variable names.

Also, converting everything to a string ensures that tap-mocha-reporter always shows an error diff -- it does not print any diff output if the types of expected vs. actual are not the same.

This also fixes a bug where roca.log("message") was not working due to a reference error.

Screen Shot 2021-03-05 at 1 32 23 PM

Screen Shot 2021-03-05 at 1 31 08 PM

@lkipke lkipke added the bug Something isn't working label Mar 5, 2021
@lkipke lkipke requested review from sjbarag and alimnios72 March 5, 2021 21:05
@lkipke lkipke self-assigned this Mar 5, 2021
level = level + 1
m.printExtras(extra[item], level)
if extra[item]._roca_isMultilineString = true
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 where does this get set? I can't find it 🔎

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a little funky... but i'm not sure how else to render multiline in only these scenarios

Copy link
Contributor

Choose a reason for hiding this comment

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

👓 I swear I command-f'd for that 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

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

although we could always render every string as multiline? the raw YAML output would just be harder to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

although we could always render every string as multiline? the raw YAML output would just be harder to read.

That's certainly an option. I'm fine with either!

Copy link
Contributor

@alimnios72 alimnios72 left a comment

Choose a reason for hiding this comment

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

LG

@@ -0,0 +1,7 @@
' Initializes a tap instance and prints out the plan.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a previous bug I just noticed. When I converted roca_main to TS, I put tap.brs in executable script format, instead of pulling it into the execution scope, because I thought that it only self-referenced.

But, I missed the roca.log() function, that also uses tap.brs. So now, the executable part is still run (this file), but the Tap object and function definitions are pulled into scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I'm fixing here is because roca.log() uses the printExtras function that this PR modifies.

Copy link
Contributor

@sjbarag sjbarag left a comment

Choose a reason for hiding this comment

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

This seems about right! Good catch(es) 😃

@lkipke lkipke merged commit 9c5de50 into hulu:main Mar 5, 2021
@lkipke lkipke deleted the fix-yaml-output-errors branch March 5, 2021 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escape YAML characters in error output
3 participants