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

mgr/dashboard: fix ESOCKETTIMEDOUT E2E failure #41104

Merged

Conversation

avanthakkar
Copy link
Contributor

@avanthakkar avanthakkar commented Apr 30, 2021

Increasing responseTimeout may help resolve the e2e failures with cy.visit()/cy.request() failed to load error, as it's considered the most accepted solution here cypress-io/cypress#7062 too, which happens only in CI jobs. The top failure jobs in last 3 months have the same error of timeout/failed to load.
Example error message : cy.request() timed out waiting 30000ms for a response from your server.
Fixes: https://tracker.ceph.com/issues/49828
Signed-off-by: Avan Thakkar athakkar@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@avanthakkar avanthakkar requested a review from a team April 30, 2021 12:45
@avanthakkar avanthakkar added this to In progress in Dashboard via automation Apr 30, 2021
@avanthakkar avanthakkar requested review from pereman2 and Waadkh7 and removed request for a team April 30, 2021 12:45
@alfonsomthd alfonsomthd moved this from In progress to Review in progress in Dashboard May 3, 2021
@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from 6ee7802 to b32017e Compare May 3, 2021 12:15
@avanthakkar avanthakkar requested a review from a team as a code owner May 3, 2021 12:15
@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from b32017e to b652b6a Compare May 3, 2021 12:31
@avanthakkar avanthakkar changed the title mgr/dashboard: fix ESOCKETTIMEDOUT E2E failure [WIP] mgr/dashboard: fix ESOCKETTIMEDOUT E2E failure May 3, 2021
@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from b652b6a to f58ffb6 Compare May 3, 2021 13:48
@avanthakkar
Copy link
Contributor Author

avanthakkar commented May 3, 2021

I think we don't need to make changes in script in order to change the responseTimeout. As when we run npx cypress run... it looks for cypress.json file and takes the environment var values from there unless we have changed it in script itself like for baseURL. Also no need to change defaultCommandTimeout as it's taken as 120s only while running e2e jenkins job. @alfonsomthd

@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from f58ffb6 to 0f71418 Compare May 3, 2021 16:31
@avanthakkar avanthakkar changed the title [WIP] mgr/dashboard: fix ESOCKETTIMEDOUT E2E failure mgr/dashboard: fix ESOCKETTIMEDOUT E2E failure May 3, 2021
@avanthakkar
Copy link
Contributor Author

I don't understand why we do this in afterEach hook ? Shouldn't we instead call something like cy.logout(), as for all components testing we login in beforeEach() hook and I think it also keeps up with best practices. Thoughts @ceph/dashboard ?

@alfonsomthd
Copy link
Contributor

I don't understand why we do this in afterEach hook ? Shouldn't we instead call something like cy.logout(), as for all components testing we login in beforeEach() hook and I think it also keeps up with best practices. Thoughts @ceph/dashboard ?

Try to run the whole suite without the afterEach hook to check if it's not needed anymore.

@avanthakkar
Copy link
Contributor Author

avanthakkar commented May 4, 2021

I don't understand why we do this in afterEach hook ? Shouldn't we instead call something like cy.logout(), as for all components testing we login in beforeEach() hook and I think it also keeps up with best practices. Thoughts @ceph/dashboard ?

Try to run the whole suite without the afterEach hook to check if it's not needed anymore.

Yes..but I'm sure abt why we redirect to 403 page..I understand atleast why we should redirect (I think it's following best practices in order to clean up the state generated by the current test(s)), but not sure about 403 page in specific..can't we do instead logout?

But in any case the fix should remain the same i.e. to increase responseTimeout. It sounds a bit strange here as usually we should increase it if cy.request() is taking long enough than expected, but looking at this thread I think it's the same issue here, also the workaround suggested looks the same.

@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from 0f71418 to 0617400 Compare May 4, 2021 08:12
@avanthakkar
Copy link
Contributor Author

jenkins test dashboard

@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from 0617400 to 5fed826 Compare May 4, 2021 09:58
@avanthakkar avanthakkar requested a review from epuertat May 4, 2021 09:58
@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from 5fed826 to 1159d82 Compare May 4, 2021 10:14
Fixes: https://tracker.ceph.com/issues/49828
Signed-off-by: Avan Thakkar <athakkar@redhat.com>
@avanthakkar avanthakkar force-pushed the fix-ESOCKETTIMEDOUT-e2e-failure branch from 1159d82 to bf4894f Compare May 4, 2021 11:09
@avanthakkar
Copy link
Contributor Author

jenkins test dashboard

Dashboard automation moved this from Review in progress to Reviewer approved May 5, 2021
@@ -7,6 +7,7 @@
"supportFile": "cypress/support/index.ts",
"video": false,
"defaultCommandTimeout": 120000,
"responseTimeout": 45000,
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we need to increase the response time from 30 secs to 45 secs? Are we talking about HTTP requests? If that's the case I feel that the issue will be somewhere else :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we sure we need to increase the response time from 30 secs to 45 secs? Are we talking about HTTP requests? If that's the case I feel that the issue will be somewhere else :/

I followed this cypress-io/cypress#7062 (comment) and I think it's same issue for us. Somehow cy.request() is taking some more time in cypress browser env so cy.visit() after it is failing to load(also I tried removing that afterEach hook(which isn't consider best practice) but it's somehow broke some tests).

@epuertat epuertat moved this from Reviewer approved to Ready-to-merge in Dashboard May 19, 2021
Dashboard automation moved this from Ready-to-merge to Reviewer approved May 19, 2021
@epuertat epuertat merged commit 67fb1b1 into ceph:master May 19, 2021
Dashboard automation moved this from Reviewer approved to Done May 19, 2021
@epuertat epuertat deleted the fix-ESOCKETTIMEDOUT-e2e-failure branch May 19, 2021 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Dashboard
  
Done
5 participants