-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
asterisks displaying instead of single quotes in Command Log assertion #1360
asterisks displaying instead of single quotes in Command Log assertion #1360
Comments
We send the values down wrapped with We need to escape newlines so they actually appear like |
Hi, I'm new to contributing, but I don't think the problem is specifically due to newlines. It's the fact that the entire string itself is being processed as markdown. I don't think that's a good idea. In this specific case, the issue is that markdown treats However, this issue goes deeper than this. Let's say I'm looking to match
Now we have italicized our test Let's take it to the extreme, let's say we do this:
We get this: Basically, if your tests include anything that can be parsed by markdown, it will. I have not looked at where specifically in the code these strings are being processed by markdown, but I think it should be avoided (or at least the contents of these strings should not be parsed at all). Here's another example of how this could be really confusing: Let's look for
Now we get this: This makes no sense at all since it's saying that two identical strings don't match. It should show exactly what it does further down: My conclusion would be that these strings shouldn't be parsed by markdown at all. It would also make sense to add quotes so that it would be more obvious why the two strings don't match. I'd be happy to investigate the code and make these changes, but I'd imagine a design change like this would have to be signed off by someone. |
We originally tried quotes but then it looked like everything you were comparing were strings. Remember that you can always click on an assertion and it will print the actual values to the console completely unformatted. Stringifying any kind of value will ultimately format it potentially different than the way its data structure actually is. Comparing function references has the same problem. I think all we need to do in this situation is escape the values so that they will not be parsed or rendered as markdown - but we can otherwise continue using it. It's a fairly trivial solution. @chrisbreiding can you knock this one out? |
Interesting. That would work if you escape every markdown special character, but isn't one of the issues here that if the string starts with whitespace, it will also display incorrectly? for instance, ' Test ' will display as '** Test **' since you cannot start a bolded section in markdown with whitespace (as shown in the first comment). You also cannot escape the whitespace in markdown (as far as I can see). What would be the solution for that? Would you strip the whitespace from the front and back of each string as well as escaping special characters? Also, I noticed that this was tagged as first-timers-only. Sorry if I'm overstepping, but it would be awesome if I could help out with a solution here :). I really like cypress, and I'd love to get involved with some open source! Steve |
We could add quotes if the assertion subject or expected value is a string. That would fix the bold issue - since as long as quotes come after the asterisks then you can subsequently have spaces. That would also provide more context on the assertion subject and values (like chrome dev tools does). But other data structures should not be quoted - such as numbers, arrays, objects, etc. For other characters and new lines we likely still need to escape those so they are not literally evaluated by markdown. The tests for the assertion messages are here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/test/cypress/integration/commands/assertions_spec.coffee You're more than welcome to start on a PR. Most of the time our team will "top off" or finish PR's from outside contributors. Having an initial PR will definitely get the ball rolling and we prioritize those over issues that are "ready for work". We do have good contributing docs - but Cypress is a large and complex project - so we don't expect people to get everything right by themselves. Because we are a team working on this every day - in some cases there are clear bug fixes where the solution does not require discussion - in other cases (like this one) the actual solution may not be clear. In those cases, we may iterate internally as a team to quickly come to a decision. Oftentimes there are areas of the system that some team members are more familiar with than others. In this case - @chrisbreiding was the original person who implemented markdown in order to structure the assertions. My thought process on this is that bolding the subjects and the values is the most common and correct use case. It's ideal but as you've pointed out - there are edge cases. Instead of replacing the system, I believe it would be better to handle those edge cases and further stick to the 80/20 rule. It doesn't have to be perfect - you can always click on an assertion to see its true valuel I think the current discussion is about handling:
We have a set of common stringifier utils in the driver that are used throughout the system that handle stringification. We likely just need to utilize those. |
Cool! Thank you for the detailed explanation of your workflow! I look forward to contributing if I see the opportunity. |
The code for this is done in cypress-io/cypress#5619, but has yet to be released. |
Released in |
Current behavior:
Command Log for assertion chain includes confusing asterisks. I suspect it may be replacing
\n
with**
.Even displays in passing tests
Desired behavior:
Error in Command Log should display as:
expected '<div.duration>' to have text '37:46', but the text was ' 37:46 '
Code to reproduce
The text was updated successfully, but these errors were encountered: