Fix Object{N} + options mutation + command display names + big array/object message

[sainthkh]

• Closes command args are not displayed in the Command Log when options also passed into command #485 => feat: Show options with Collapsible #7716
• Closes cy commands with options object should not display as Object{N} in the Command Log #678 => feat: Show options with Collapsible #7716
• Closes Clone command options before passing into commands #3171 => User-provided options immutability. #6459
• Closes Reporter message improvements #5773 => Improve command name readability. #7527
• Closes Adding assertion that the subject is a very large array hangs the test runner #5929

User facing changelog

• Show options when users passed them to cy commands.
• Clone options object so that they're not mutated by the cy commands.
• Show spaces between words in commands.
• Test runner doesn't freeze when it processes a big array or object.

Additional details

Why was this change necessary?

• Issue command args are not displayed in the Command Log when options also passed into command #485: Show message when there is an option.
• Issue cy commands with options object should not display as Object{N} in the Command Log #678: In some cases, users see Object{N} message when they pass options. like below:
• Issue Clone command options before passing into commands #3171: User-provided options value has been changed after calling commands. It is unintended.
• Issue Reporter message improvements #5773: Messages like PREVUNTIL, SCROLLINTOVIEW are less readable than PREV UNTIL, SCROLL INTO VIEW.
• Issue Adding assertion that the subject is a very large array hangs the test runner #5929: Test runner freezes when a big array or object is used.

What is affected by this change?

Check changes below.

Any implementation details to explain?

• issue command args are not displayed in the Command Log when options also passed into command #485: Not much. It is accidentally fixed.
• issue cy commands with options object should not display as Object{N} in the Command Log #678: When a user passes options object to any command, it is copied and passed to CommandModel. And it is rendered by Message component.
• issue Clone command options before passing into commands #3171: now options are used to call the user-provided options. And opts are options filled with default values.
• issue Reporter message improvements #5773: Added displayName to functions.
• issue Adding assertion that the subject is a very large array hangs the test runner #5929: replace #{this} manually when array or objects are too big.

How has the user experience changed?

Issue #485

Before:

After:

Issue #678

Before:

After:

Issue #3171

const options = {
  requestTimeout: 3000,
}

cy.wait(100, options)

expect(options).to.deep.eq({ requestTimeout: 3000 })

Before: Test above failed.
After: It passes.

Issue #5773

Before:

After:

Issue #5929

Before:

Endless array/object.

After:

Show ...more when an array or object is too big.

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation?
  • Screenshot in cy.click() should be changed. Because message is a bit changed.
  • Update cy.each(). Because it has options but it is not documented.
  • Needs to add options option and explanation to Cypress.log() page.
  • Doc work will be started when this PR is reviewed.
• Have API changes been updated in the type definitions?
• [NA] Have new configuration options been added to the cypress.schema.json?

[cypress-bot]

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

• Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
• Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

• The feature/bugfix is self-documenting from within the product.
• The change provides the end user with a way to fix their problem (no dead ends).

Functionality

• The code works and performs its intended function with the correct logic.
• Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
• The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

• The code is readable (too many nested 'if's are a bad sign).
• Names used for variables, methods, etc, clearly describe their function.
• The code is easy to understood and there are relevant comments explaining.
• New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
• There are comments containing link(s) to the addressed issue (in tests and code).

Quality

• The change does not reimplement code.
• There's not a module from the ecosystem that should be used instead.
• There is no redundant or duplicate code.
• There are no irrelevant comments left in the code.
• Tests are testing the code’s intended functionality in the best way possible.

Internal

• The original issue has been tagged with a release in ZenHub.

[jennifer-shehane]

Thanks so much for this large amount of work! 💯

Overall, we'll need to discuss the visual changes to the reporter with the engineering team. I think a lot of this is in the right direction, especially with

• cloning the options object
• displaying the full options (the cy.request() with all its options printed actually looks really nice).
• splitting out the message from the options for the reporter. @chrisbreiding and I have been doing some work on splitting up the portions of the messaging from commands for the work in Improve Test Runner error experience #3762

I do have some thoughts on changes though:

1. We do have a subset of reporter tests that are small, but growing. This would be an ideal place to test the new option displays. They're not super intuitive, and require some mock data, so ask if you are stuck somewhere. We'd love to have the visual changes tested. ✅
   • Here is an example: https://github.com/cypress-io/cypress/blob/develop/packages/reporter/cypress/integration/suites_spec.js
   • Here are instructions to run them: https://github.com/cypress-io/cypress/blob/develop/packages/reporter/README.md#L65:L65
