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(net-stubbing): fix waiting on responses with no-body status codes #9097

Merged
merged 3 commits into from Dec 2, 2020

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Nov 4, 2020

User facing changelog

  • Fixed an issue where HTTP responses that cannot have a body (like HTTP 304 and HTTP 204) could not be awaited using cy.intercept.
  • Fixed an issue where HTTP redirects could not be awaited using cy.intercept unless dynamically intercepted.

Additional details

How has the user experience changed?

PR Tasks

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 4, 2020

Thanks for taking the time to open a PR!

@flotwig flotwig changed the title fix(route2): fix waiting on responses that send no body fix(route2): fix waiting on responses with no-body status codes Nov 4, 2020
@cypress
Copy link

cypress bot commented Nov 4, 2020



Test summary

4446 0 49 2


Run details

Project cypress
Status Passed
Commit 1657874
Started Dec 2, 2020 6:41 PM
Ended Dec 2, 2020 6:52 PM
Duration 11:02 💡
OS Linux Debian - 10.5
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

@eilgin
Copy link

eilgin commented Nov 24, 2020

Hello @flotwig, i've just seen that cypress is now launching 6.0 (congrats!). I'd like to know if this PR is still relevant?

@flotwig
Copy link
Contributor Author

flotwig commented Nov 24, 2020

Hello @flotwig, i've just seen that cypress is now launching 6.0 (congrats!). I'd like to know if this PR is still relevant?

@eilgin yep, this is still a work in progress, i hope to get it in the next release

@flotwig flotwig marked this pull request as ready for review December 1, 2020 20:38
@flotwig flotwig requested a review from bahmutov December 1, 2020 20:47
const NO_BODY_STATUS_CODES = [204, 304]

export function responseMustHaveEmptyBody (req: IncomingMessage, res: IncomingMessage) {
return _.some([_.includes(NO_BODY_STATUS_CODES, res.statusCode), _.invoke(req.method, 'toLowerCase') === 'head'])
Copy link
Contributor

Choose a reason for hiding this comment

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

wow, some people write such interesting code just to avoid ||

Copy link
Contributor Author

Choose a reason for hiding this comment

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

job security

@kevinold
Copy link
Contributor

kevinold commented Dec 1, 2020

@flotwig It appears that 302 POST's might also be grouped into this. See this failure when attempting to cy.intercept for /logout in the RWA.

cypress-io/cypress-realworld-app@1b54220

Test to reproduce the issue: https://github.com/cypress-io/cypress-realworld-app/pull/676/files#diff-8781249f032a90ab825382873828906eed104c18c2a93d20e9de10b4872b0df6R143

Screen Shot 2020-12-01 at 3 44 37 PM

@bahmutov bahmutov self-requested a review December 2, 2020 02:25
bahmutov
bahmutov previously approved these changes Dec 2, 2020
Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

Works, verified against 304 in cypress-io/cypress-example-recipes#598

@flotwig
Copy link
Contributor Author

flotwig commented Dec 2, 2020

@flotwig It appears that 302 POST's might also be grouped into this. See this failure when attempting to cy.intercept for /logout in the RWA.

nice, i was able to reproduce this with a simple cy.intercept('/redirect'), and it led me to fixing the underlying logic issue that was causing #8999 along with your issue with redirects. So this PR should also fix waiting on redirects.

Funnily enough, if you intercept the redirect, it does work as-is:

it('intercepts redirects as expected', function () {
const href = `/fixtures/generic.html?t=${Date.now()}`
const url = `/redirect?href=${encodeURIComponent(href)}`
cy.intercept('/redirect', (req) => {
req.reply((res) => {
expect(res.statusCode).to.eq(301)
expect(res.headers.location).to.eq(href)
res.send()
})
})
.as('redirect')
.intercept('/fixtures/generic.html').as('dest')
.then(() => fetch(url))
.wait('@redirect')
.wait('@dest')
})

@flotwig flotwig changed the title fix(route2): fix waiting on responses with no-body status codes fix(net-stubbing): fix waiting on responses with no-body status codes Dec 2, 2020
@flotwig flotwig requested a review from bahmutov December 2, 2020 17:33
bahmutov
bahmutov previously approved these changes Dec 2, 2020
Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

nice

@flotwig flotwig merged commit 810a9b4 into develop Dec 2, 2020
kevinold pushed a commit to cypress-io/cypress-realworld-app that referenced this pull request Dec 3, 2020
kevinold pushed a commit to cypress-io/cypress-realworld-app that referenced this pull request Dec 3, 2020
kevinold pushed a commit to cypress-io/cypress-realworld-app that referenced this pull request Apr 6, 2021
* wip - convert cy.route to cy.http; remove cy.server; adjust to use JSON.parse()

* fix delete bankaccounts route

* fix empty list test using cy.http

* wip - experiments

* uncomment wait

* reproduction and notes around cy.http issue with public transactions stub

* set run mode to not retry

* remove only

* rename cy.http to cy.intercept

* remove JSON.parse per cypress-io/cypress-example-kitchensink#458

* update to get user id if not 401 response

* add glob-match to usersSearch for cy.intercept

* wip - iterate

* install nocache

* configure routes not to cache (temporarily)

* note about failing cy.intercept for POST /logout

* adjust cy.intercepts; notes on open Cypress issues

* update matching for stubbed public transactions

* restore to substring match for mocked response

* uninstall and remove nocache for routes since resolved in cypress-io/cypress#9097

* adjust to use response.statusCode for fix in cypress-io/cypress#9097 to come in 6.1.0

* update syntax and expectation

* delete if-none-match request header to prevent cached results for cy.wait

* update to use regex for cy.intercept url matchers; delete if-none-match request header to prevent cached results for cy.wait

* update /notifications to delete if-none-match request header to prevent cached results for cy.wait

* cleanup and remove if-none-match for all feed routes, but continues to return 304

* refactor to delete if-none-match for all routes in a global beforeEach

* restore limit to 10

* update route matching to be exact

* update to get url from response object

* update to use response.statusCode

* update transaction view spec to use response.statusCode

* update intercepts to use minimatch

* cleanup

* cleanup

* example of res.send fixture not returning fixture

* add delay middleware for all mobile viewport tests

* cleanup

* cleanup

* update mobile cy.intercept to throttle all routes

* fix syntax error

* update from throttle -> setThrottle

* update matching for global intercepts

* update path matching

* update matching for global intercepts

* update intercept matching

* temporary disable throttling for mobile

* fix patch matching

* update matching for cy.intercept

* wait for public transactions

Co-authored-by: Amir Rustamzadeh <amir@cypress.io>
obaidaattaee pushed a commit to obaidaattaee/cypress-realworld-app that referenced this pull request Nov 30, 2021
* wip - convert cy.route to cy.http; remove cy.server; adjust to use JSON.parse()

* fix delete bankaccounts route

* fix empty list test using cy.http

* wip - experiments

* uncomment wait

* reproduction and notes around cy.http issue with public transactions stub

* set run mode to not retry

* remove only

* rename cy.http to cy.intercept

* remove JSON.parse per cypress-io/cypress-example-kitchensink#458

* update to get user id if not 401 response

* add glob-match to usersSearch for cy.intercept

* wip - iterate

* install nocache

* configure routes not to cache (temporarily)

* note about failing cy.intercept for POST /logout

* adjust cy.intercepts; notes on open Cypress issues

* update matching for stubbed public transactions

* restore to substring match for mocked response

* uninstall and remove nocache for routes since resolved in cypress-io/cypress#9097

* adjust to use response.statusCode for fix in cypress-io/cypress#9097 to come in 6.1.0

* update syntax and expectation

* delete if-none-match request header to prevent cached results for cy.wait

* update to use regex for cy.intercept url matchers; delete if-none-match request header to prevent cached results for cy.wait

* update /notifications to delete if-none-match request header to prevent cached results for cy.wait

* cleanup and remove if-none-match for all feed routes, but continues to return 304

* refactor to delete if-none-match for all routes in a global beforeEach

* restore limit to 10

* update route matching to be exact

* update to get url from response object

* update to use response.statusCode

* update transaction view spec to use response.statusCode

* update intercepts to use minimatch

* cleanup

* cleanup

* example of res.send fixture not returning fixture

* add delay middleware for all mobile viewport tests

* cleanup

* cleanup

* update mobile cy.intercept to throttle all routes

* fix syntax error

* update from throttle -> setThrottle

* update matching for global intercepts

* update path matching

* update matching for global intercepts

* update intercept matching

* temporary disable throttling for mobile

* fix patch matching

* update matching for cy.intercept

* wait for public transactions

Co-authored-by: Amir Rustamzadeh <amir@cypress.io>
@flotwig flotwig deleted the issue-8999 branch January 24, 2022 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Route returning 204 (No content) can't be waited for with route2 cy.route2 times out waiting for image load
4 participants