Fix timeout progress bar for cy.wait

[panzarino]

• Closes Timeout progress indicator for cy.wait is incorrect if timeout option is not passed #7881

User facing changelog

Fixes a bug where the timeout progress bar would not reflect the actual timeout for cy.wait().

How has the user experience changed?

Before:

After:

PR Tasks

• Have tests been added/updated?
• Has the original issue been tagged with a release in ZenHub?

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[cypress]

---

Test summary

7896 • 0 • 130 • 0

---

Run details

Project	cypress
Status	Passed
Commit	a1de94d
Started	Jul 20, 2020 5:19 AM
Ended	Jul 20, 2020 5:27 AM
Duration	07:58 💡
OS	Linux Debian - 10.1
Browser	Multiple

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[jennifer-shehane]

This does now reflect the default requestTimeout of cy.wait() in the progress bar.

I feel like this is likely related, but if you do cy.wait() with just a number timeout, the progress bar is not accurate either (bug before this PR). Where the progress bar shows it has about 10secs, but really I've set it to expire at 5 secs.

Feel free to ignore and open a new issue if you think this is outside the scope of this work.

I think there should be some tests for this PR though. 🙃

Also, I'm a bit curious how the progress bar shows for different requestTimeout + responseTimeout. So...say I have a requestTimeout waiting for the XHR to make a request for 20 secs - it take 10 secs to request, and THEN I have a responseTimeout waiting 30 secs for it to respond - it takes 20 secs to respond.

• Does the progress bar show 50 secs of countdown?
• When does the progress bar resolve? At the request resolution? At the response resolution?
• Does it do the request countdown and then reset a new progress bar for response countdown?

This is the only command that combines 2 timeouts I believe, so this is a unique concern here.

[panzarino]

@jennifer-shehane I've fixed the issue regarding passing a number to cy.wait() (my original implementation used the wrong timeout). I've also added tests for ensuring the log timeout is properly set with the requestTimeout and responseTimeout.

There are a couple of ways that we can go about designing the progress bar updates:

I just pushed changes that look like the following, where the progress bar will keep its position but slow down to reflect the updated timeout:

Alternatively, we can make sure that the progress bar always reflects the actual percentage complete. For example (shown in the gif below), if we're at 2000ms out of 3000ms (66.67%) and the timeout is increased to 5000ms, the percentage complete reverts to 40%.

To test this design yourself, replace lines 80-86 of command-model.js with the following:

this.timeout = props.timeout

Finally, we can reset the progress bar to the beginning and have it progress to the end of the time remaining in the updated timeout:

To test this design yourself, replace lines 80-86 of command-model.js with the following:

const timeElapsed = Date.now() - new Date(this.wallClockStartedAt).getTime()

this.timeout = props.timeout - timeElapsed
this.wallClockStartedAt = new Date().toJSON()

Here is the test case that I used to generate the examples:

it('tests', () => {
  const start = Date.now()
  let pass = false

  cy.on('command:retry', () => {
    if (Date.now() - start > 2000 && !pass) {
      pass = true

      // trigger request to move onto response timeout verification
      const win = cy.state('window')

      xhrGet(win, '/foo')
    }
  })

  cy
  .server({ delay: 3000 })
  .route('GET', '*', {}).as('fetch')
  .wait('@fetch', { requestTimeout: 3000, responseTimeout: 5000 })
})

Personally I think that designs 1 and 2 make the most sense. Design 1 is the most visually appealing (but might move too slow if near the end and the timeout changes to something huge like 30000) and design 2 makes sense since it actually reflects the real percentage complete (and has the most simple implementation).

[jennifer-shehane]

@panzarino I think that Design 1 looks the least 'buggy'. While maybe still not completely clearly communicated why the progress bar slows down, the other 2 look more like glitches, where someone may think there was a bug with wait running twice. Thanks for putting the effort in to implement this thoroughly!

[JessicaSachs]

Some code cleanliness nits.

More importantly, I found a bug with how the animation is being calculated and reset...

• When you toggle the parent component, the latest timer state is lost. (see the gif)

• If you let the timer run out and then toggle the parent component, then the animation correctly stays completed.

• When the tests are all passing, the timeout still counts down if you expand the succeeding tests.

Let me know if you agree w those as issues. I'm around to pair tomorrow

[panzarino]

So the main issue with the first design is that it doesn't look great with the default timeouts. It starts moving so slow that users might actually think something got stuck.

The second option alleviates this issue, where the progress bar actually reflects the percentage until it times out. Since there is another command logged simultaneously when the progress changes, I don't think that users will think there is an issue and the command is running twice.

In addition, the requestTimeout unusually doesn't even come close to actually timing out (anecdotally). More often, the command is waiting on the response and the responseTimeout is quite large, and if most users are modifying it they are increasing it rather than decreasing (again, anecdotally).

Here's an example of what a more realistic use case might look like:

Option 1

Option 2

Most realistically though, is when the command is not alone and is contained in a suite of tests (the user might be debugging with only, but in that case they'll understand the request is working). In this case, the test won't even be expanded until isLongRunning becomes true. This is what that looks like:

It's not even discernible which version I used in that demo. Therefore I suggest that we use option 2 to simplify the code and display the real percentage complete.

@JessicaSachs regarding the issue with toggling, during our brief discussion on this issue earlier I believe I said that was introduced when I added a key to the progress bar to make it rerender. That is not the case. I had previously thought this was a non issue because when toggling the test (not the hook) on and off the progress bar would remain correct. That's because that toggle actually does cause a rerender, while clicking the hook does not. I'll file a separate issue and PR.

[jennifer-shehane]

@panzarino Yeah I can see your point. Design 2 is fine with me.