2. I think splitting up the commands into separate words is not the right direction.
   • Going from SCROLLINTOVIEW to SCROLL INTO VIEW, I don't know, it takes up more space and doesn't look much more visually appealing. It also make the words return at smaller sizes (see screenshot).
   • Perhaps we should not do ALLCAPS for these, so that they reflect the way the actual command was called instead of splitting up the words, so instead of SCROLLINTOVIEW have scrollIntoView. I think this should be as simple as removing this line of css: https://github.com/cypress-io/cypress/blob/develop/packages/reporter/src/commands/commands.scss#L202:L202 , so it may be quick to see what it looks like.
3. I threw together some random commands with options. The .type() command (as shown in screenshot below) shows the options in both the message and in the options now.
4. I think the options styling in the reporter could use some work. They text is too tiny, making it hard to read.

Before

After

[sainthkh]

@jennifer-shehane

1. I added commands. But it doesn't show options. I'm trying to understand why it happened.
2. You're right. Removing uppercase is better than adding spaces.
   
3. Fixed cy.type().
   
4. Currently, options is 0.8 opacity and 0.8 font-size of the message. Should we raise them to 0.85 opacity and 0.9 font-size like below?
   
   Or there might be better solutions.

[sainthkh]

@jennifer-shehane It didn't show options because I didn't run npm run watch in report folder. Now, everything works fine.

• flaky test.

[jennifer-shehane]

Hello again @sainthkh I ran by the UI with some of the team and have some feedback.

1. Let's go back to the all caps styling. We are good with the .SROLL INTO VIEW, but would like it to not create a new line as in the case pictured and instead push the command further out.
   
2. The options styling is better with the newer larger slightly darker font, so is 👌
3. I was also asked to see what happens if someone has an extraordinary large amount of data in the options object. We do not appear to be doing any sort of truncation on this and would likely want to handle this situation better. Even if it's some really large character limit, it'd be better than none.
   

[sainthkh]

3 things have been changed.

1. Add space after colon, comma. + Show quotes when there is colon, comma, space in the string.
2. There are 3 levels of method name area width: 80, 100, 130. Previously, there were only 1 level: 80. These widths are decided by the length of method name (+ if it's a child command, add 2).
3. Max height of the options area is 90px. If it goes over that height, scrollbar is shown.

Everything is in this screenshot:

[sainthkh]

Fixed conflicts.

[jennifer-shehane]

Another interesting issue concerning display of very large output. #5929

[jennifer-shehane]

@sainthkh We were thinking the display of the object might be nice displayed as expandable, similar to how the new 'configuration' tab handles objects in the Desktop-GUI. See this PR: #5068 I wonder if it could use the same components.

If you consider this outside of the scope of this PR, that's alright too. We're not sure about the scrollbar in general though.

Also, have you ever considered applying for a position here? You can apply online: https://cypress.io/jobs/

[sainthkh]

@jennifer-shehane

1. I think Adding assertion that the subject is a very large array hangs the test runner #5929 should be added to this PR. I'll fix it tomorrow.

2. I think we need to decide after seeing it work in that way. I am not so sure about this. I'm worrying if it would make things too complicated. I'll try tomorrow.

3. Actually, that's what I wanted to do. But I have very mediocre career as a developer. So, my resume isn't persuasive enough. That's why I've been doing these.

My actual goal was to finish out-of-the-box typescript support and cypress init and apply for the job.

But it seems that the chance came early. Thanks. As it's Friday now, I'll apply for the position next week.

[bahmutov]

Super, we are glad to hear this

…

Sent from my iPhone

On Dec 13, 2019, at 07:23, Kukhyeon Heo ***@***.***> wrote:  @jennifer-shehane I think #5929 should be added to this PR. I'll fix it tomorrow. I think we need to decide after seeing it work in that way. I am not so sure about this. I'm worrying if it would make things too complicated. I'll try tomorrow. Actually, that's what I wanted to do. But I have very mediocre career as a developer. So, my resume isn't persuasive enough. That's why I've been doing these. My actual goal was to finish out-of-the-box typescript support and cypress init and apply for the job. But it seems that the chance came early. Thanks. As it's Friday now, I'll apply for the position next week. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

[sainthkh]

1. The cause of #5929 was Cypress trying to create a string representation of an object. In the test case, Cypress was trying to make the string: "0: 0, 1: 0, 2: 0, ..., 2559999: 0". I didn't calculate the length of that string, we are sure that that's really loooooooooong.

Processing that long string caused the freeze. And it is solved by showing '...more' message when an array or object is too big.

Big here means to have more than 10 items. Maybe, we need to discuss 10 is the right number.

Before:

Endless array/object.

After:

Show ...more when an array or object is too big.

2. Added react-inspector component to show the options.

I think it looks better than stringified object.

3. We might need to do similar thing to the wrap message. It tries to show 2560000 items in arrays. And it slows down the reaction of the browsers. And Object{2560000} doesn't give us much information.

The problem is: Should we use react-inspector for wrap or use a simple string?

4. I sent my application.

[sainthkh]

Closing because things have changed a lot.